Skip to content

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

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.

Sign up for GitHub

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

Merged
nodejs-github-bot merged 8 commits into from
Jul 28, 2025
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c837d14
src: add percentage support to --max-old-space-size
Asaf-Federman Jul 23, 2025
90f8288
doc: remove unrelated changes
Asaf-Federman Jul 23, 2025
84d3433
src: introduce max-old-space-size-percentage as a new cli flag
Asaf-Federman Jul 23, 2025
ebaa850
doc: remove doc related to modified implementation
Asaf-Federman Jul 23, 2025
c377756
doc: change max-old-space-size-percentage type to be number
Asaf-Federman Jul 23, 2025
a23ece3
doc: revert max-old-space-size-percentage type to be a string
Asaf-Federman Jul 23, 2025
b919043
doc: fix linting and doc issues
Asaf-Federman Jul 23, 2025
8e5f5ea
src: handle UINT64_MAX (according to libuv doc) and remove redundant …
Asaf-Federman Jul 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
src: handle UINT64_MAX (according to libuv doc) and remove redundant … 
…validation
  • Loading branch information
@Asaf-Federman
Asaf-Federman committed Jul 23, 2025
commit 8e5f5ea35119a0d66790eb5ac3cd61af924d4120
25 changes: 12 additions & 13 deletions src/node_options.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include <algorithm>
#include <array>
#include <charconv>
#include <cstdint>
#include <limits>
#include <sstream>
#include <string_view>
Expand Down Expand Up @@ -112,12 +113,6 @@ void PerIsolateOptions::HandleMaxOldSpaceSizePercentage(
std::vector<std::string>* errors,
std::string* max_old_space_size_percentage) {
std::string original_input_for_error = *max_old_space_size_percentage;
// Check if the percentage value is empty
if (max_old_space_size_percentage->empty()) {
errors->push_back("--max-old-space-size-percentage must not be empty");
return;
}

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

@jasnell jasnell Jul 18, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@Asaf-Federman Asaf-Federman Jul 18, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@jasnell jasnell Jul 18, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@MoLow MoLow Jul 22, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Expand All @@ -131,17 +126,21 @@ void PerIsolateOptions::HandleMaxOldSpaceSizePercentage(
return;
}

// Get available memory in MB
size_t total_memory = uv_get_total_memory();
size_t constrained_memory = uv_get_constrained_memory();
// Get available memory in bytes
uint64_t total_memory = uv_get_total_memory();
uint64_t constrained_memory = uv_get_constrained_memory();

// Use constrained memory if available, otherwise use total memory
size_t available_memory =
(constrained_memory > 0) ? constrained_memory : total_memory;
// This logic correctly handles the documented guarantees.
// Use uint64_t for the result to prevent data loss on 32-bit systems.
uint64_t available_memory =
(constrained_memory > 0 && constrained_memory != UINT64_MAX)
? constrained_memory
: total_memory;

// Convert to MB and calculate the percentage
size_t memory_mb = available_memory / (1024 * 1024);
size_t calculated_mb = static_cast<size_t>(memory_mb * percentage / 100.0);
uint64_t memory_mb = available_memory / (1024 * 1024);
uint64_t calculated_mb = static_cast<size_t>(memory_mb * percentage / 100.0);

// Convert back to string
max_old_space_size = std::to_string(calculated_mb);
Expand Down
8 changes: 4 additions & 4 deletions test/parallel/test-max-old-space-size-percentage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,27 +99,27 @@ testPercentages.forEach((percentage) => {
// 50% should be roughly half of 100%
const ratio50to100 = heapSizes[50] / heapSizes[100];
assert(
ratio50to100 >= 0.45 && ratio50to100 <= 0.55,
ratio50to100 >= 0.4 && ratio50to100 <= 0.6,
`50% heap size should be roughly half of 100% (got ${ratio50to100.toFixed(2)}, expected ~0.5)`
);

// 25% should be roughly quarter of 100%
const ratio25to100 = heapSizes[25] / heapSizes[100];
assert(
ratio25to100 >= 0.2 && ratio25to100 <= 0.3,
ratio25to100 >= 0.15 && ratio25to100 <= 0.35,
`25% heap size should be roughly quarter of 100% (got ${ratio25to100.toFixed(2)}, expected ~0.25)`
);

// 75% should be roughly three-quarters of 100%
const ratio75to100 = heapSizes[75] / heapSizes[100];
assert(
ratio75to100 >= 0.7 && ratio75to100 <= 0.8,
ratio75to100 >= 0.65 && ratio75to100 <= 0.85,
`75% heap size should be roughly three-quarters of 100% (got ${ratio75to100.toFixed(2)}, expected ~0.75)`
);

// Validate heap sizes against system memory
const totalMemoryMB = Math.floor(os.totalmem() / 1024 / 1024);
const margin = 5; // 5% margin
const margin = 10; // 5% margin
testPercentages.forEach((percentage) => {
const upperLimit = totalMemoryMB * ((percentage + margin) / 100);
assert(
Expand Down