src: add percentage support to --max-old-space-size #59082

Asaf-Federman · 2025-07-16T07:45:02Z

This commit adds support for specifying --max-old-space-size as a percentage of system memory, in addition to the existing MB format. A new HandleMaxOldSpaceSizePercentage method parses percentage values, validates that they are within the 0-100% range, and provides clear error messages for invalid input. The heap size is now calculated based on available system memory when a percentage is used.

Test coverage has been added for both valid and invalid cases. Documentation and the JSON schema for CLI options have been updated with examples for both formats.

Refs: #57447

nodejs-github-bot · 2025-07-16T07:45:07Z

Review requested:

@nodejs/config
@nodejs/startup

legendecas

I'm -1 on re-interpreting a V8 flag with different semantic in Node.js as it could lead to confusions. I'd be open to a new flag though.

doc/api/cli.md

Asaf-Federman · 2025-07-16T09:30:29Z

I'm -1 on re-interpreting a V8 flag with different semantic in Node.js as it could lead to confusions. I'd be open to a new flag though.

Isn't it dangerous having two different flags for the same underlying v8 option?
If user will specify both flags, which one will take precedence? Or should we throw exception in this case?

legendecas · 2025-07-16T10:55:46Z

We can define that a flag will alway take precedence over another, e.g. --max-old-space-size-percentage always override --max-old-space-size. I'd only consider it is harmful if a flag conditionally take precedence.

See node --v8-options:

  --max-heap-size (max size of the heap (in Mbytes) both max_semi_space_size and max_old_space_size take precedence. All three flags cannot be specified at the same time.)

It could also be a good practice to abort if both flags are set. When all three heap size options --max-heap-size=20 --max-old-space-size=20 --max-semi-space-size=20 are set at the same time, V8 aborts.

And, like mentioned, there are three heap size options. I'm -0 on introduce 3 new flags on each of them, TBH.

Asaf-Federman · 2025-07-16T11:01:43Z

We can define that a flag will alway take precedence over another, e.g. --max-old-space-size-percentage always override --max-old-space-size. I'd only consider it is harmful if a flag conditionally take precedence.

See node --v8-options:
  --max-heap-size (max size of the heap (in Mbytes) both max_semi_space_size and max_old_space_size take precedence. All three flags cannot be specified at the same time.)
It could also be a good practice to abort if both flags are set. When all three heap size options --max-heap-size=20 --max-old-space-size=20 --max-semi-space-size=20 are set at the same time, V8 aborts.

And, like mentioned, there are three heap size options. I'm -0 on introduce 3 new flags on each of them, TBH.

As long as there's an agreed way of action, I don't mind changing the implementation

jasnell · 2025-07-18T15:50:18Z

Another approach would be to see if v8 would accept a patch adding this as an additional v8 flag... I can see it potentially being generically useful for all v8 users

jasnell · 2025-07-18T16:05:13Z

src/node_options.cc

+
+  // Parse the percentage value
+  char* end_ptr;
+  double percentage =


Why double? It's likely best to just allow integer values here between 0 and 100. Not critical, of course, I'd just find it a bit cleaner

Why double? It's likely best to just allow integer values here between 0 and 100. Not critical, of course, I'd just find it a bit cleaner

Should this be an integer, or would a float provide better flexibility for nodes with a large amount of memory?

Really depends on how granular we want it to be. Personally I'd be fine with integer and just not worry about fractional percentages but I'd be interested in what others think

if it was a breaking change I would agree, but this is a new feature, why not support more usecases even if they are rare?

Asaf-Federman · 2025-07-18T16:30:26Z

Another approach would be to see if v8 would accept a patch adding this as an additional v8 flag... I can see it potentially being generically useful for all v8 users

#57447

I have already approached the V8 team, and they have indicated that a contribution to the Node.js project would be more appropriate.
Please see here

staskorz · 2025-07-20T14:14:55Z

This will be a huge help, appreciate you working on this!

codecov · 2025-07-21T14:55:56Z

Codecov Report

❌ Patch coverage is 89.28571% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.06%. Comparing base (6bb08f7) to head (8e5f5ea).
⚠️ Report is 29 commits behind head on main.

Files with missing lines	Patch %	Lines
src/node_options.cc	86.36%	1 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #59082      +/-   ##
==========================================
+ Coverage   90.04%   90.06%   +0.02%     
==========================================
  Files         648      648              
  Lines      190978   191006      +28     
  Branches    37434    37438       +4     
==========================================
+ Hits       171964   172028      +64     
+ Misses      11641    11606      -35     
+ Partials     7373     7372       -1

Files with missing lines	Coverage Δ
src/node.cc	`74.93% <100.00%> (+0.20%)`	⬆️
src/node_options.h	`97.87% <ø> (ø)`
src/node_options.cc	`84.65% <86.36%> (+0.04%)`	⬆️

... and 33 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

doc/api/cli.md

This commit adds support for specifying --max-old-space-size as a percentage of system memory, in addition to the existing MB format. A new HandleMaxOldSpaceSizePercentage method parses percentage values, validates that they are within the 0-100% range, and provides clear error messages for invalid input. The heap size is now calculated based on available system memory when a percentage is used. Test coverage has been added for both valid and invalid cases. Documentation and the JSON schema for CLI options have been updated with examples for both formats. Refs: nodejs#57447

…validation

Asaf-Federman · 2025-07-23T18:22:31Z

I've just force-pushed an update to this branch. I had to amend the commit history to unify the author identity across all commits.

The underlying code logic has not changed.

Apologies for any inconvenience this may cause.

test/parallel/test-max-old-space-size-percentage.js

Asaf-Federman · 2025-07-25T18:01:44Z

The CI failures look unrelated to my changes

nodejs-github-bot · 2025-07-25T20:13:03Z

CI: https://ci.nodejs.org/job/node-test-pull-request/68201/

Asaf-Federman · 2025-07-27T20:15:01Z

It looks like the CI failures are due to unrelated issues, and I'm not sure how to get past them.

Could someone with knowledge of our CI pipeline lend a hand or point me in the right direction? Any help would be greatly appreciated!

Thanks!

nodejs-github-bot · 2025-07-28T00:43:59Z

CI: https://ci.nodejs.org/job/node-test-pull-request/68255/

nodejs-github-bot · 2025-07-28T05:06:18Z

Landed in 0bbe7c3

This commit adds support for specifying --max-old-space-size as a percentage of system memory, in addition to the existing MB format. A new HandleMaxOldSpaceSizePercentage method parses percentage values, validates that they are within the 0-100% range, and provides clear error messages for invalid input. The heap size is now calculated based on available system memory when a percentage is used. Test coverage has been added for both valid and invalid cases. Documentation and the JSON schema for CLI options have been updated with examples for both formats. Refs: nodejs#57447 PR-URL: nodejs#59082 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: theanarkh <theratliter@gmail.com> Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>

This commit adds support for specifying --max-old-space-size as a percentage of system memory, in addition to the existing MB format. A new HandleMaxOldSpaceSizePercentage method parses percentage values, validates that they are within the 0-100% range, and provides clear error messages for invalid input. The heap size is now calculated based on available system memory when a percentage is used. Test coverage has been added for both valid and invalid cases. Documentation and the JSON schema for CLI options have been updated with examples for both formats. Refs: #57447 PR-URL: #59082 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: theanarkh <theratliter@gmail.com> Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Jul 16, 2025

legendecas requested changes Jul 16, 2025

View reviewed changes

doc/api/cli.md Outdated Show resolved Hide resolved

Asaf-Federman requested a review from legendecas July 17, 2025 12:25

jasnell reviewed Jul 18, 2025

View reviewed changes

jasnell approved these changes Jul 21, 2025

View reviewed changes

legendecas reviewed Jul 21, 2025

View reviewed changes

doc/api/cli.md Outdated Show resolved Hide resolved

MoLow approved these changes Jul 22, 2025

View reviewed changes

Asaf-Federman added 8 commits July 23, 2025 21:19

doc: remove unrelated changes

90f8288

src: introduce max-old-space-size-percentage as a new cli flag

84d3433

doc: remove doc related to modified implementation

ebaa850

doc: change max-old-space-size-percentage type to be number

c377756

doc: revert max-old-space-size-percentage type to be a string

a23ece3

doc: fix linting and doc issues

b919043

src: handle UINT64_MAX (according to libuv doc) and remove redundant …

8e5f5ea

…validation

Asaf-Federman force-pushed the feat/max-old-space-size-percentage branch from a121c70 to 8e5f5ea Compare July 23, 2025 18:19

Asaf-Federman requested a review from legendecas July 24, 2025 07:51

legendecas added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label Jul 24, 2025

legendecas reviewed Jul 24, 2025

View reviewed changes

test/parallel/test-max-old-space-size-percentage.js Show resolved Hide resolved

legendecas approved these changes Jul 25, 2025

View reviewed changes

legendecas added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 25, 2025

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 25, 2025

This comment was marked as outdated.

Sign in to view

theanarkh approved these changes Jul 25, 2025

View reviewed changes

daeyeon approved these changes Jul 25, 2025

View reviewed changes

This comment was marked as outdated.

Sign in to view

daeyeon added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. labels Jul 28, 2025

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 28, 2025

nodejs-github-bot merged commit 0bbe7c3 into nodejs:main Jul 28, 2025
68 checks passed

github-actions bot mentioned this pull request Aug 11, 2025

2025-08-12, Version 24.6.0 (Current) #59449

Merged

Jul	AUG	Sep
	14
2024	2025	2026

Uh oh!

src: add percentage support to --max-old-space-size #59082

src: add percentage support to --max-old-space-size #59082

Uh oh!

Conversation

Asaf-Federman commented Jul 16, 2025

Uh oh!

nodejs-github-bot commented Jul 16, 2025

Uh oh!

legendecas left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Asaf-Federman commented Jul 16, 2025

Uh oh!

legendecas commented Jul 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Asaf-Federman commented Jul 16, 2025

Uh oh!

jasnell commented Jul 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jasnell Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

Asaf-Federman Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

jasnell Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

MoLow Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

Asaf-Federman commented Jul 18, 2025

Uh oh!

staskorz commented Jul 20, 2025

Uh oh!

codecov bot commented Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Asaf-Federman commented Jul 23, 2025

Uh oh!

Uh oh!

This comment was marked as outdated.

Asaf-Federman commented Jul 25, 2025

Uh oh!

nodejs-github-bot commented Jul 25, 2025

Uh oh!

This comment was marked as outdated.

Asaf-Federman commented Jul 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Jul 28, 2025

Uh oh!

Uh oh!

nodejs-github-bot commented Jul 28, 2025

Uh oh!

Uh oh!

legendecas left a comment •

edited

Loading

legendecas commented Jul 16, 2025 •

edited

Loading

jasnell commented Jul 18, 2025 •

edited

Loading

codecov bot commented Jul 21, 2025 •

edited

Loading

Asaf-Federman commented Jul 27, 2025 •

edited

Loading