-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
[BUG] npm 10.4.0+ running out of memory with no package-lock.json
#7276
Comments
Pretty sure your dependence on “18” is a typo (not sure if it’s relevant) also, your engines.npm isn’t using ^ and doesn’t match the npm version you’re reporting. |
Thanks, @ljharb -- report fixed; same result. |
I can reproduce this myself under WSL with half the host's RAM available (8 GB) and it's quite a serious regression especially since these npm versions ship with Node.js LTS. npm v10.2.4 works fine that I could try with the Node.js 20.11.1 docker image. v20.12.0 which has npm 10.5.0 has the memory leak issue which brings down my machine since it starts swapping. Could someone have a look at it please? |
I got the same problem I believe with: #7351 |
Somewhat related to #7276 and #7463. I don't think there is a reason to cache the promise here. And if we ever did choose to replace this with an LRUCache we would need to know the size of what we are caching which will be easier if we only cache the resulting manifest. I also added a comment about why I think we are removing the license from manifests here. License was removed in #7126 which looks to be a purposeful change but I could not find a reason. Adding the license back in causes many snapshots to fail because the license is now present in lockfiles, so that's how I came up with the comment.
We have identified the primary culprit here, it's an unbound cache that saves all packuments in Arborist. Because we now fetch full packuments for all situations, this is causing issues on what would otherwise have been a working system configuration. This was always technically a problem. We are doing some metrics so we know exactly the right way to fix this, and fix it in a way that hopefully even benefits those running on more memory limited environments. You can see some initial proof of concept testing at #7463. Ultimately we plan on putting in an There is also an existing bug that has up until now not meaningfully impacted npm, and that is when the cache has stale data the |
Thank you for the update, @wraithgar -- have you identified a root-cause commit for this regression? |
Yes, as I said.
However it must be reiterated this was always a problem, it just didn't manifest as commonly on systems with plenty of memory. This commit pushed the line much lower, is all. |
Somewhat related to #7276 and #7463. I don't think there is a reason to cache the promise here. And if we ever did choose to replace this with an LRUCache we would need to know the size of what we are caching which will be easier if we only cache the resulting manifest. I also added a comment about why I think we are removing the license from manifests here. License was removed in #7126 which looks to be a purposeful change but I could not find a reason. Adding the license back in causes many snapshots to fail because the license is now present in lockfiles, so that's how I came up with the comment.
This adds a new packument cache that is an instance of `lru-cache`. It uses that package's ability to limit content based on size, and has some multipliers based on research to mostly correctly approximate the correlation between packument size and its memory usage. It also limits the total size of the cache based on the actual heap available. Closes: #7276 Related: npm/pacote#369
This adds a new packument cache that is an instance of `lru-cache`. It uses that package's ability to limit content based on size, and has some multipliers based on research to mostly correctly approximate the correlation between packument size and its memory usage. It also limits the total size of the cache based on the actual heap available. Closes: #7276 Related: npm/pacote#369
Is there an existing issue for this?
This issue exists in the latest npm version
Current Behavior
It appears the latest dependency resolving algorithm is taking more memory than before. The following fails:
as does
with
I have described this issue on SO as well.
Expected Behavior
The following succeeds:
as does:
Steps To Reproduce
or
with
Environment
The text was updated successfully, but these errors were encountered: