-
Notifications
You must be signed in to change notification settings - Fork 24.8k
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
fix(ivy): LFrame
needs to release memory on leaveView()
#35156
Conversation
d6fd7d5
to
ceb6373
Compare
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 code LGTM but I'm surprised that there is code size increase (and it is even bigger than what is written in the code as the CI check fails: https://circleci.com/gh/angular/angular/621857#tests/containers/3)
I think this is the case where the limit was close to 500 bytes and this CL took os over, so now you have the 500 bytes plus this CL. |
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.
LGTM
Root cause is that for perf reasons we cache `LFrame` so that we don't have to allocate it all the time. To be extra fast we clear the `LFrame` on `enterView()` rather that on `leaveView()`. The implication of this strategy is that the deepest `LFrame` will retain objects until the `LFrame` allocation depth matches the deepest object. The fix is to simply clear the `LFrame` on `leaveView()` rather then on `enterView()` Fix angular#35148
Root cause is that for perf reasons we cache `LFrame` so that we don't have to allocate it all the time. To be extra fast we clear the `LFrame` on `enterView()` rather that on `leaveView()`. The implication of this strategy is that the deepest `LFrame` will retain objects until the `LFrame` allocation depth matches the deepest object. The fix is to simply clear the `LFrame` on `leaveView()` rather then on `enterView()` Fix #35148 PR Close #35156
…35156) Root cause is that for perf reasons we cache `LFrame` so that we don't have to allocate it all the time. To be extra fast we clear the `LFrame` on `enterView()` rather that on `leaveView()`. The implication of this strategy is that the deepest `LFrame` will retain objects until the `LFrame` allocation depth matches the deepest object. The fix is to simply clear the `LFrame` on `leaveView()` rather then on `enterView()` Fix angular#35148 PR Close angular#35156
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Root cause is that for perf reasons we cache
LFrame
so that we don't have to allocate it all the time. To be extra fast we clear theLFrame
onenterView()
rather that onleaveView()
. The implication of this strategy is that the deepestLFrame
will retain objects until theLFrame
allocation depth matches the deepest object.The fix is to simply clear the
LFrame
onleaveView()
rather then onenterView()
Fix #35148
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information