fix(core): fix i18n sites SSG memory leak - require.cache #10599
Conversation
This file contains 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
slorber
added
the
pr: performance
⚡️ Lighthouse report for the deploy preview of this PR
|
slorber
changed the title
✅ [V2]
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
Size Change: +352 B (0%) Total Size: 11.2 MB ℹ️ View Unchanged
|
slorber
added a commit
that referenced
this pull request
Oct 22, 2024
slorber
added a commit
that referenced
this pull request
Oct 22, 2024
slorber
added a commit
that referenced
this pull request
Oct 22, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Motivation
When building a localized Docusaurus site, the memory would keep growing as we iterate sequentially over the locales to build. This can lead to potential out-of-memory errors for locales at the end.
Each localized site would keep a lot of strings in memory, related to the server bundle modules. This is because we run SSG using node-eval which is a tiny wrapper around
vm.Script.Unfortunately,
vm.Scriptdoesn't really isolate memory like workers or subprocesses. At the end of the SSG process, many strings remain retained in memory related to the localized server-side JS modules loaded during the SSG process.When we run SSG, the
require()calls in the server bundle are using thecreateRequire(serverBundlePath)we provide to globals. The thing is, Node.jsrequire()andcreateRequire()share a global cache that keeps server-side loaded modules for a given locale in memory, even after the site of that locale as finished building.The solution was to force the cleanup of the cache manually after the SSG process.
Note: we may still have memory leaks, but this one is probably 10x more significant than any others. We'll try to fix the smaller leaks later. The Rspack team also think they might have one.
Note: slightly related, we also removed the force terminate
process.exit(0)in #10410 which may hide memory leaks users have in their own code, such assetIntervalstarted during the render init / SSG render phase (antipattern). This is another reason for us to try to isolate SSG render in a dedicated worker.Other attempts
I tried to isolate the SSG process in a
worker_threadsbut it couldn't work because some data structured we need can't be serialized (helmet state, we need a breaking change to refactor that).I also tried to build sequentially the localized sites in distinct
worker_threadsbut got weird SIGSEGV / SIGBUS errors, which might be because of N-API usage in Rspack.I tried building localized sites in
child_processand it worked, but I'd prefer not to for now because we would need to add other options that let users define the node parameters of that subprocess, including memory usage like--max-old-space-sizewhich is not automatically forwarded from main to subprocess (and both processes might not use the same value 🤷♂️ )Cleaning up the require cache works even though it doesn't totally solve the isolation problem.
Test Plan
CI + local tests
Test links
https://deploy-preview-10599--docusaurus-2.netlify.app/
Details
Here's what a heap dump looks like after building 4 locales, by putting
require('v8').writeHeapSnapshot();at the end ofbuild.tsHere's a failing build of 2 locales without the fix:
Here's a successful build of 2 locales with this fix: