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

Reduce memory footprint of storing fs erros in cache #393

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

shYkiSto
Copy link

@shYkiSto shYkiSto commented Aug 18, 2023

We are using enhanced-resolve together with jest test runner, and noticed substantial memory leaks using this combo. Further inspection revealed that original FS errors are retaining references to sandbox environments that jest creates to keep individual tests isolated from one another. These references occur in the internal representation of the Error object used for lazy-computation of the stack property later in the process (when user code happens to read from it for the first time).

Cloning the original error allows GC to collect that stack trace information and all retained objects that in turn helps to resolve the memory leak, and reduce the memory usage. One drawback to this is that the stack property would have to be computed eagerly, which comes at a cost of consuming more CPU. But since it's rarely required, and gets messed up due caching aspect of it anyway - a trade off has to be made to omit stack traces from the fs errors altogether.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 18, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@rtsao
Copy link

rtsao commented Oct 18, 2023

Any feedback @alexander-akait @TheLarkInn ? This would be nice fix to get landed

@Twipped
Copy link

Twipped commented Oct 30, 2023

Is there a workaround for this that we can employ while we wait for this to be merged? We're seeing substantial memory problems in our jest suites that we've traced back to this exact issue.

@alexander-akait
Copy link
Member

Sorry for delay, it sounds like the problem is with jest...

@shYkiSto
Copy link
Author

Sorry for delay, it sounds like the problem is with jest...

Jest's approach to sandboxing definitely presents a challenge here. But at the same time there's not much that can be done there, as it's essentially the user code (enhanced-resolve) causing the leakage by holding references to the test environments that prevents them from being GC'd, as summarized in the PR description.

@alexander-akait
Copy link
Member

@shYkiSto Sorry for the delay, can you measure the memory or demonstrate the leak (small reproduce example)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants