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
Install crashes because getPrettyLogs() tries to allocate a huge string object #4949
Comments
I was able to workaround these crashes by manually editing pnpm/dist/pnpm.cjs and commenting out the call to @zkochan what is the right way to fix this? By design, is it important for the file node_modules/.pnpm-debug.log {
"0 info pnpm": {
"message": "Using hooks from: /Users/nobody/code/repo/common/temp-split/global-pnpmfile.cjs",
"prefix": "/Users/nobody/code/repo/common/temp-split"
},
"1 info pnpm": {
"message": "readPackage hook is declared. Manifests of dependencies might get overridden",
"prefix": "/Users/nobody/code/repo/common/temp-split"
},
"2 debug pnpm:scope": {
"selected": 329,
"total": 329,
"workspacePrefix": "/Users/nobody/code/repo/common/temp-split"
}, The above file format is more difficult to write/read incrementally. Compare with something like this: possible different design {
"key": "0 info pnpm",
"message": "Using hooks from: /Users/nobody/code/repo/common/temp-split/global-pnpmfile.cjs",
"prefix": "/Users/nobody/code/repo/common/temp-split"
}
{
"key": "1 info pnpm",
"message": "readPackage hook is declared. Manifests of dependencies might get overridden",
"prefix": "/Users/nobody/code/repo/common/temp-split"
}
{
"key": "2 debug pnpm:scope",
"selected": 329,
"total": 329,
"workspacePrefix": "/Users/nobody/code/repo/common/temp-split"
} (In this possible design, the file reader can delimit blocks by looking for |
I don't care that much how that log file will look like. If you have a fix, submit it and I'll accept it |
We can fix it. 👍 |
BTW is there an option to disable this logging entirely? That could provide an interim workaround, and for very large installs it might even speed up the install time. |
No, there is no such option. |
Would you want to add an option for this? (What should it be called?) We can measure the effect of disabling this logging, and if it makes a significant difference, maybe we could implement the option in the same PR that fixes the memory leak. |
If it makes performance bad, we just need to remove logging. No option is needed. |
Ok, we'll investigate and follow up. 👍 FYI however it is fixed, we'll probably need to backport it to PNPM 6. |
Generally, why does PNPM keep the entire (For example, was there some concern that the target folder won't exist if the output file is created too early? Or that the file might not get closed if the process terminates? Or that file writes are slow?) |
I don't remember any reason. I think it can be changed to a file that is incrementally written. |
pnpm version: 6.32.23 (however the same code is still in the latest 7.4.0)
Code to reproduce the issue:
Encountered by invoking these command:
...in a very large monorepo with these settings in
.npmrc
:Crash output:
Analysis
RangeError: Invalid string length
is Node.js's way of reporting that it has run out of memory.getPrettyLogs()
appears to be creating a large in-memory data structure that is an array of all the log entries in.pnpm-debug.log
. If I understand the code correctly,JSON.stringify()
will then allocate a single massive JavaScript string, which can potentially be very large:file-reporter/src/index.ts
A possible solution would be to write the log incrementally rather than stringifying the entire data structure at once.
Additional information:
node -v
prints: v14.19.2The text was updated successfully, but these errors were encountered: