cli: allow --huge-max-old-generation-size in NODE_OPTIONS - #32251
cli: allow --huge-max-old-generation-size in NODE_OPTIONS#32251addaleax wants to merge 1 commit into
Conversation
Sorry, something went wrong.
|
CI: https://ci.nodejs.org/job/node-test-pull-request/29816/ (:white_check_mark:) |
Sorry, something went wrong.
wesleytodd
left a comment
There was a problem hiding this comment.
Should this also add what it increased to?
As you said on twitter: "For machines with a lot of RAM, that behavior is changed to 4GB on x64 if --huge-max-old-generation-size is passed to V8."
Sorry, something went wrong.
|
Honestly, I doubt that many people would use end up using this – I’m okay with documenting it more verbosely, but it gets tricky to say that this increases the limit of the default limit of the heap (and not the actual limit like --max-old-space-size does). Ultimately, it probably makes more sense to document --max-old-space-size explicitly in the CLI docs + the man page – this could be mentioned as a detail there, then. (I was kind of surprised to see that we don’t already document --max-old-space-size, tbh.) |
Sorry, something went wrong.
|
I guess I have one other question: Is there a reason node is not defaulting to --huge-max-old-generation-size? It seems like for most cases this is the correct default for >=16gb situations. Not many folks are running servers with that (maybe a huge assumption), and for local machines it seems like a safe default. |
Sorry, something went wrong.
|
I think this goes back to: should we be documenting V8 flags? While I agree these flags are important, it's still not something under our control. The behavior can change across V8 versions and we don't have test their behaviors, only for their existence. I'm not against it, especially for important flags like memory-tuning, but maybe there's something else we can do? Could we add a NODE_V8_OPTIONS environment variable instead, so it's explicit those flags are not supported by us? For the memory-related flags, could Node.js expose a "simplified" tuning flag (and those wanting something more fine-tuned can use the V8 flags)? |
Sorry, something went wrong.
@wesleytodd Yeah, I've thought about that too. It would be fairly easy to make that happen, I'm not sure why it's not the default in V8. @mmarchini Yeah, I think it's a bit of a grey are, but I'd prefer to keep them unless we have a reason to disable some of them... |
Sorry, something went wrong.
PR-URL: #32251 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: David Carlier <devnexen@gmail.com>
PR-URL: #32251 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: David Carlier <devnexen@gmail.com>
PR-URL: nodejs#32251 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: David Carlier <devnexen@gmail.com>
PR-URL: #32251 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: David Carlier <devnexen@gmail.com>
Checklist