-
-
Notifications
You must be signed in to change notification settings - Fork 802
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
feat[codegen]: deallocate variables after last use #4219
base: master
Are you sure you want to change the base?
Conversation
deallocate variables after last use, improving memory usage.
marked this for inclusion in 0.4.1, but depending on when our next audit can be, might push to 0.4.2 |
use_count inside of Expr is dangerous, since Expr() can be called multiple times for a given node. separate the analysis.
var = self.vars[varname] | ||
for s in self._scopes: | ||
if s not in var.blockscopes: | ||
# defer deallocation until we hit the end of its scope |
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 this accurate? we defer the dealoc until we hit its scope again (not necessarily its end)
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.
or if "it" refers to the scope then still we might have to wait until multiple scopes finish
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.
"it" refers to the scope of the variable
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.
but we sweep after statements, right?
a: uint256 = 0
for i: uint256 in range(1, 3):
a += i # <- can't dealoc, defer
b: uint256 = 0 # a already deallocated, swept after the for
b += 1
return b # actual scope end but a is already deallocated
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.
yea, this is the idea here
@@ -40,6 +40,7 @@ class VariableRecord: | |||
defined_at: Any = None | |||
is_internal: bool = False | |||
alloca: Optional[Alloca] = None | |||
system: bool = False |
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.
can't we rather modify the analysis than create a new type of variable given it's used only at one place?
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.
yea, it's a bit of a kludge, until we refactor sqrt
to be either pure IR or a library function
Given that we log write as both write+read, the dealoc won't happen until the last write (although the memory variable might be long dead at that point). So, in this context, the write operation is considered as "use". |
i think a write is a use |
deallocate variables after last use, improving memory usage.
What I did
How I did it
How to verify it
Commit message
Commit message for the final, squashed PR. (Optional, but reviewers will appreciate it! Please see our commit message style guide for what we would ideally like to see in a commit message.)
Description for the changelog
Cute Animal Picture