[ConstraintSolver] NFC: refactor candidate solver to use constraint system arena #11078
Conversation
xedin
force-pushed
the
shrink-memory-leak
branch
from
2a10240 to
48f37ab
Compare
xedin
force-pushed
the
shrink-memory-leak
branch
2 times, most recently
from
8f4878b to
a7df2b5
Compare
|
/cc @jrose-apple mostly changes in the |
|
@swift-ci please smoke test |
lib/Sema/CSSolver.cpp
Outdated
There was a problem hiding this comment.
I think I would just skip the whole change for ASTContext and use the BumpPtrAllocator directly. You should be able to do this:
MutableArrayRef<ValueDecl *> decls(Allocator.Allocate<ValueDecl *>(choices.size()),
choices.size());There was a problem hiding this comment.
Done! Thanks for the suggestion!
xedin
force-pushed
the
shrink-memory-leak
branch
from
a7df2b5 to
499be58
Compare
|
@swift-ci please clean smoke test |
xedin
force-pushed
the
shrink-memory-leak
branch
from
499be58 to
0eeab31
Compare
|
@swift-ci please smoke test |
lib/Sema/CSSolver.cpp
Outdated
There was a problem hiding this comment.
What keeps this from outliving the constraint system?
There was a problem hiding this comment.
If expression is correct there is not going to be any OSR expressions in it after ExprRewriter because all of the ambiguities have been resolved. But I think the right fix is to allocate to constraint system arena all of the intermediate arrays that shrink creates but at the end of the shrink phrase re-allocate all of the final arrays to perm arena to make sure that diagnostics are ok.
There was a problem hiding this comment.
Right, that first part isn't good enough for code completion and indexing, where the expression may not be complete or correct.
…ystem arena While shrinking we have to allocate containers for the reduced domains for some of the candidates, it's currently done using permanent arena of the `ASTContext` allocator. This patch changes candidate solver to use arena associated with the parent constraint system, which significantly limits lifetime of domain containers.
xedin
force-pushed
the
shrink-memory-leak
branch
from
0eeab31 to
7a1bae8
Compare
|
@swift-ci please smoke test |
|
@rudkx all of the intermediate arrays used for decls are going to be allocated temporarily while main constraint system is alive but the final versions are going to be still permanent because we might need to keep OSR longer for diagnostics. |
3 participants
While shrinking we have to allocate containers for the reduced domains
for some of the candidates, it's currently done using permanent arena
of the
ASTContextallocator. This patch changes candidate solver touse arena associated with the parent constraint system, which significantly
limits lifetime of domain containers.