GeezerVison

Review requested:

• @nodejs/config
• @nodejs/startup

cc @aduh95

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

While some of the test failures are flakes, the test added in this backport PR is failing on 32-bit Linux on ARM and 32-bit Windows. e.g.

Since this flag's behavior depends on V8's complex and platform-specific heap sizing logic, creating a test that perfectly models these heuristics is likely to be brittle. Therefore, I think the most pragmatic solution is to skip these assertions.

What are your thoughts?

Since this flag's behavior depends on V8's complex and platform-specific heap sizing logic, creating a test that perfectly models these heuristics is likely to be brittle. Therefore, I think the most pragmatic solution is to skip these assertions.

What are your thoughts?

My concern is that we don't run tests on 32-bit platforms on main/Node.js 24.x, so this is the first time we've tried this feature on those platforms. And while allowances might be made for the comparison in these tests to not be exactly half, the tests would seem to indicate that a 50% value is leading in some cases to a larger heap being allocated than 100% in some cases which indicates the opposite of the desired effect?

Hi @richardlau,

Following up on the discussion, I've run some tests on the behavior of --max-old-space-size on a 32-bit ARM (armv7l) system. The results show some significant inconsistencies that likely explain why --max-old-space-size-percentage would also be unreliable on this architecture.

Test Environment

• Node.js Version: v22.19.0
• Architecture: armv7l (32-bit ARM)
• Script Used:

  // heap_check.js
  const v8 = require('v8');
  
  // Get heap statistics from the V8 engine.
  const stats = v8.getHeapStatistics();
  
  // A simple function to format bytes into megabytes.
  const toMB = (bytes) => (bytes / 1024 / 1024).toFixed(2);
  
  console.log('V8 Heap Statistics:');
  console.log(`- Used Heap Size: ${toMB(stats.used_heap_size)} MB`);
  console.log(`- Total Heap Available: ${toMB(stats.total_available_size)} MB`);
  console.log(`- Heap Size Limit: ${toMB(stats.heap_size_limit)} MB`);
  
  // The --max-old-space-size flag primarily affects this limit.
  // The total limit is slightly higher to account for other memory spaces.

Results

Here is a summary of the requested heap size versus the actual limit reported by V8.

  root@e0332c781f29:/# node --version
  v22.19.0
  root@e0332c781f29:/# uname -m
  armv7l
  root@e0332c781f29:/# node --max-old-space-size=2000 heap_check.js
  V8 Heap Statistics:
  - Used Heap Size: 2.00 MB
  - Total Heap Available: 2009.87 MB
  - Heap Size Limit: 2012.00 MB
  root@e0332c781f29:/# node --max-old-space-size=5000 heap_check.js
  V8 Heap Statistics:
  - Used Heap Size: 1.99 MB
  - Total Heap Available: 913.87 MB
  - Heap Size Limit: 916.00 MB
  root@e0332c781f29:/# node --max-old-space-size=13000 heap_check.js
  V8 Heap Statistics:
  - Used Heap Size: 1.99 MB
  - Total Heap Available: 721.87 MB
  - Heap Size Limit: 724.00 MB
  root@e0332c781f29:/# node --max-old-space-size=14000 heap_check.js
  V8 Heap Statistics:
  - Used Heap Size: 1.99 MB
  - Total Heap Available: 1721.87 MB
  - Heap Size Limit: 1724.00 MB

Analysis & Conclusion

The data shows that V8's memory allocation on 32-bit ARM is not just capped, but behaves erratically when given large values. The most telling result is that requesting 13000 MB results in a smaller heap limit than requesting 5000 MB.

This underlying instability confirms that the heap size is not handled predictably on this architecture. It strongly suggests that the --max-old-space-size-percentage flag would be subject to the same unreliable behavior, as it's ultimately bound by this same allocation logic.

I strongly believe we should skip those tests for 32bit architecture.
Maybe it's worth adding the lack of support for 32bit architecture to the cli documentation as well.

WDYT?

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

The tests seem to fail due to flakiness

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

Hi @richardlau,
the CI is 🆗, can we please move this effort forward?

Thanks in advance :)

I think 8733b91 should land on main first before we can proceed

Hi @richardlau @aduh95,
I've updated the PR with ffdc838 (after it was merged to main)

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

Hi @richardlau,
how can we move this PR toward landing?

@aduh95 Do you think we can pull this into #60230 (the last release before Node.js 22 enters maintenance)?

Landed on 7067d79...660d573