-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Free typechecker state and ASTs when they are no longer needed #7280
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
"""Generic node traverser visitor""" | ||
|
||
from mypy.traverser import TraverserVisitor | ||
from mypy.nodes import Block, MypyFile | ||
|
||
|
||
class TreeFreer(TraverserVisitor): | ||
def visit_block(self, block: Block) -> None: | ||
super().visit_block(block) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would clearing just the top-level blocks be sufficient here, instead of recursively clearing? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to go recursively or we'll miss nested structures that appear in the symbol table. |
||
block.body.clear() | ||
|
||
|
||
def free_tree(tree: MypyFile) -> None: | ||
"""Free all the ASTs associated with a module. | ||
|
||
This needs to be done recursively, since symbol tables contain | ||
references to definitions, so those won't be freed but we want their | ||
contents to be. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We might fix this in long term, see #5159, but definitely not right now. I don't have any additional comments here, thanks for improving this! |
||
""" | ||
tree.accept(TreeFreer()) | ||
tree.defs.clear() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason why we can't do
self.tree = None
? Or maybe doingself.tree.defs.clear()
or something?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason we need to go deeper than just clearing
defs
is that the symbol table contains references to functions and classes and the like, and we need to clear out their bodies.It would be possible to drive this from the symbol table, and I considered that, but it is more complex for not much gain. (Probably a bit faster, but probably doesn't matter)
(We actually probably should also do
self.tree.defs.clear()
, which would free a little bit more memory, though probably not a substantial amount.)