Skip to content
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(compiler-cli): prevent prior compilations from being retained in watch builds #42537

Closed

Conversation

JoostK
Copy link
Member

@JoostK JoostK commented Jun 9, 2021

In watch builds, the compiler attempts to reuse as much information from
a prior compilation as possible. To accomplish this, it keeps a
reference to the most recently succeeded TraitCompiler, which contains
all analysis data for the program. However, TraitCompiler has an
internal reference to an IncrementalBuild, which is itself built on
top of its prior state. Consequently, all prior compilations continued
to be referenced, preventing garbage collection from cleaning up these
instances.

This commit changes the AnalyzedIncrementalState to no longer retain
a TraitCompiler instance, but only the analysis data it contains. This
breaks the retainer path to the prior incremental state, allowing it to
be garbage collected.

@JoostK JoostK added target: patch This PR is targeted for the next patch release area: compiler Issues related to `ngc`, Angular's template compiler labels Jun 9, 2021
@ngbot ngbot bot added this to the Backlog milestone Jun 9, 2021
@google-cla google-cla bot added the cla: yes label Jun 9, 2021
…watch builds

In watch builds, the compiler attempts to reuse as much information from
a prior compilation as possible. To accomplish this, it keeps a
reference to the most recently succeeded `TraitCompiler`, which contains
all analysis data for the program. However, `TraitCompiler` has an
internal reference to an `IncrementalBuild`, which is itself built on
top of its prior state. Consequently, all prior compilations continued
to be referenced, preventing garbage collection from cleaning up these
instances.

This commit changes the `AnalyzedIncrementalState` to no longer retain
a `TraitCompiler` instance, but only the analysis data it contains. This
breaks the retainer path to the prior incremental state, allowing it to
be garbage collected.
@JoostK JoostK force-pushed the ngtsc/incremental/memleak-traitcompiler branch from dd3aa84 to 22c44cd Compare June 9, 2021 19:46
@JoostK JoostK added action: merge The PR is ready for merge by the caretaker action: presubmit The PR is in need of a google3 presubmit labels Jun 9, 2021
@JoostK JoostK marked this pull request as ready for review June 9, 2021 20:29
@AndrewKushnir
Copy link
Contributor

Presubmit.

@AndrewKushnir AndrewKushnir removed the action: presubmit The PR is in need of a google3 presubmit label Jun 9, 2021
alxhub pushed a commit that referenced this pull request Jun 9, 2021
…watch builds (#42537)

In watch builds, the compiler attempts to reuse as much information from
a prior compilation as possible. To accomplish this, it keeps a
reference to the most recently succeeded `TraitCompiler`, which contains
all analysis data for the program. However, `TraitCompiler` has an
internal reference to an `IncrementalBuild`, which is itself built on
top of its prior state. Consequently, all prior compilations continued
to be referenced, preventing garbage collection from cleaning up these
instances.

This commit changes the `AnalyzedIncrementalState` to no longer retain
a `TraitCompiler` instance, but only the analysis data it contains. This
breaks the retainer path to the prior incremental state, allowing it to
be garbage collected.

PR Close #42537
@alxhub alxhub closed this in 22bda22 Jun 9, 2021
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jul 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: compiler Issues related to `ngc`, Angular's template compiler cla: yes target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants