perf(@angular/build): reduce memory usage + improve times when i18n inlining - #33768
Open
mattlewis92 wants to merge 4 commits into
Open
perf(@angular/build): reduce memory usage + improve times when i18n inlining#33768mattlewis92 wants to merge 4 commits into
mattlewis92 wants to merge 4 commits into
Conversation
The inliner is covered end to end by the localized builder specs and the `i18n` e2e suite, but `I18nInliner` itself has no unit tests, so behaviour that only shows up across several inline requests is untested. These add that layer: each locale gets its own translations when several are inlined in sequence through one pool, a locale without translations keeps the original messages without reporting them as missing, a locale with translations reports the ones it is missing, a modified file's source map is remapped back to the original sources, and template updates are inlined for both a translated and an untranslated locale. A single thread is used so that every request of every locale is served by the same Worker, which is what makes translation state retained between requests observable.
…s by reference `inlineForLocale` passes the locale's translations to `workerPool.run()` once per file, so every request structure-clones the whole set of messages into a Worker. For an application with a few thousand localized chunks and a catalog of tens of thousands of messages, that is tens of gigabytes of short-lived allocation and minutes of serialization on the builder's main thread. Because each clone is several megabytes it lands in V8's large object space, where only a major collection reclaims it, so a build with a heap ceiling sized for the machine can exhaust the machine before the collector intervenes. The messages are now serialized once per locale and passed as a Blob. Cloning a Blob shares its data by reference, which is the same reason the application files are already passed that way. Each Worker deserializes the messages once per locale and retains only the active locale, so at most one set of messages is held per Worker. `node:v8` serialization is used rather than JSON so that the messages arrive in the Worker exactly as the structured clone delivered them today. `translate` only reads from the messages, so sharing one deserialized set across the files of a locale is safe. Measured on an application with 1,721 localized chunks across 9 locales, where the largest catalog serializes to 6.0MB: inlining 400 chunks across 4 locales went from 23,679ms and 14,504MB peak RSS to 915ms and 1,118MB, with byte-identical output.
… locale Each file's persistent cache key is built from `file.hash`, the filename, and the inline options. The options include the locale's translations, so the multi-megabyte set of messages was fed into a fresh SHA-256 for every localized file: for an application with a few thousand localized chunks that is tens of gigabytes of hash input per build, all of it recomputing the same value. The options are digested once per locale instead, so each file's key is derived from a fixed 32 bytes. This changes the keys, so the first build after this change repopulates the i18n cache.
…apping `generateMap` is `generateDecodedMap` followed by encoding the mappings to VLQ, and remapping decodes whatever it is handed. The encoded string was therefore built only to be parsed straight back, holding two representations of the largest structure involved in inlining a file at the point where a build is already at its peak. `generateDecodedMap` is used instead. The mapping resolution is unchanged: `hires`, `source` and `includeContent` are untouched, and VLQ round-trips integers exactly, so the remapped output is byte-identical. Verified over the output of a 10MB chunk with 488 messages, whose map carries 78,036 mapping segments across 303 sources.
angular-robot
Bot
added
area: performance
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request optimizes the i18n inlining process by serializing translation messages using V8 serialization and passing them to workers as a Blob to share them by reference instead of copying. It also switches to generating decoded source maps to reduce peak memory usage, and adds a comprehensive test suite for I18nInliner. The reviewer feedback focuses on a performance optimization to avoid stringifying the potentially large translation object when generating the cache key. Instead, the reviewer suggests computing the SHA-256 hash of the serialized V8 buffer once during serialization and reusing it.
alan-agius4
added
the
action: review
Member
|
This has been on the backlog so thank you for taking a look. |
Member
|
Looks like there is one lint failure. Once that is addressed, this should be good to merge. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Checklist
Please check to confirm your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
For context, our application has 33,000 translated strings and each translation file is 2-3MB in size. Our CI builds recently started running out of memory when building localised builds due to the overhead of repeatedly transferring these large translation files when running i18n inlining during builds (we had gone up to a 96GB runner which was still running out of memory 😅)
Issue Number: N/A
What is the new behavior?
Peak memory usage has dropped by 32GB+ of memory when building the localised app in CI (I don't have the exact drop, but from a test on my local machine, inlining 400 chunks across 4 locales went from ~14GB to ~1GB), and this also made the app build significantly faster: ~10m to ~7m (~1m of this was removing a local workaround that capped the inliner pool at 4 workers, which the memory reduction made unnecessary)
Does this PR introduce a breaking change?
Other information
Broken out into 4 commits to make it easier to review: