Skip to content

[v19 backport] fix(core): reduce total memory usage of various migration schematics #60776

Closed
JoostK wants to merge 2 commits intoangular:19.2.xfrom
JoostK:core/schematics/tsurge-reduce-memory-v19
Closed

[v19 backport] fix(core): reduce total memory usage of various migration schematics #60776
JoostK wants to merge 2 commits intoangular:19.2.xfrom
JoostK:core/schematics/tsurge-reduce-memory-v19

Conversation

@JoostK
Copy link
Copy Markdown
Member

@JoostK JoostK commented Apr 7, 2025

This is cherry-picking #60386 and #60774 to the patch branch for v19.

Closes #59813

crisbeto and others added 2 commits April 7, 2025 21:37
…the CLI (angular#60386)

Currently when we reuse a Tsurge migration is reused externally, there's some glue code that needs to be executed in a specific order. The code gets copied between the different migrations which is error-prone and means that bugs may have to be fixed several times.

These changes move the common steps out into a separate function so that only the migration-specific logic (mostly instantiation and logging) is left in the schematic.

PR Close angular#60386
This commit changes Tsurge's operation within angular-devkit (i.e. the CLI) to
no longer retain all programs across all migrations. This isn't necessary for
so-called "funnel" migrations so not retaining the programs for those migrations
is a pure performance win. The "complex" migrations may see increased execution time
given that the program is now being recreated for the actual migration phase to run,
although reduced memory pressure may help alleviate this overhead. Since this new
approach should help prevent Node from running out of memory and failing entirely
this is preferred over a potentially increased execution time.

Fixes angular#59813
@JoostK JoostK added area: core Issues related to the framework runtime target: patch This PR is targeted for the next patch release area: migrations Issues related to `ng update`/`ng generate` migrations labels Apr 7, 2025
@ngbot ngbot bot added this to the Backlog milestone Apr 7, 2025
@pullapprove pullapprove bot requested a review from crisbeto April 7, 2025 19:41
crisbeto
crisbeto previously approved these changes Apr 7, 2025
@JoostK
Copy link
Copy Markdown
Member Author

JoostK commented Apr 7, 2025

I'm AFK at the moment and don't quite see why this is failing the way it does, but I haven't ran the tests locally assuming the cherry-picks would stick (there were no meaningful conflicts except for one file addition). Will look again tomorrow.

@JeanMeche
Copy link
Copy Markdown
Member

JeanMeche commented Apr 7, 2025

The breakage was unrelated and fixed by #60753.

@JeanMeche JeanMeche force-pushed the core/schematics/tsurge-reduce-memory-v19 branch from 67a547b to eb672b9 Compare April 7, 2025 23:01
@JeanMeche JeanMeche added the action: merge The PR is ready for merge by the caretaker label Apr 7, 2025
atscott pushed a commit that referenced this pull request Apr 8, 2025
…the CLI (#60386) (#60776)

Currently when we reuse a Tsurge migration is reused externally, there's some glue code that needs to be executed in a specific order. The code gets copied between the different migrations which is error-prone and means that bugs may have to be fixed several times.

These changes move the common steps out into a separate function so that only the migration-specific logic (mostly instantiation and logging) is left in the schematic.

PR Close #60386

PR Close #60776
atscott pushed a commit that referenced this pull request Apr 8, 2025
…60776)

This commit changes Tsurge's operation within angular-devkit (i.e. the CLI) to
no longer retain all programs across all migrations. This isn't necessary for
so-called "funnel" migrations so not retaining the programs for those migrations
is a pure performance win. The "complex" migrations may see increased execution time
given that the program is now being recreated for the actual migration phase to run,
although reduced memory pressure may help alleviate this overhead. Since this new
approach should help prevent Node from running out of memory and failing entirely
this is preferred over a potentially increased execution time.

Fixes #59813

PR Close #60776
@atscott
Copy link
Copy Markdown
Contributor

atscott commented Apr 8, 2025

This PR was merged into the repository by commit 9241615.

The changes were merged into the following branches: 19.2.x

@angular-automatic-lock-bot
Copy link
Copy Markdown

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 May 9, 2025
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: core Issues related to the framework runtime area: migrations Issues related to `ng update`/`ng generate` migrations target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants