performance.getEntries and performance.getEntriesByName cause call stack size to be exceeded

[luketaher]

Version

v18.14.0 - v22.7.0

Platform

Darwin x86_64

Subsystem

perf_hooks

What steps will reproduce the bug?

Executing this script:

// Established warning threshold for number of performance entries
// https://github.com/nodejs/node/blob/v22.x/lib/internal/perf/observe.js#L105
const performanceEntryBufferWarnSizeThreshold = 1e6;

for (let numEntries = 1e4; numEntries <= performanceEntryBufferWarnSizeThreshold; numEntries += 1e4) {
  console.log(`Testing ${numEntries} entries`);

  for (let i = 0; i < numEntries; i++) {
    performance.mark(`mark-${i}`);
  }

  performance.getEntriesByName('mark-0')

  performance.clearMarks();
}

console.log('Done');

How often does it reproduce? Is there a required condition?

100% of the time

What is the expected behavior? Why is that the expected behavior?

The expected behaviour is that the script completes successfully given the number of performance entries at any time is below the established warning threshold:

...
Testing 980000 entries
Testing 990000 entries
Testing 1000000 entries
Done

What do you see instead?

With the default stack size of 984kB the script consistently fails at an order of magnitude lower than the established warning threshold:

...
Testing 110000 entries
Testing 120000 entries
Testing 130000 entries
node:internal/perf/observe:517
    ArrayPrototypePushApply(bufferList, markEntryBuffer);
    ^

RangeError: Maximum call stack size exceeded
    at filterBufferMapByNameAndType (node:internal/perf/observe:517:5)
    at Performance.getEntriesByName (node:internal/perf/performance:106:12)
    at Object.<anonymous> (test_file.js:12:15)
    at Module._compile (node:internal/modules/cjs/loader:1546:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1691:10)
    at Module.load (node:internal/modules/cjs/loader:1317:32)
    at Module._load (node:internal/modules/cjs/loader:1127:12)
    at TracingChannel.traceSync (node:diagnostics_channel:315:14)
    at wrapModuleLoad (node:internal/modules/cjs/loader:217:24)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:166:5)

Node.js v22.7.0

Additional information

This bug appears to have been introduced in v18.14.0 when #44445 added a usage of the ArrayPrototypePushApply primordial in filterBufferMapByNameAndType as part of an effort to eliminate usages of ArrayPrototypeConcat.

ArrayPrototypePushApply is a variadic function, which appears to be problematic in this instance because it’s being called with each element in the buffer of performance entries - each passed as individual arguments. The size of the performance entries buffer is effectively unbounded, so at scale the underlying Array.prototype.push method is being called with an unbounded number of arguments resulting in massive stack frames which cause the maximum call stack size to be exceeded.

If this is, as it appears, to be a general risk with the usage of variadic primordials with unbounded argument lists it may at least be worth a callout in the primordials documentation on variadic functions.

[anonrig]

cc @nodejs/primordials

[ljharb]

That's a good risk to document of ArrayPrototypePushApply.

[benjamingr]

I'll open a PR to fix this though I'm not sure how to test this (i.e. it tests internal implementation detail of V8 rather than a JavaScript behavior and I'm hesitant to add an explicit test since it tests that but I'll do what people thing).

[aduh95]

ArrayPrototypePushApply is a variadic function

It’s not, it takes exactly two arguments. %Array.prototype.push% is though, but I wonder how that affects the call stack size.

[aduh95]

The error can be reproduced by running [].push(...Array.from({ length:1e6 })). I've tested also with Firefox, Safari, and Chromium, they all throw a similar error, maybe it's a restriction in the spec?

[ljharb]

yes, argument lists are limited to 2^32 - 1 items, i believe.