From 66e030a399fc2a6afea65bd99ce5f45eff210053 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Fri, 28 Aug 2026 18:54:40 +0000 Subject: [PATCH 01/14] lib: add runId, fileRunId, entryFile, namePath to node:bench Signed-off-by: James M Snell Assisted-by: Opencode --- doc/api/bench.md | 45 +++++-- lib/internal/bench_runner/benchmark.js | 20 ++- lib/internal/bench_runner/cli.js | 90 +++++++++++++- lib/internal/bench_runner/harness.js | 117 +++++++++++++++--- .../bench-runner/identity-child-a.cjs | 11 ++ .../bench-runner/identity-child-b.cjs | 11 ++ .../bench-runner/identity-entry-a.cjs | 10 ++ .../bench-runner/identity-entry-b.cjs | 10 ++ test/fixtures/bench-runner/identity-hook.cjs | 9 ++ .../bench-runner/identity-preload.cjs | 9 ++ .../fixtures/bench-runner/identity-shared.cjs | 11 ++ test/fixtures/bench-runner/identity-suite.cjs | 10 ++ .../bench-runner/malformed-record.cjs | 13 +- test/parallel/test-bench-cli.js | 101 +++++++++++++++ test/parallel/test-bench-create-runner.js | 10 ++ 15 files changed, 440 insertions(+), 37 deletions(-) create mode 100644 test/fixtures/bench-runner/identity-child-a.cjs create mode 100644 test/fixtures/bench-runner/identity-child-b.cjs create mode 100644 test/fixtures/bench-runner/identity-entry-a.cjs create mode 100644 test/fixtures/bench-runner/identity-entry-b.cjs create mode 100644 test/fixtures/bench-runner/identity-hook.cjs create mode 100644 test/fixtures/bench-runner/identity-preload.cjs create mode 100644 test/fixtures/bench-runner/identity-shared.cjs create mode 100644 test/fixtures/bench-runner/identity-suite.cjs diff --git a/doc/api/bench.md b/doc/api/bench.md index 7cb5720ffe28..9c24d04ee4f0 100644 --- a/doc/api/bench.md +++ b/doc/api/bench.md @@ -137,6 +137,12 @@ Benchmark files passed to `--bench` should declare benchmarks but must not call `--bench-warmup`, `--bench-reporter`, and `--bench-reporter-destination`. See the [command-line options documentation][] for details. +Preload modules passed through `--require` or `--import` should not declare +benchmarks. Such declarations are not associated with an entry file and have +an `entryFile` value of `null`. Their `fileRunId` identifies the runner or child +execution in which they occurred. With process isolation, a preload is evaluated +and its declarations run once for every benchmark child process. + ## Benchmark reporters The built-in reporters are available from the scheme-only @@ -262,9 +268,19 @@ benchmark. Later benchmarks continue to run. A timeout or abort cannot interrupt synchronous JavaScript and does not forcibly cancel asynchronous work that ignores `context.signal`. -The stable `benchId` is based on the source file, hierarchical suite and -benchmark names, and canonicalized parameters. Declaring the same identity -more than once reports an error rather than merging the samples. +The `benchId` is based on the declaration source file, hierarchical suite and +benchmark names, and canonicalized parameters. It is stable for repeated runs +from the same source location, but the embedded source value is not normalized +across checkout roots, module formats, operating systems, or path casing. + +Execution scope is represented separately. A `runId` identifies one logical +run, while `fileRunId` identifies a file runner or child execution within that +run. The `entryFile` field records which entry-file import caused a declaration +and is `null` for declarations made by preload modules. +The same `benchId` can therefore occur under multiple `fileRunId` values when +entry files use a shared declaration helper. Declaring the same `benchId` more +than once within one file execution scope reports an error rather than merging +the samples. ### `bench.skip([name][, options], fn)` @@ -537,14 +553,20 @@ The events are emitted in execution order: * `'bench:diagnostic'` * `'bench:summary'` -Every benchmark-scoped event contains `benchId` and `parentId`. +Every benchmark-scoped event contains `runId`, `fileRunId`, `entryFile`, +`benchId`, `parentId`, and `namePath`. `runId` and `fileRunId` are opaque and +change between runs. `entryFile` identifies the top-level benchmark file whose +loading caused the declaration, while `file` identifies the source location of +the declaration itself. `parentId` is based on the containing suite's source +file and hierarchical name path. + `'bench:complete'` data contains a [benchmark result][]. A failed result has an additional `error` property and may contain samples recorded before the error. A skipped result has an additional `skip` property and an empty `samples` array. `'bench:diagnostic'` reports suite and hook errors. `'bench:summary'` -contains overall `success`, `counts`, `duration_ns`, and `file` properties. The -`file` is {string|null}; it is `null` when the summary aggregates multiple -files. +contains overall `runId`, `fileRunId`, `entryFile`, `success`, `counts`, +`duration_ns`, and `file` properties. `fileRunId`, `entryFile`, and `file` are +{string|null}; they are `null` when the summary aggregates multiple files. ## Sample result @@ -560,10 +582,15 @@ Each measured sample has the following properties: A completed benchmark result contains: -* `benchId` {string} The stable benchmark identity. +* `runId` {string} The opaque logical run identity. +* `fileRunId` {string} The opaque file runner or child execution identity. +* `entryFile` {string|null} The top-level file that caused this declaration. +* `benchId` {string} The stable declaration identity within the same source + layout. * `parentId` {string|null} The stable containing suite identity. * `name` {string} The benchmark name. -* `file` {string} The source file. +* `namePath` {string\[]} The hierarchical suite and benchmark names. +* `file` {string} The declaration source file. * `line` {number} The source line. * `column` {number} The source column. * `tags` {string\[]} The inherited canonical tags. diff --git a/lib/internal/bench_runner/benchmark.js b/lib/internal/bench_runner/benchmark.js index d3c55c1b2ef5..dae60f6e0599 100644 --- a/lib/internal/bench_runner/benchmark.js +++ b/lib/internal/bench_runner/benchmark.js @@ -48,6 +48,7 @@ const { structuredClone } = require('internal/worker/js_transferable'); const { bigint: hrtime } = process.hrtime; const kDefaultSamples = 30; const kDefaultWarmup = 0; +const kEmptyNamePath = ObjectFreeze([]); const kEmptyParams = ObjectFreeze({ __proto__: null }); const kEmptyTags = ObjectFreeze([]); @@ -163,10 +164,19 @@ class Suite extends AsyncResource { this.name = name; this.fn = fn; this.loc = createLocation(loc, harness.entryFile); + this.isRoot = isRoot; + this.fileScope = isRoot ? null : + (parent.isRoot ? harness.getFileScope() : parent.fileScope); + this.namePath = isRoot ? kEmptyNamePath : + ObjectFreeze(getNamePath(parent, name)); + this.suiteId = isRoot ? null : JSONStringify([ + this.loc.file, + this.namePath, + ]); + this.parentId = isRoot || parent.isRoot ? null : parent.suiteId; this.only = validated.only; this.skip = validated.skip; this.tags = validated.tags; - this.isRoot = isRoot; this.children = []; this.hooks = { __proto__: null, @@ -214,17 +224,15 @@ class Bench extends AsyncResource { this.warmup = warmup; this.timeout = timeout; this.outerSignal = signal; - this.namePath = getNamePath(parent, name); + this.fileScope = parent.isRoot ? harness.getFileScope() : parent.fileScope; + this.namePath = ObjectFreeze(getNamePath(parent, name)); this.fullName = ArrayPrototypeJoin(this.namePath, ' '); this.benchId = JSONStringify([ this.loc.file, this.namePath, this.params, ]); - this.parentId = parent.isRoot ? null : JSONStringify([ - this.loc.file, - getNamePath(parent.parent, parent.name), - ]); + this.parentId = parent.suiteId; this.finished = false; this.result = null; this.completion = PromiseWithResolvers(); diff --git a/lib/internal/bench_runner/cli.js b/lib/internal/bench_runner/cli.js index ce54be0e1e22..6c717aab6fff 100644 --- a/lib/internal/bench_runner/cli.js +++ b/lib/internal/bench_runner/cli.js @@ -2,6 +2,7 @@ const { ArrayFrom, + ArrayIsArray, ArrayPrototypeFilter, ArrayPrototypeIncludes, ArrayPrototypeJoin, @@ -32,6 +33,9 @@ const { kBenchmarksStreamDrain, } = require('internal/bench_runner/benchmarks_stream'); const { + configureRunScope, + createRunId, + runInFileScope, runBenchmarks, } = require('internal/bench_runner/harness'); const { deserializeError, serializeError } = require('internal/error_serdes'); @@ -116,6 +120,19 @@ function createChildFileList(patterns, cwd) { return null; } +function createFileScopes(files, options) { + const scopes = []; + for (let i = 0; i < files.length; i++) { + ArrayPrototypePush(scopes, { + __proto__: null, + entryFile: resolve(options.cwd, files[i]), + fileRunId: options.isChild && i === 0 && + options.fileRunId !== undefined ? options.fileRunId : createRunId(), + }); + } + return scopes; +} + function parseNamePattern(value) { if (value.length === 0) return undefined; try { @@ -163,11 +180,15 @@ function parseCommandLine() { __proto__: null, cwd: process.cwd(), destinations, + fileRunId: isChild && process.env.NODE_BENCH_FILE_RUN_ID ? + process.env.NODE_BENCH_FILE_RUN_ID : undefined, isChild, isolation: getOptionValue('--bench-isolation'), namePattern: parseNamePattern(getOptionValue('--bench-name-pattern')), namePatternSource: getOptionValue('--bench-name-pattern'), reporters, + runId: isChild && process.env.NODE_BENCH_RUN_ID ? + process.env.NODE_BENCH_RUN_ID : undefined, samples, warmup, }; @@ -292,7 +313,24 @@ function deserializeRecord(record) { function validateRecord(record) { if (record === null || typeof record !== 'object' || !kEventTypes.has(record.type) || record.data === null || - typeof record.data !== 'object') { + typeof record.data !== 'object' || + typeof record.data.runId !== 'string' || + (record.data.fileRunId !== null && + typeof record.data.fileRunId !== 'string') || + (record.data.entryFile !== null && + typeof record.data.entryFile !== 'string')) { + throw new ERR_INVALID_ARG_VALUE( + 'benchmark child message', record, 'is not a valid benchmark record'); + } + if ((record.type === 'bench:start' || record.type === 'bench:sample' || + record.type === 'bench:complete') && + (typeof record.data.benchId !== 'string' || + (record.data.parentId !== null && + typeof record.data.parentId !== 'string') || + typeof record.data.name !== 'string' || + !ArrayIsArray(record.data.namePath) || + ArrayPrototypeSome( + record.data.namePath, (name) => typeof name !== 'string'))) { throw new ERR_INVALID_ARG_VALUE( 'benchmark child message', record, 'is not a valid benchmark record'); } @@ -345,7 +383,8 @@ async function loadBenchmarkFiles(files, options, modules, onRecord) { for (let i = 0; i < files.length; i++) { const file = resolve(options.cwd, files[i]); try { - await loader.import(pathToFileURL(file), parentURL, kEmptyObject); + await runInFileScope(options.fileScopes[i], () => + loader.import(pathToFileURL(file), parentURL, kEmptyObject)); } catch (error) { loadFailed = true; await onRecord({ @@ -353,6 +392,8 @@ async function loadBenchmarkFiles(files, options, modules, onRecord) { type: 'bench:diagnostic', data: { __proto__: null, + runId: options.runId, + ...options.fileScopes[i], message: error?.message ?? String(error), error, level: 'error', @@ -377,6 +418,10 @@ async function loadBenchmarkFiles(files, options, modules, onRecord) { if (record.type === 'bench:summary') { record.data.file = files.length === 1 ? resolve(options.cwd, files[0]) : null; + record.data.fileRunId = files.length === 1 ? + options.fileScopes[0].fileRunId : null; + record.data.entryFile = files.length === 1 ? + options.fileScopes[0].entryFile : null; summary = record.data; } await onRecord(record); @@ -445,7 +490,7 @@ function getChildArgs(path, options) { return args; } -async function runChild(path, options, onRecord) { +async function runChild(path, options, scope, onRecord) { const child = spawn(process.execPath, getChildArgs(path, options), { __proto__: null, cwd: options.cwd, @@ -453,6 +498,8 @@ async function runChild(path, options, onRecord) { __proto__: null, ...process.env, NODE_BENCH_CONTEXT: 'child', + NODE_BENCH_FILE_RUN_ID: scope.fileRunId, + NODE_BENCH_RUN_ID: options.runId, }, serialization: 'advanced', stdio: ['inherit', 'pipe', 'pipe', 'ipc'], @@ -487,6 +534,8 @@ async function runChild(path, options, onRecord) { type: 'bench:diagnostic', data: { __proto__: null, + runId: options.runId, + ...scope, message, level: 'info', file: path, @@ -506,8 +555,15 @@ async function runChild(path, options, onRecord) { child.on('message', (message) => { if (message?.type !== kChildMessageType) return; try { - const pending = handleRecord( - deserializeRecord(validateRecord(message.record))); + const record = deserializeRecord(validateRecord(message.record)); + record.data.runId = options.runId; + if (record.data.fileRunId !== null) { + record.data.fileRunId = scope.fileRunId; + } + if (record.data.entryFile !== null) { + record.data.entryFile = scope.entryFile; + } + const pending = handleRecord(record); trackPending(pending); } catch (error) { protocolError = error; @@ -533,10 +589,11 @@ async function runIsolated(files, options, output) { for (let i = 0; i < files.length; i++) { const path = files[i]; + const scope = options.fileScopes[i]; let childSummary; let result; try { - result = await runChild(path, options, (record) => { + result = await runChild(path, options, scope, (record) => { if (record.type === 'bench:summary') { childSummary = record.data; return; @@ -547,6 +604,8 @@ async function runIsolated(files, options, output) { success = false; output.diagnostic({ __proto__: null, + runId: options.runId, + ...scope, message: error.message, error, level: 'error', @@ -570,6 +629,8 @@ async function runIsolated(files, options, output) { `exit code ${result.code}` : `signal ${result.signal}`; output.diagnostic({ __proto__: null, + runId: options.runId, + ...scope, message: `Benchmark file '${path}' failed with ${status}`, level: 'error', file: path, @@ -578,8 +639,12 @@ async function runIsolated(files, options, output) { } } + const scope = files.length === 1 ? options.fileScopes[0] : null; const summary = { __proto__: null, + runId: options.runId, + fileRunId: scope?.fileRunId ?? null, + entryFile: scope?.entryFile ?? null, success: success && (process.exitCode ?? 0) === 0, counts, duration_ns: hrtime() - start, @@ -596,6 +661,19 @@ async function run(patterns) { createBenchmarkFileList(patterns, options.cwd); if (files === null) return { __proto__: null, success: false }; + options.runId ??= createRunId(); + options.fileScopes = createFileScopes(files, options); + const scope = files.length === 1 ? options.fileScopes[0] : { + __proto__: null, + entryFile: null, + fileRunId: options.runId, + }; + configureRunScope({ + __proto__: null, + runId: options.runId, + ...scope, + }); + if (options.isChild) { try { const modules = await loadUserImports(options); diff --git a/lib/internal/bench_runner/harness.js b/lib/internal/bench_runner/harness.js index 59b85860c2e9..09c06245c239 100644 --- a/lib/internal/bench_runner/harness.js +++ b/lib/internal/bench_runner/harness.js @@ -6,6 +6,7 @@ const { ArrayPrototypeSlice, BigInt, FunctionPrototypeCall, + JSONStringify, MathCeil, Promise, PromisePrototypeThen, @@ -16,6 +17,7 @@ const { RegExpPrototypeExec, SafeMap, SafePromiseRace, + String, SymbolDispose, } = primordials; const { getCallerLocation } = internalBinding('util'); @@ -59,6 +61,11 @@ const { const { bigint: hrtime } = process.hrtime; const kHookNames = ['after', 'afterEach', 'before', 'beforeEach']; const kIsCliRunner = getOptionValue('--bench'); +let nextRunId = 0; + +function createRunId() { + return `${process.pid}:${String(hrtime())}:${nextRunId++}`; +} function eventLoopTurn() { return new Promise((resolve) => setImmediate(resolve)); @@ -78,6 +85,7 @@ class Harness { #buildPromises = []; #duplicateErrors = new SafeMap(); #explicitRun = false; + #fileScopeStorage = new AsyncLocalStorage(); #hasOnly = false; #runPromise = null; #scheduled = false; @@ -96,7 +104,9 @@ class Harness { yieldBetweenSamples, 'options.yieldBetweenSamples'); this.#autoRun = autoRun; this.#yieldBetweenSamples = yieldBetweenSamples; - this.entryFile = process.argv?.[1]; + this.runId = createRunId(); + this.fileRunId = this.runId; + this.entryFile = process.argv?.[1] ?? null; this.stream = new BenchmarksStream(); this.state = 'collecting'; this.namePattern = null; @@ -122,6 +132,24 @@ class Harness { ); } + getFileScope() { + return this.#fileScopeStorage.getStore() ?? null; + } + + runInFileScope(scope, fn) { + return this.#fileScopeStorage.run(scope, fn); + } + + setRunScope({ entryFile, fileRunId, runId }) { + if (this.state !== 'collecting') { + throw new ERR_INVALID_STATE( + 'benchmark execution scope cannot change after execution has started'); + } + this.entryFile = entryFile; + this.fileRunId = fileRunId; + this.runId = runId; + } + #ensureCollecting() { if (this.state === 'building' && this.#storage.getStore() instanceof Suite) return; @@ -178,6 +206,7 @@ class Harness { const parent = this.#getParent(); ArrayPrototypePush(parent.hooks[name], { __proto__: null, + fileScope: parent.isRoot ? this.getFileScope() : parent.fileScope, fn, loc: getCallerLocation(), }); @@ -312,9 +341,13 @@ class Harness { if (!(node instanceof Bench)) return; this.counts.total++; - const existing = identities.get(node.benchId); + const identity = JSONStringify([ + this.#getRecordScope(node).fileRunId, + node.benchId, + ]); + const existing = identities.get(identity); if (existing === undefined) { - identities.set(node.benchId, node); + identities.set(identity, node); } else { this.#duplicateErrors.set(node, new ERR_INVALID_STATE( `duplicate benchmark identity for "${node.fullName}"`)); @@ -378,13 +411,48 @@ class Harness { name: suite.name, signal: this.outerSignal, }; - await this.#runHooks(suite, name, suite, suite, context); + const hooks = suite.hooks[name]; + for (let i = 0; i < hooks.length; i++) { + try { + await this.#invoke(suite, suite, hooks[i].fn, [context]); + } catch (error) { + return { __proto__: null, error, hook: hooks[i] }; + } + } + return null; + } + + #getRecordScope(node = undefined) { + const scope = node?.fileScope; + if (scope !== null && scope !== undefined) { + return { + __proto__: null, + runId: this.runId, + fileRunId: scope.fileRunId, + entryFile: scope.entryFile, + }; + } + if (node !== undefined && kIsCliRunner) { + return { + __proto__: null, + runId: this.runId, + fileRunId: this.fileRunId, + entryFile: null, + }; + } + return { + __proto__: null, + runId: this.runId, + fileRunId: this.fileRunId, + entryFile: this.entryFile, + }; } - #diagnostic(error, loc, level = 'info') { + #diagnostic(error, loc, level = 'info', node = undefined) { this.success = false; this.stream.diagnostic({ __proto__: null, + ...this.#getRecordScope(node), message: error?.message ?? `${error}`, error, level, @@ -410,7 +478,7 @@ class Harness { async #executeSuite(suite) { if (suite.buildError !== null) { - this.#diagnostic(suite.buildError, suite.loc, 'error'); + this.#diagnostic(suite.buildError, suite.loc, 'error', suite); await this.#completeSubtree(suite, suite.buildError); suite.finished = true; suite.completion.resolve(); @@ -421,11 +489,11 @@ class Harness { const active = this.#suiteHasActiveBench(suite); let beforeError; if (active) { - try { - await this.#runSuiteHooks(suite, 'before'); - } catch (error) { - beforeError = error; - this.#diagnostic(error, suite.loc, 'error'); + const failure = await this.#runSuiteHooks(suite, 'before'); + if (failure !== null) { + beforeError = failure.error; + this.#diagnostic( + failure.error, failure.hook.loc, 'error', failure.hook); } } @@ -443,10 +511,10 @@ class Harness { } if (active) { - try { - await this.#runSuiteHooks(suite, 'after'); - } catch (error) { - this.#diagnostic(error, suite.loc, 'error'); + const failure = await this.#runSuiteHooks(suite, 'after'); + if (failure !== null) { + this.#diagnostic( + failure.error, failure.hook.loc, 'error', failure.hook); } } suite.finished = true; @@ -537,9 +605,11 @@ class Harness { #createResult(benchmark, samples, extra = kEmptyObject) { return { __proto__: null, + ...this.#getRecordScope(benchmark), benchId: benchmark.benchId, parentId: benchmark.parentId, name: benchmark.name, + namePath: ArrayPrototypeSlice(benchmark.namePath), file: benchmark.loc.file, line: benchmark.loc.line, column: benchmark.loc.column, @@ -595,9 +665,11 @@ class Harness { this.stream.start({ __proto__: null, + ...this.#getRecordScope(benchmark), benchId: benchmark.benchId, parentId: benchmark.parentId, name: benchmark.name, + namePath: ArrayPrototypeSlice(benchmark.namePath), file: benchmark.loc.file, line: benchmark.loc.line, column: benchmark.loc.column, @@ -648,9 +720,11 @@ class Harness { ArrayPrototypePush(samples, sample); this.stream.sample({ __proto__: null, + ...this.#getRecordScope(benchmark), benchId: benchmark.benchId, parentId: benchmark.parentId, name: benchmark.name, + namePath: ArrayPrototypeSlice(benchmark.namePath), index: i - warmup, ...sample, }); @@ -697,6 +771,7 @@ class Harness { const duration = startTime === undefined ? 0n : hrtime() - startTime; this.stream.summary({ __proto__: null, + ...this.#getRecordScope(), success: this.success, counts: this.counts, duration_ns: duration, @@ -716,6 +791,7 @@ class Harness { this.state = 'building'; const startTime = hrtime(); await this.#waitForBuild(); + this.#fileScopeStorage.disable(); this.#prepare(); this.state = 'running'; await this.#executeSuite(this.root); @@ -780,6 +856,14 @@ function createRunner(options = kEmptyObject) { }; } +function configureRunScope(scope) { + lazyHarness().setRunScope(scope); +} + +function runInFileScope(scope, fn) { + return lazyHarness().runInFileScope(scope, fn); +} + function runBenchmarks(options, force) { return lazyHarness().run(options, force); } @@ -791,7 +875,10 @@ module.exports = { before: createHook(kHookNames[2], lazyHarness), beforeEach: createHook(kHookNames[3], lazyHarness), bench, + configureRunScope, + createRunId, createRunner, + runInFileScope, runBenchmarks, suite, }; diff --git a/test/fixtures/bench-runner/identity-child-a.cjs b/test/fixtures/bench-runner/identity-child-a.cjs new file mode 100644 index 000000000000..2b8c4b728b1c --- /dev/null +++ b/test/fixtures/bench-runner/identity-child-a.cjs @@ -0,0 +1,11 @@ +'use strict'; + +const { bench } = require('node:bench'); + +module.exports = function declareChildA() { + bench('child a', { samples: 1 }, (b) => { + b.start(); + process.hrtime.bigint(); + b.end(1); + }); +}; diff --git a/test/fixtures/bench-runner/identity-child-b.cjs b/test/fixtures/bench-runner/identity-child-b.cjs new file mode 100644 index 000000000000..249bcdcfc4f1 --- /dev/null +++ b/test/fixtures/bench-runner/identity-child-b.cjs @@ -0,0 +1,11 @@ +'use strict'; + +const { bench } = require('node:bench'); + +module.exports = function declareChildB() { + bench('child b', { samples: 1 }, (b) => { + b.start(); + process.hrtime.bigint(); + b.end(1); + }); +}; diff --git a/test/fixtures/bench-runner/identity-entry-a.cjs b/test/fixtures/bench-runner/identity-entry-a.cjs new file mode 100644 index 000000000000..2892ea3f3eca --- /dev/null +++ b/test/fixtures/bench-runner/identity-entry-a.cjs @@ -0,0 +1,10 @@ +'use strict'; + +const { suite } = require('node:bench'); +const { setImmediate } = require('timers/promises'); +const registerSharedIdentity = require('./identity-shared.cjs'); + +suite('shared suite', async () => { + await setImmediate(); + registerSharedIdentity(); +}); diff --git a/test/fixtures/bench-runner/identity-entry-b.cjs b/test/fixtures/bench-runner/identity-entry-b.cjs new file mode 100644 index 000000000000..2892ea3f3eca --- /dev/null +++ b/test/fixtures/bench-runner/identity-entry-b.cjs @@ -0,0 +1,10 @@ +'use strict'; + +const { suite } = require('node:bench'); +const { setImmediate } = require('timers/promises'); +const registerSharedIdentity = require('./identity-shared.cjs'); + +suite('shared suite', async () => { + await setImmediate(); + registerSharedIdentity(); +}); diff --git a/test/fixtures/bench-runner/identity-hook.cjs b/test/fixtures/bench-runner/identity-hook.cjs new file mode 100644 index 000000000000..14c4cacd9cdc --- /dev/null +++ b/test/fixtures/bench-runner/identity-hook.cjs @@ -0,0 +1,9 @@ +'use strict'; + +const { before, bench } = require('node:bench'); + +before(() => { + throw new Error('scoped hook failed'); +}); + +bench('scoped hook benchmark', { samples: 1 }, () => {}); diff --git a/test/fixtures/bench-runner/identity-preload.cjs b/test/fixtures/bench-runner/identity-preload.cjs new file mode 100644 index 000000000000..ed55e627527b --- /dev/null +++ b/test/fixtures/bench-runner/identity-preload.cjs @@ -0,0 +1,9 @@ +'use strict'; + +const { bench } = require('node:bench'); + +bench('preload identity', { samples: 1 }, (b) => { + b.start(); + process.hrtime.bigint(); + b.end(1); +}); diff --git a/test/fixtures/bench-runner/identity-shared.cjs b/test/fixtures/bench-runner/identity-shared.cjs new file mode 100644 index 000000000000..40ce0f27be29 --- /dev/null +++ b/test/fixtures/bench-runner/identity-shared.cjs @@ -0,0 +1,11 @@ +'use strict'; + +const { bench } = require('node:bench'); + +module.exports = function registerSharedIdentity() { + bench('shared identity', { samples: 1 }, (b) => { + b.start(); + process.hrtime.bigint(); + b.end(1); + }); +}; diff --git a/test/fixtures/bench-runner/identity-suite.cjs b/test/fixtures/bench-runner/identity-suite.cjs new file mode 100644 index 000000000000..53c5982ce1a9 --- /dev/null +++ b/test/fixtures/bench-runner/identity-suite.cjs @@ -0,0 +1,10 @@ +'use strict'; + +const { suite } = require('node:bench'); +const declareChildA = require('./identity-child-a.cjs'); +const declareChildB = require('./identity-child-b.cjs'); + +suite('cross-module suite', () => { + declareChildA(); + declareChildB(); +}); diff --git a/test/fixtures/bench-runner/malformed-record.cjs b/test/fixtures/bench-runner/malformed-record.cjs index 5744bf1ea4b0..ab8b4bb84dc1 100644 --- a/test/fixtures/bench-runner/malformed-record.cjs +++ b/test/fixtures/bench-runner/malformed-record.cjs @@ -2,13 +2,24 @@ const common = require('../../common'); -const record = process.env.NODE_BENCH_MALFORMED_RECORD === 'summary' ? { +const kind = process.env.NODE_BENCH_MALFORMED_RECORD; +const record = kind === 'summary' ? { type: 'bench:summary', data: { counts: { completed: 0, failed: 0, skipped: 0, total: -1 }, duration_ns: 1n, + entryFile: __filename, + fileRunId: process.env.NODE_BENCH_FILE_RUN_ID, + runId: process.env.NODE_BENCH_RUN_ID, success: true, }, +} : kind === 'identity' ? { + type: 'bench:complete', + data: { + entryFile: __filename, + fileRunId: process.env.NODE_BENCH_FILE_RUN_ID, + runId: process.env.NODE_BENCH_RUN_ID, + }, } : null; process.send?.({ type: 'node:bench:record', record }); diff --git a/test/parallel/test-bench-cli.js b/test/parallel/test-bench-cli.js index 28dcc8b2dbb7..8130bf38dc9f 100644 --- a/test/parallel/test-bench-cli.js +++ b/test/parallel/test-bench-cli.js @@ -121,6 +121,106 @@ for (const { patterns, message } of [ }); } +for (const isolation of ['process', 'none']) { + const result = spawnBench([ + `--bench-isolation=${isolation}`, + '--bench-reporter=json', + fixtures.path('bench-runner/identity-entry-*.cjs'), + ]); + assert.strictEqual(result.status, 0); + const records = parseRecords(result); + const completions = records.filter( + ({ type }) => type === 'bench:complete').map(({ data }) => data); + const summary = records.at(-1).data; + + assert.strictEqual(completions.length, 2); + assert.strictEqual(completions[0].benchId, completions[1].benchId); + assert.strictEqual(completions[0].runId, completions[1].runId); + assert.strictEqual(completions[0].runId, summary.runId); + assert.notStrictEqual( + completions[0].fileRunId, completions[1].fileRunId); + assert.deepStrictEqual(completions.map(({ entryFile }) => entryFile), [ + fixtures.path('bench-runner/identity-entry-a.cjs'), + fixtures.path('bench-runner/identity-entry-b.cjs'), + ]); + assert.deepStrictEqual(completions.map(({ namePath }) => namePath), [ + ['shared suite', 'shared identity'], + ['shared suite', 'shared identity'], + ]); + assert.strictEqual(summary.fileRunId, null); + assert.strictEqual(summary.entryFile, null); +} + +{ + const result = spawnBench([ + '--bench-reporter=json', + fixtures.path('bench-runner/identity-suite.cjs'), + ]); + assert.strictEqual(result.status, 0); + const records = parseRecords(result); + const completions = records.filter( + ({ type }) => type === 'bench:complete').map(({ data }) => data); + const parentId = JSON.stringify([ + fixtures.path('bench-runner/identity-suite.cjs'), + ['cross-module suite'], + ]); + + assert.deepStrictEqual(completions.map(({ name }) => name), [ + 'child a', + 'child b', + ]); + assert(completions.every((completion) => + completion.parentId === parentId)); + assert.deepStrictEqual(completions.map(({ namePath }) => namePath), [ + ['cross-module suite', 'child a'], + ['cross-module suite', 'child b'], + ]); + assert(completions.every(({ entryFile }) => + entryFile === fixtures.path('bench-runner/identity-suite.cjs'))); +} + +for (const isolation of ['process', 'none']) { + const result = spawnBench([ + '--require', fixtures.path('bench-runner/identity-preload.cjs'), + `--bench-isolation=${isolation}`, + '--bench-reporter=json', + fixtures.path('bench-runner/identity-entry-*.cjs'), + ]); + assert.strictEqual(result.status, 0); + const records = parseRecords(result); + const preloads = records.filter( + ({ type, data }) => type === 'bench:complete' && + data.name === 'preload identity').map(({ data }) => data); + assert.strictEqual(preloads.length, isolation === 'process' ? 2 : 1); + assert.strictEqual( + new Set(preloads.map(({ fileRunId }) => fileRunId)).size, + preloads.length, + ); + assert(preloads.every(({ entryFile }) => entryFile === null)); + assert(preloads.every( + ({ runId }) => runId === records.at(-1).data.runId)); +} + +{ + const result = spawnBench([ + '--bench-isolation=none', + '--bench-reporter=json', + fixtures.path('bench-runner/a.cjs'), + fixtures.path('bench-runner/identity-hook.cjs'), + ]); + assert.strictEqual(result.status, 1); + const records = parseRecords(result); + const diagnostic = records.find( + ({ type, data }) => type === 'bench:diagnostic' && + data.message === 'scoped hook failed').data; + const completion = records.find( + ({ type, data }) => type === 'bench:complete' && + data.name === 'scoped hook benchmark').data; + assert.strictEqual(diagnostic.entryFile, + fixtures.path('bench-runner/identity-hook.cjs')); + assert.strictEqual(diagnostic.fileRunId, completion.fileRunId); +} + { const result = spawnBench([ '--bench-reporter=json', @@ -297,6 +397,7 @@ for (const isolation of ['process', 'none']) { for (const { kind, message } of [ { kind: 'record', message: /not a valid benchmark record/ }, + { kind: 'identity', message: /not a valid benchmark record/ }, { kind: 'summary', message: /not a valid benchmark summary/ }, ]) { const result = spawnBench([ diff --git a/test/parallel/test-bench-create-runner.js b/test/parallel/test-bench-create-runner.js index a7194d002d55..5540a189ad52 100644 --- a/test/parallel/test-bench-create-runner.js +++ b/test/parallel/test-bench-create-runner.js @@ -54,6 +54,16 @@ const { setImmediate } = require('timers/promises'); assert.strictEqual(secondResult.samples.length, 1); assert.strictEqual(firstResult.error, undefined); assert.strictEqual(secondResult.error, undefined); + assert.strictEqual(firstResult.benchId, secondResult.benchId); + assert.notStrictEqual(firstResult.runId, secondResult.runId); + assert.strictEqual(firstResult.fileRunId, firstResult.runId); + assert.strictEqual(secondResult.fileRunId, secondResult.runId); + assert.strictEqual(firstResult.entryFile, process.argv[1]); + assert.strictEqual(secondResult.entryFile, process.argv[1]); + assert.deepStrictEqual(firstResult.namePath, ['same name']); + assert.deepStrictEqual(secondResult.namePath, ['same name']); + assert(firstRecords.every(({ data }) => data.runId === firstResult.runId)); + assert(secondRecords.every(({ data }) => data.runId === secondResult.runId)); assert.strictEqual( firstRecords.filter(({ type }) => type === 'bench:summary').length, 1); assert.strictEqual( From 9c4e755b2e5e038d925f8f6a9aaca738513d5d29 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Fri, 28 Aug 2026 21:01:21 +0000 Subject: [PATCH 02/14] lib: improve node:bench stream handling Signed-off-by: James M Snell Assisted-by: Opencode --- doc/api/bench.md | 24 ++ .../bench_runner/benchmarks_stream.js | 247 +++++++++++++-- lib/internal/bench_runner/cli.js | 128 ++++++-- lib/internal/bench_runner/harness.js | 179 ++++++++--- lib/internal/error_serdes.js | 10 +- .../bench-runner/acknowledged-records.mjs | 41 +++ .../bench-runner/malformed-record.cjs | 3 +- test/parallel/test-bench-cli.js | 17 + test/parallel/test-bench-stream.js | 292 ++++++++++++++++++ test/sequential/test-error-serdes.js | 4 + 10 files changed, 846 insertions(+), 99 deletions(-) create mode 100644 test/fixtures/bench-runner/acknowledged-records.mjs create mode 100644 test/parallel/test-bench-stream.js diff --git a/doc/api/bench.md b/doc/api/bench.md index 9c24d04ee4f0..00faab4a8d2b 100644 --- a/doc/api/bench.md +++ b/doc/api/bench.md @@ -553,6 +553,30 @@ The events are emitted in execution order: * `'bench:diagnostic'` * `'bench:summary'` +Named event payloads, readable records, and benchmark completion values are +independent snapshots. Mutating a value received through one delivery mechanism +does not change values received through the others. As with other +{EventEmitter} events, multiple listeners for the same named event receive the +same event payload. Memory referenced through a {SharedArrayBuffer} remains +shared, following structured clone semantics. + +Once a consumer starts reading, the runner honors the stream's object-mode +high-water mark and waits between records when the consumer is slower than the +producer. These waits occur after sample timing has ended, and records are not +dropped. Snapshot creation and delivery waits are excluded from benchmark +timeout accounting. Before readable consumption starts, records accumulate in +the standard readable buffer and are included in `readableLength`. This keeps an +unread stream and a consumer using only named events from deadlocking, but the +buffer can grow without bound. A named-event-only consumer that does not need +readable records should call `stream.resume()` to discard them. Destroying the +stream stops readable delivery but does not cancel benchmark execution, so +benchmark completion promises still settle. Automatically scheduled +module-level runs drain their stream internally. + +With process isolation, each record sent by a child is acknowledged only after +the parent has accepted it. A child sends no additional record until it receives +that acknowledgement, bounding the IPC relay when a reporter is slow. + Every benchmark-scoped event contains `runId`, `fileRunId`, `entryFile`, `benchId`, `parentId`, and `namePath`. `runId` and `fileRunId` are opaque and change between runs. `entryFile` identifies the top-level benchmark file whose diff --git a/lib/internal/bench_runner/benchmarks_stream.js b/lib/internal/bench_runner/benchmarks_stream.js index 21d34896608b..6d656e0ffdd4 100644 --- a/lib/internal/bench_runner/benchmarks_stream.js +++ b/lib/internal/bench_runner/benchmarks_stream.js @@ -1,18 +1,201 @@ 'use strict'; const { + ArrayFrom, + ArrayIsArray, ArrayPrototypePush, - ArrayPrototypeShift, + MapPrototypeClear, + MapPrototypeEntries, + MapPrototypeSet, + ObjectDefineProperty, + ObjectGetOwnPropertyDescriptor, + ObjectGetOwnPropertyNames, + ObjectGetPrototypeOf, + ObjectKeys, + ObjectPrototype, + ObjectPrototypeHasOwnProperty, + ObjectSetPrototypeOf, + PromiseReject, + PromiseResolve, + PromiseWithResolvers, + SafeMap, + SetPrototypeAdd, + SetPrototypeClear, + SetPrototypeValues, Symbol, } = primordials; const Readable = require('internal/streams/readable'); +const { deserializeError, serializeError } = require('internal/error_serdes'); +const { + codes: { + ERR_INVALID_STATE, + }, +} = require('internal/errors'); +const { isError } = require('internal/util'); +const { isMap, isSet } = require('internal/util/types'); +const { structuredClone } = require('internal/worker/js_transferable'); const kEmitMessage = Symbol('kEmitMessage'); -const kBenchmarksStreamDrain = Symbol('kBenchmarksStreamDrain'); + +function repairError(source, clone, seen) { + const serialized = deserializeError(serializeError(source)); + let sourceName; + try { + sourceName = source.name; + } catch { + // The serialized form already omits properties whose getters throw. + } + let repaired = clone; + if (!isError(repaired) || + (sourceName !== undefined && repaired.name !== sourceName)) { + repaired = serialized; + } + if (repaired === null || typeof repaired !== 'object') return repaired; + seen.set(source, repaired); + + if (serialized !== null && typeof serialized === 'object') { + const serializedKeys = ObjectGetOwnPropertyNames(serialized); + for (let i = 0; i < serializedKeys.length; i++) { + const key = serializedKeys[i]; + if (ObjectPrototypeHasOwnProperty(repaired, key)) continue; + const descriptor = ObjectGetOwnPropertyDescriptor(serialized, key); + ObjectSetPrototypeOf(descriptor, null); + ObjectDefineProperty(repaired, key, descriptor); + } + } + + const keys = ObjectGetOwnPropertyNames(source); + for (let i = 0; i < keys.length; i++) { + const key = keys[i]; + const descriptor = ObjectGetOwnPropertyDescriptor(source, key); + if (!ObjectPrototypeHasOwnProperty(descriptor, 'value') || + typeof descriptor.value === 'function' || + typeof descriptor.value === 'symbol') { + continue; + } + const existing = ObjectGetOwnPropertyDescriptor(repaired, key); + const value = descriptor.value !== null && + typeof descriptor.value === 'object' ? + repairClone(descriptor.value, existing?.value, seen) : descriptor.value; + if ((existing !== undefined && existing.value === value) || + existing?.configurable === false) { + continue; + } + ObjectDefineProperty(repaired, key, { + __proto__: null, + configurable: descriptor.configurable, + enumerable: descriptor.enumerable, + value, + writable: descriptor.writable, + }); + } + return repaired; +} + +function repairClone(source, clone, seen) { + if (source === null || (typeof source !== 'object' && + typeof source !== 'function')) { + return clone === undefined ? source : clone; + } + if (typeof source === 'function') return clone; + if (seen.has(source)) return seen.get(source); + if (isError(source)) return repairError(source, clone, seen); + if (clone === null || typeof clone !== 'object') { + try { + clone = structuredClone(source); + } catch { + const prototype = ObjectGetPrototypeOf(source); + if (!ArrayIsArray(source) && prototype !== null && + prototype !== ObjectPrototype) { + return clone; + } + clone = ArrayIsArray(source) ? [] : { __proto__: prototype }; + } + } + seen.set(source, clone); + if (isMap(source) && isMap(clone)) { + const sourceEntries = ArrayFrom(MapPrototypeEntries(source)); + const cloneEntries = ArrayFrom(MapPrototypeEntries(clone)); + const repairedEntries = []; + for (let i = 0; i < sourceEntries.length; i++) { + ArrayPrototypePush(repairedEntries, [ + repairClone(sourceEntries[i][0], cloneEntries[i][0], seen), + repairClone(sourceEntries[i][1], cloneEntries[i][1], seen), + ]); + } + MapPrototypeClear(clone); + for (let i = 0; i < repairedEntries.length; i++) { + MapPrototypeSet(clone, repairedEntries[i][0], repairedEntries[i][1]); + } + return clone; + } + if (isSet(source) && isSet(clone)) { + const sourceValues = ArrayFrom(SetPrototypeValues(source)); + const cloneValues = ArrayFrom(SetPrototypeValues(clone)); + const repairedValues = []; + for (let i = 0; i < sourceValues.length; i++) { + ArrayPrototypePush( + repairedValues, repairClone(sourceValues[i], cloneValues[i], seen)); + } + SetPrototypeClear(clone); + for (let i = 0; i < repairedValues.length; i++) { + SetPrototypeAdd(clone, repairedValues[i]); + } + return clone; + } + const prototype = ObjectGetPrototypeOf(source); + if (prototype === null) ObjectSetPrototypeOf(clone, null); + if (prototype !== null && prototype !== ObjectPrototype && + !ArrayIsArray(source)) { + return clone; + } + const keys = ObjectKeys(source); + for (let i = 0; i < keys.length; i++) { + const key = keys[i]; + const descriptor = ObjectGetOwnPropertyDescriptor(source, key); + if (!ObjectPrototypeHasOwnProperty(descriptor, 'value')) continue; + const existing = ObjectGetOwnPropertyDescriptor(clone, key); + const value = repairClone(descriptor.value, existing?.value, seen); + if ((existing !== undefined && existing.value === value) || + existing?.configurable === false) { + continue; + } + ObjectDefineProperty(clone, key, { + __proto__: null, + configurable: descriptor.configurable, + enumerable: descriptor.enumerable, + value, + writable: descriptor.writable, + }); + } + if (ArrayIsArray(source)) { + const descriptor = ObjectGetOwnPropertyDescriptor(source, 'length'); + ObjectSetPrototypeOf(descriptor, null); + ObjectDefineProperty(clone, 'length', descriptor); + } + return clone; +} + +function cloneRecordData(data) { + let clone; + try { + clone = structuredClone(data); + } catch (error) { + if (data.error === undefined) throw error; + clone = structuredClone({ __proto__: null, ...data, error: undefined }); + clone.error = deserializeError(serializeError(data.error)); + } + try { + return repairClone(data, clone, new SafeMap()); + } catch { + return clone; + } +} class BenchmarksStream extends Readable { - #buffer = []; - #canPush = true; + #blocked = false; + #drainWaiters = []; + #hasReader = false; constructor() { super({ @@ -22,13 +205,37 @@ class BenchmarksStream extends Readable { } _read() { - const wasBlocked = !this.#canPush; - this.#canPush = true; - while (this.#buffer.length > 0) { - const record = ArrayPrototypeShift(this.#buffer); - if (!this.#tryPush(record)) return; + if (this.#blocked) { + this.#blocked = false; + const waiters = this.#drainWaiters; + this.#drainWaiters = []; + for (let i = 0; i < waiters.length; i++) waiters[i].resolve(); + } + } + + read(size) { + if (size !== 0) this.#hasReader = true; + return super.read(size); + } + + _destroy(error, callback) { + const failure = error ?? + new ERR_INVALID_STATE('benchmark stream is closed'); + const waiters = this.#drainWaiters; + this.#drainWaiters = []; + for (let i = 0; i < waiters.length; i++) waiters[i].reject(failure); + callback(error); + } + + waitForDrain() { + if (this.destroyed) { + return PromiseReject(this.errored ?? + new ERR_INVALID_STATE('benchmark stream is closed')); } - if (wasBlocked) this.emit(kBenchmarksStreamDrain); + if (!this.#blocked) return PromiseResolve(); + const waiter = PromiseWithResolvers(); + ArrayPrototypePush(this.#drainWaiters, waiter); + return waiter.promise; } start(data) { @@ -56,21 +263,25 @@ class BenchmarksStream extends Readable { } [kEmitMessage](type, data) { - this.emit(type, data); - return this.#tryPush({ type, data }); + const recordData = cloneRecordData(data); + const record = { __proto__: null, type, data: recordData }; + if (this.listenerCount(type) > 0) { + this.emit(type, cloneRecordData(recordData)); + } + return this.#tryPush(record); } #tryPush(record) { - if (this.#canPush) { - this.#canPush = this.push(record); - } else { - ArrayPrototypePush(this.#buffer, record); + if (this.destroyed) return false; + const canPush = this.push(record); + if (record !== null && !canPush && this.#hasReader) { + this.#blocked = true; + return false; } - return this.#canPush; + return true; } } module.exports = { BenchmarksStream, - kBenchmarksStreamDrain, }; diff --git a/lib/internal/bench_runner/cli.js b/lib/internal/bench_runner/cli.js index 6c717aab6fff..ae33694818b5 100644 --- a/lib/internal/bench_runner/cli.js +++ b/lib/internal/bench_runner/cli.js @@ -16,6 +16,7 @@ const { PromisePrototypeThen, PromiseReject, PromiseResolve, + PromiseWithResolvers, RegExp, SafeMap, SafePromiseAllReturnVoid, @@ -30,7 +31,6 @@ const { createWriteStream, statSync } = require('fs'); const { Glob } = require('internal/fs/glob'); const { BenchmarksStream, - kBenchmarksStreamDrain, } = require('internal/bench_runner/benchmarks_stream'); const { configureRunScope, @@ -65,6 +65,7 @@ const kBuiltinReporters = new SafeMap([ ['json', 'internal/bench_runner/reporter/json'], ['spec', 'internal/bench_runner/reporter/spec'], ]); +const kChildAckMessageType = 'node:bench:ack'; const kChildMessageType = 'node:bench:record'; const kEventTypes = new SafeSet([ 'bench:start', @@ -281,7 +282,7 @@ function emitRecordAndWait(stream, record) { return PromiseReject(stream.errored ?? new ERR_INVALID_STATE('benchmark output stream is closed')); } - return once(stream, kBenchmarksStreamDrain); + return stream.waitForDrain(); } function serializeRecord(record) { @@ -350,20 +351,66 @@ function validateRecord(record) { return record; } +let nextChildRecordId = 0; +let listeningForAcks = false; +const pendingChildRecordAcks = new SafeMap(); + +function listenForAcks() { + if (listeningForAcks) return; + listeningForAcks = true; + process.on('message', (message) => { + if (message?.type !== kChildAckMessageType) return; + const pending = pendingChildRecordAcks.get(message.id); + if (pending === undefined) return; + pendingChildRecordAcks.delete(message.id); + pending.resolve(); + }); + process.once('disconnect', () => { + const error = new ERR_INVALID_STATE( + 'benchmark IPC channel closed before acknowledging records'); + for (const pending of pendingChildRecordAcks.values()) { + pending.reject(error); + } + pendingChildRecordAcks.clear(); + }); +} + function sendRecord(record) { + listenForAcks(); + const id = nextChildRecordId++; + const acknowledged = PromiseWithResolvers(); + pendingChildRecordAcks.set(id, acknowledged); + try { + process.send({ + __proto__: null, + id, + type: kChildMessageType, + record: serializeRecord(record), + }, undefined, undefined, (error) => { + if (error) { + pendingChildRecordAcks.delete(id); + acknowledged.reject(error); + } + }); + } catch (error) { + pendingChildRecordAcks.delete(id); + acknowledged.reject(error); + } + return acknowledged.promise; +} + +function sendAck(child, id) { return new Promise((resolve, reject) => { - try { - process.send({ - __proto__: null, - type: kChildMessageType, - record: serializeRecord(record), - }, undefined, undefined, (error) => { - if (error) reject(error); - else resolve(); - }); - } catch (error) { - reject(error); + if (!child.connected) { + reject(new ERR_INVALID_STATE( + 'benchmark child disconnected before acknowledgement')); + return; } + child.send({ __proto__: null, id, type: kChildAckMessageType }, + undefined, undefined, (error) => { + if (error) reject(error); + else resolve(); + }); }); } @@ -505,6 +552,7 @@ async function runChild(path, options, scope, onRecord) { stdio: ['inherit', 'pipe', 'pipe', 'ipc'], }); let protocolError; + let recordPending = false; const pendingRecords = new SafeSet(); const handleRecord = (record) => { if (protocolError !== undefined) return; @@ -555,6 +603,12 @@ async function runChild(path, options, scope, onRecord) { child.on('message', (message) => { if (message?.type !== kChildMessageType) return; try { + if (!NumberIsSafeInteger(message.id) || message.id < 0 || recordPending) { + throw new ERR_INVALID_ARG_VALUE( + 'benchmark child message', message, + 'does not have a valid record sequence'); + } + recordPending = true; const record = deserializeRecord(validateRecord(message.record)); record.data.runId = options.runId; if (record.data.fileRunId !== null) { @@ -564,7 +618,13 @@ async function runChild(path, options, scope, onRecord) { record.data.entryFile = scope.entryFile; } const pending = handleRecord(record); - trackPending(pending); + if (protocolError === undefined) { + const acknowledged = PromisePrototypeThen( + PromiseResolve(pending), () => sendAck(child, message.id)); + trackPending(PromisePrototypeThen(acknowledged, () => { + recordPending = false; + })); + } } catch (error) { protocolError = error; child.kill(); @@ -602,14 +662,18 @@ async function runIsolated(files, options, output) { }); } catch (error) { success = false; - output.diagnostic({ + await emitRecordAndWait(output, { __proto__: null, - runId: options.runId, - ...scope, - message: error.message, - error, - level: 'error', - file: path, + type: 'bench:diagnostic', + data: { + __proto__: null, + runId: options.runId, + ...scope, + message: error.message, + error, + level: 'error', + file: path, + }, }); continue; } @@ -627,13 +691,17 @@ async function runIsolated(files, options, output) { if (childSummary === undefined || childSummary.success) { const status = result.signal === null ? `exit code ${result.code}` : `signal ${result.signal}`; - output.diagnostic({ + await emitRecordAndWait(output, { __proto__: null, - runId: options.runId, - ...scope, - message: `Benchmark file '${path}' failed with ${status}`, - level: 'error', - file: path, + type: 'bench:diagnostic', + data: { + __proto__: null, + runId: options.runId, + ...scope, + message: `Benchmark file '${path}' failed with ${status}`, + level: 'error', + file: path, + }, }); } } @@ -650,7 +718,11 @@ async function runIsolated(files, options, output) { duration_ns: hrtime() - start, file: files.length === 1 ? resolve(options.cwd, files[0]) : null, }; - output.summary(summary); + await emitRecordAndWait(output, { + __proto__: null, + type: 'bench:summary', + data: summary, + }); return summary; } diff --git a/lib/internal/bench_runner/harness.js b/lib/internal/bench_runner/harness.js index 09c06245c239..fa0a2146d53d 100644 --- a/lib/internal/bench_runner/harness.js +++ b/lib/internal/bench_runner/harness.js @@ -8,6 +8,7 @@ const { FunctionPrototypeCall, JSONStringify, MathCeil, + Number, Promise, PromisePrototypeThen, PromiseResolve, @@ -311,15 +312,50 @@ class Harness { this.#scheduled = true; queueMicrotask(() => { if (this.#runPromise === null) { - this.#runPromise = this.#execute(); - PromisePrototypeThen(this.#runPromise, undefined, (error) => { - this.#diagnostic(error, undefined, 'error'); - this.#finish(); - }); + if (!this.#explicitRun) this.stream.resume(); + this.#runPromise = PromisePrototypeThen( + this.#execute(), undefined, (error) => this.#recover(error)); } }); } + async #recover(error) { + this.#settleSubtree(this.root, error); + if (this.state !== 'finished') { + try { + await this.#diagnostic(error, undefined, 'error'); + } catch { + // The stream can fail while reporting the original error. + } + } + try { + await this.#finish(); + } catch { + // Stream failure has already been reported to its consumer. + } + } + + #settleSubtree(node, error) { + for (let i = 0; i < node.children.length; i++) { + const child = node.children[i]; + if (child.finished) continue; + if (child instanceof Suite) { + this.#settleSubtree(child, error); + child.finished = true; + child.completion.resolve(); + } else { + this.success = false; + this.counts.failed++; + const result = this.#createResult( + child, [], { __proto__: null, error }); + child.finished = true; + child.result = result; + child.completion.resolve(result); + } + child.emitDestroy(); + } + } + async #waitForBuild() { for (let i = 0; i < this.#buildPromises.length; i++) { await this.#buildPromises[i]; @@ -448,9 +484,18 @@ class Harness { }; } - #diagnostic(error, loc, level = 'info', node = undefined) { + async #waitForStream(canContinue) { + if (canContinue) return; + try { + await this.stream.waitForDrain(); + } catch (error) { + if (!this.stream.destroyed) throw error; + } + } + + async #diagnostic(error, loc, level = 'info', node = undefined) { this.success = false; - this.stream.diagnostic({ + await this.#waitForStream(this.stream.diagnostic({ __proto__: null, ...this.#getRecordScope(node), message: error?.message ?? `${error}`, @@ -459,7 +504,7 @@ class Harness { file: loc?.file ?? loc?.[2], line: loc?.line ?? loc?.[0], column: loc?.column ?? loc?.[1], - }); + })); } async #completeSubtree(node, error) { @@ -478,7 +523,7 @@ class Harness { async #executeSuite(suite) { if (suite.buildError !== null) { - this.#diagnostic(suite.buildError, suite.loc, 'error', suite); + await this.#diagnostic(suite.buildError, suite.loc, 'error', suite); await this.#completeSubtree(suite, suite.buildError); suite.finished = true; suite.completion.resolve(); @@ -492,7 +537,7 @@ class Harness { const failure = await this.#runSuiteHooks(suite, 'before'); if (failure !== null) { beforeError = failure.error; - this.#diagnostic( + await this.#diagnostic( failure.error, failure.hook.loc, 'error', failure.hook); } } @@ -513,7 +558,7 @@ class Harness { if (active) { const failure = await this.#runSuiteHooks(suite, 'after'); if (failure !== null) { - this.#diagnostic( + await this.#diagnostic( failure.error, failure.hook.loc, 'error', failure.hook); } } @@ -540,7 +585,7 @@ class Harness { } } - async #runWithStop(benchmark, controller, callback) { + async #runWithStop(benchmark, controller, callback, deadline) { const signals = []; if (this.outerSignal !== undefined) { ArrayPrototypePush(signals, this.outerSignal); @@ -569,15 +614,31 @@ class Harness { stop.reject(error); })); } - if (benchmark.timeout !== Infinity) { + const armTimer = () => { + let remaining = deadline.value - hrtime(); + if (remaining < 0n) remaining = 0n; timer = setTimeout(() => { const error = createTimeoutError(benchmark); controller.abort(error); stop.reject(error); - }, benchmark.timeout); - } + }, Number(remaining) / 1e6); + }; + if (deadline !== null) armTimer(); + + const pause = async (work) => { + if (deadline === null) return work(); + clearTimeout(timer); + timer = undefined; + const start = hrtime(); + try { + return await work(); + } finally { + deadline.value += hrtime() - start; + if (!controller.signal.aborted) armTimer(); + } + }; - const work = callback(); + const work = callback(pause); try { if (signals.length === 0 && timer === undefined) return await work; return await SafePromiseRace([work, stop.promise]); @@ -620,12 +681,17 @@ class Harness { }; } - #recordResult(benchmark, result) { + async #recordResult(benchmark, result) { benchmark.finished = true; benchmark.result = result; - this.stream.complete(result); - benchmark.completion.resolve(result); - benchmark.emitDestroy(); + let canContinue; + try { + canContinue = this.stream.complete(result); + } finally { + benchmark.completion.resolve(result); + benchmark.emitDestroy(); + } + await this.#waitForStream(canContinue); } async #executeBench(benchmark, forcedError = undefined) { @@ -633,7 +699,7 @@ class Harness { if (duplicateError !== undefined) { this.success = false; this.counts.failed++; - this.#recordResult(benchmark, this.#createResult( + await this.#recordResult(benchmark, this.#createResult( benchmark, [], { __proto__: null, error: duplicateError }, @@ -644,7 +710,7 @@ class Harness { const skip = this.#getSkip(benchmark); if (skip !== null) { this.counts.skipped++; - this.#recordResult(benchmark, this.#createResult( + await this.#recordResult(benchmark, this.#createResult( benchmark, [], { __proto__: null, skip }, @@ -655,7 +721,7 @@ class Harness { if (forcedError !== undefined) { this.success = false; this.counts.failed++; - this.#recordResult(benchmark, this.#createResult( + await this.#recordResult(benchmark, this.#createResult( benchmark, [], { __proto__: null, error: forcedError }, @@ -663,7 +729,7 @@ class Harness { return; } - this.stream.start({ + await this.#waitForStream(this.stream.start({ __proto__: null, ...this.#getRecordScope(benchmark), benchId: benchmark.benchId, @@ -675,13 +741,15 @@ class Harness { column: benchmark.loc.column, tags: ArrayPrototypeSlice(benchmark.tags), params: benchmark.params, - }); + })); const controller = new AbortController(); - const deadline = benchmark.timeout === Infinity ? - null : hrtime() + BigInt(MathCeil(benchmark.timeout * 1e6)); + const deadline = benchmark.timeout === Infinity ? null : { + __proto__: null, + value: hrtime() + BigInt(MathCeil(benchmark.timeout * 1e6)), + }; const checkDeadline = () => { - if (deadline !== null && hrtime() >= deadline) { + if (deadline !== null && hrtime() >= deadline.value) { const timeoutError = createTimeoutError(benchmark); controller.abort(timeoutError); throw timeoutError; @@ -697,7 +765,7 @@ class Harness { let error; try { - await this.#runWithStop(benchmark, controller, async () => { + await this.#runWithStop(benchmark, controller, async (pause) => { try { await this.#runBenchHooks( benchmark, 'beforeEach', hookContext); @@ -718,7 +786,7 @@ class Harness { } if (i >= warmup) { ArrayPrototypePush(samples, sample); - this.stream.sample({ + await pause(() => this.#waitForStream(this.stream.sample({ __proto__: null, ...this.#getRecordScope(benchmark), benchId: benchmark.benchId, @@ -727,7 +795,7 @@ class Harness { namePath: ArrayPrototypeSlice(benchmark.namePath), index: i - warmup, ...sample, - }); + }))); } if (done) break; if (i + 1 < total && this.#yieldBetweenSamples) { @@ -739,7 +807,7 @@ class Harness { benchmark, 'afterEach', hookContext); checkDeadline(); } - }); + }, deadline); } catch (cause) { error = cause; } finally { @@ -749,7 +817,7 @@ class Harness { if (error !== undefined) { this.success = false; this.counts.failed++; - this.#recordResult(benchmark, this.#createResult( + await this.#recordResult(benchmark, this.#createResult( benchmark, samples, { __proto__: null, error }, @@ -758,32 +826,41 @@ class Harness { } this.counts.completed++; - this.#recordResult(benchmark, this.#createResult( + await this.#recordResult(benchmark, this.#createResult( benchmark, samples, { __proto__: null, summary: summarizeSamples(samples) }, )); } - #finish(startTime) { + async #finish(startTime) { if (this.state === 'finished') return; this.state = 'finished'; const duration = startTime === undefined ? 0n : hrtime() - startTime; - this.stream.summary({ - __proto__: null, - ...this.#getRecordScope(), - success: this.success, - counts: this.counts, - duration_ns: duration, - file: this.entryFile, - }); - this.stream.end(); - this.root.finished = true; - this.root.completion.resolve(); - this.root.emitDestroy(); - this.#storage.disable(); - if (!this.#explicitRun && !this.success) { - process.exitCode = kGenericUserError; + try { + await this.#waitForStream(this.stream.summary({ + __proto__: null, + ...this.#getRecordScope(), + success: this.success, + counts: this.counts, + duration_ns: duration, + file: this.entryFile, + })); + } catch (error) { + try { + await this.#diagnostic(error, undefined, 'error'); + } catch { + // The stream can fail while reporting the summary listener error. + } + } finally { + this.stream.end(); + this.root.finished = true; + this.root.completion.resolve(); + this.root.emitDestroy(); + this.#storage.disable(); + if (!this.#explicitRun && !this.success) { + process.exitCode = kGenericUserError; + } } } @@ -795,7 +872,7 @@ class Harness { this.#prepare(); this.state = 'running'; await this.#executeSuite(this.root); - this.#finish(startTime); + await this.#finish(startTime); } } diff --git a/lib/internal/error_serdes.js b/lib/internal/error_serdes.js index efe75192d9f5..d473da06fd49 100644 --- a/lib/internal/error_serdes.js +++ b/lib/internal/error_serdes.js @@ -1,6 +1,7 @@ 'use strict'; const { + AggregateError, ArrayPrototypeForEach, Error, EvalError, @@ -41,7 +42,14 @@ const kCircularReference = 5; const kSymbolStringLength = 'Symbol('.length; const errors = { - Error, TypeError, RangeError, URIError, SyntaxError, ReferenceError, EvalError, + AggregateError, + Error, + EvalError, + RangeError, + ReferenceError, + SyntaxError, + TypeError, + URIError, }; const errorConstructorNames = new SafeSet(ObjectKeys(errors)); diff --git a/test/fixtures/bench-runner/acknowledged-records.mjs b/test/fixtures/bench-runner/acknowledged-records.mjs new file mode 100644 index 000000000000..633f3412eaaf --- /dev/null +++ b/test/fixtures/bench-runner/acknowledged-records.mjs @@ -0,0 +1,41 @@ +import common from '../../common/index.js'; + +const pending = new Map(); +const onMessage = (message) => { + if (message?.type !== 'node:bench:ack') return; + pending.get(message.id)?.(); + pending.delete(message.id); +}; +process.on('message', onMessage); + +const timeout = setTimeout(() => { + throw new Error('benchmark record was not acknowledged'); +}, common.platformTimeout(10_000)); + +function sendDiagnostic(id) { + return new Promise((resolve, reject) => { + pending.set(id, resolve); + process.send({ + id, + type: 'node:bench:record', + record: { + type: 'bench:diagnostic', + data: { + runId: process.env.NODE_BENCH_RUN_ID, + fileRunId: process.env.NODE_BENCH_FILE_RUN_ID, + entryFile: process.argv[1], + message: `acknowledged ${id}`, + level: 'info', + file: process.argv[1], + }, + }, + }, (error) => { + if (error) reject(error); + }); + }); +} + +for (let i = 0; i < 32; i++) await sendDiagnostic(10_000 + i); + +clearTimeout(timeout); +process.off('message', onMessage); diff --git a/test/fixtures/bench-runner/malformed-record.cjs b/test/fixtures/bench-runner/malformed-record.cjs index ab8b4bb84dc1..909e11877e72 100644 --- a/test/fixtures/bench-runner/malformed-record.cjs +++ b/test/fixtures/bench-runner/malformed-record.cjs @@ -3,6 +3,7 @@ const common = require('../../common'); const kind = process.env.NODE_BENCH_MALFORMED_RECORD; +const id = kind === 'sequence' ? null : 0; const record = kind === 'summary' ? { type: 'bench:summary', data: { @@ -22,5 +23,5 @@ const record = kind === 'summary' ? { }, } : null; -process.send?.({ type: 'node:bench:record', record }); +process.send?.({ id, type: 'node:bench:record', record }); setTimeout(() => process.exit(2), common.platformTimeout(10_000)); diff --git a/test/parallel/test-bench-cli.js b/test/parallel/test-bench-cli.js index 8130bf38dc9f..07924acadcfc 100644 --- a/test/parallel/test-bench-cli.js +++ b/test/parallel/test-bench-cli.js @@ -396,6 +396,7 @@ for (const isolation of ['process', 'none']) { } for (const { kind, message } of [ + { kind: 'sequence', message: /valid record sequence/ }, { kind: 'record', message: /not a valid benchmark record/ }, { kind: 'identity', message: /not a valid benchmark record/ }, { kind: 'summary', message: /not a valid benchmark summary/ }, @@ -561,6 +562,22 @@ if (common.hasInspector) { Array.from({ length: 30 }, (_, i) => `${i}\n`).join('')); } +{ + const result = spawnBench([ + '--bench-reporter=json', + fixtures.path('bench-runner/acknowledged-records.mjs'), + ]); + assert.strictEqual(result.status, 0); + assert.strictEqual(result.stderr, ''); + const records = parseRecords(result); + const diagnostics = records.filter( + ({ type }) => type === 'bench:diagnostic'); + assert.strictEqual(diagnostics.length, 32); + assert(diagnostics.every( + ({ data }) => /^acknowledged 10\d{3}$/.test(data.message))); + assert.strictEqual(records.at(-1).data.success, true); +} + { const result = spawnBench([ `--bench-reporter=${fixtures.fileURL('bench-runner/destroying-reporter.cjs')}`, diff --git a/test/parallel/test-bench-stream.js b/test/parallel/test-bench-stream.js new file mode 100644 index 000000000000..89425e4c079c --- /dev/null +++ b/test/parallel/test-bench-stream.js @@ -0,0 +1,292 @@ +// Flags: --no-warnings +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const { createRunner } = require('node:bench'); +const { setImmediate, setTimeout } = require('timers/promises'); + +function recordSample(b) { + b.record({ + __proto__: null, + operations: 1, + duration_ns: 1n, + }); +} + +async function testReadableBackpressure() { + const runner = createRunner({ yieldBetweenSamples: false }); + const sampleCount = 64; + let calls = 0; + const completion = runner.bench('bounded stream', { + samples: sampleCount, + }, (b) => { + calls++; + recordSample(b); + }); + const stream = runner.run(); + const iterator = stream[Symbol.asyncIterator](); + const first = await iterator.next(); + + assert.strictEqual(first.value.type, 'bench:start'); + await setImmediate(); + assert(calls < sampleCount); + assert(stream.readableLength <= stream.readableHighWaterMark); + + const records = [first.value]; + for (;;) { + const next = await iterator.next(); + if (next.done) break; + records.push(next.value); + } + + const result = await completion; + assert.strictEqual(calls, sampleCount); + assert.strictEqual(result.samples.length, sampleCount); + assert.strictEqual(records.length, sampleCount + 3); +} + +async function testNamedEventsWithoutReading() { + const runner = createRunner(); + const sampleCount = 64; + let calls = 0; + const completion = runner.bench('named events', { + samples: sampleCount, + }, (b) => { + calls++; + recordSample(b); + }); + const stream = runner.run(); + const summary = await new Promise((resolve) => { + stream.once('bench:summary', resolve); + }); + const result = await completion; + + assert.strictEqual(calls, sampleCount); + assert.strictEqual(result.samples.length, sampleCount); + assert.strictEqual(summary.success, true); + assert.strictEqual(stream.readableLength, sampleCount + 3); + assert(stream.readableLength > stream.readableHighWaterMark); + stream.destroy(); +} + +async function testCancellationCompletesBenchmarks() { + const runner = createRunner({ yieldBetweenSamples: false }); + const first = runner.bench('cancelled stream', { samples: 64 }, recordSample); + const second = runner.bench('continues headlessly', { + samples: 1, + }, recordSample); + const stream = runner.run(); + const iterator = stream[Symbol.asyncIterator](); + + await iterator.next(); + await iterator.return(); + const results = await Promise.all([first, second]); + assert.strictEqual(results[0].samples.length, 64); + assert.strictEqual(results[1].samples.length, 1); +} + +async function testDeliveryDoesNotConsumeTimeout() { + const runner = createRunner({ yieldBetweenSamples: false }); + const completion = runner.bench('slow consumer', { + samples: 32, + timeout: common.platformTimeout(20), + }, recordSample); + const stream = runner.run(); + const iterator = stream[Symbol.asyncIterator](); + + await iterator.next(); + await setTimeout(common.platformTimeout(50)); + for (;;) { + const next = await iterator.next(); + if (next.done) break; + } + + const result = await completion; + assert.strictEqual(result.error, undefined); + assert.strictEqual(result.samples.length, 32); +} + +async function testReportingFailureSettlesBenchmarks() { + const runner = createRunner({ yieldBetweenSamples: false }); + const failure = new Error('record listener failed'); + const first = runner.bench('reported', { samples: 1 }, recordSample); + const second = runner.bench('settled', { samples: 1 }, recordSample); + const stream = runner.run(); + stream.once('bench:complete', common.mustCall(() => { + throw failure; + })); + stream.resume(); + + const results = await Promise.all([first, second]); + assert.strictEqual(results[0].error, undefined); + assert.strictEqual(results[1].error, failure); + await setImmediate(); +} + +async function testSummaryListenerFailure() { + const runner = createRunner({ yieldBetweenSamples: false }); + const failure = new Error('summary listener failed'); + const completion = runner.bench('summary failure', { + samples: 1, + }, recordSample); + const stream = runner.run(); + const diagnostics = []; + stream.on('bench:diagnostic', (diagnostic) => { + diagnostics.push(diagnostic); + }); + stream.once('bench:summary', common.mustCall(() => { + throw failure; + })); + const ended = new Promise((resolve) => stream.once('end', resolve)); + stream.resume(); + + const result = await completion; + await ended; + assert.strictEqual(result.error, undefined); + assert.strictEqual(diagnostics.length, 1); + assert.strictEqual(diagnostics[0].error.message, failure.message); +} + +async function testRecordOwnership() { + const runner = createRunner({ yieldBetweenSamples: false }); + const expectedError = new Error('expected failure'); + expectedError.code = 'ERR_EXPECTED'; + expectedError.cause = expectedError; + expectedError.uncloneable = new WeakMap(); + expectedError.context = { + note: 'preserved', + callback() {}, + }; + const innerError = new Error('inner failure'); + innerError.code = 'ERR_INNER'; + const aggregate = new AggregateError([innerError], 'aggregate failure'); + aggregate.code = 'ERR_AGGREGATE'; + const causedError = new Error('caused failure', { cause: aggregate }); + causedError.references = new Map([['self', causedError]]); + causedError.members = new Set([causedError]); + const measured = runner.bench('owned result', { + params: { kind: 'original' }, + samples: 1, + }, (b) => { + b.record({ + __proto__: null, + operations: 1, + duration_ns: 1n, + detail: { value: 'original' }, + }); + }); + const failed = runner.bench('owned error', { samples: 1 }, () => { + throw expectedError; + }); + const caused = runner.bench('owned cause', { samples: 1 }, () => { + throw causedError; + }); + const thrownValue = new WeakMap(); + const uncloneable = runner.bench('uncloneable error', { + samples: 1, + }, () => { + throw thrownValue; + }); + const proxyError = new Proxy({}, { + getPrototypeOf() { + throw new Error('prototype trap'); + }, + }); + const trapped = runner.bench('trapping error', { samples: 1 }, () => { + throw proxyError; + }); + const afterTrap = runner.bench('after trapping error', { + samples: 1, + }, recordSample); + const stream = runner.run(); + let eventSample; + let eventComplete; + let eventError; + let eventSummary; + + stream.on('bench:sample', (sample) => { + if (sample.name !== 'owned result') return; + eventSample = sample; + sample.name = 'changed by event'; + sample.detail.value = 'changed by event'; + }); + stream.on('bench:complete', (result) => { + if (result.name === 'owned result') { + eventComplete = result; + result.params.kind = 'changed by event'; + result.samples[0].detail.value = 'changed by event'; + } else if (result.name === 'owned error') { + eventError = result.error; + result.error.code = 'ERR_CHANGED'; + } + }); + stream.on('bench:summary', (summary) => { + eventSummary = summary; + summary.counts.total = 100; + }); + + const records = await stream.toArray(); + const measuredResult = await measured; + const failedResult = await failed; + await caused; + const uncloneableResult = await uncloneable; + const trappedResult = await trapped; + const afterTrapResult = await afterTrap; + const streamSample = records.find( + ({ type }) => type === 'bench:sample').data; + const streamResults = records.filter( + ({ type }) => type === 'bench:complete').map(({ data }) => data); + const streamMeasured = streamResults.find( + ({ name }) => name === 'owned result'); + const streamFailed = streamResults.find( + ({ name }) => name === 'owned error'); + const streamCaused = streamResults.find( + ({ name }) => name === 'owned cause'); + const streamSummary = records.find( + ({ type }) => type === 'bench:summary').data; + + assert.notStrictEqual(eventSample, streamSample); + assert.notStrictEqual(eventComplete, streamMeasured); + assert.notStrictEqual(streamMeasured, measuredResult); + assert.strictEqual(streamSample.name, 'owned result'); + assert.strictEqual(streamSample.detail.value, 'original'); + assert.strictEqual(streamMeasured.params.kind, 'original'); + assert.strictEqual(streamMeasured.samples[0].detail.value, 'original'); + assert.strictEqual(measuredResult.params.kind, 'original'); + assert.strictEqual(measuredResult.samples[0].detail.value, 'original'); + + streamMeasured.samples[0].detail.value = 'changed by stream'; + assert.strictEqual(measuredResult.samples[0].detail.value, 'original'); + assert.notStrictEqual(eventError, streamFailed.error); + assert.notStrictEqual(streamFailed.error, expectedError); + assert.strictEqual(streamFailed.error.code, 'ERR_EXPECTED'); + assert.strictEqual(streamFailed.error.cause, streamFailed.error); + assert.strictEqual(streamFailed.error.context.note, 'preserved'); + assert.strictEqual(streamFailed.error.context.callback, undefined); + assert.strictEqual(failedResult.error, expectedError); + assert.strictEqual(failedResult.error.code, 'ERR_EXPECTED'); + assert.strictEqual(failedResult.error.cause, failedResult.error); + assert.strictEqual(uncloneableResult.error, thrownValue); + assert.strictEqual(trappedResult.error, proxyError); + assert.strictEqual(afterTrapResult.error, undefined); + assert(streamCaused.error.cause instanceof AggregateError); + assert.strictEqual(streamCaused.error.cause.name, 'AggregateError'); + assert.strictEqual(streamCaused.error.cause.code, 'ERR_AGGREGATE'); + assert.strictEqual(streamCaused.error.cause.errors[0].code, 'ERR_INNER'); + assert.strictEqual( + streamCaused.error.references.get('self'), streamCaused.error); + assert.strictEqual(streamCaused.error.members.has(streamCaused.error), true); + assert.notStrictEqual(eventSummary, streamSummary); + assert.strictEqual(streamSummary.counts.total, 6); +} + +(async () => { + await testReadableBackpressure(); + await testNamedEventsWithoutReading(); + await testCancellationCompletesBenchmarks(); + await testDeliveryDoesNotConsumeTimeout(); + await testReportingFailureSettlesBenchmarks(); + await testSummaryListenerFailure(); + await testRecordOwnership(); +})().then(common.mustCall()); diff --git a/test/sequential/test-error-serdes.js b/test/sequential/test-error-serdes.js index acd08903efab..75a37b376ca1 100644 --- a/test/sequential/test-error-serdes.js +++ b/test/sequential/test-error-serdes.js @@ -39,6 +39,10 @@ assert.strictEqual(cycle(new ReferenceError('foo')).name, 'ReferenceError'); assert.strictEqual(cycle(new URIError('foo')).name, 'URIError'); assert.strictEqual(cycle(new EvalError('foo')).name, 'EvalError'); assert.strictEqual(cycle(new SyntaxError('foo')).name, 'SyntaxError'); +const aggregate = cycle(new AggregateError([new Error('inner')], 'aggregate')); +assert(aggregate instanceof AggregateError); +assert.strictEqual(aggregate.message, 'aggregate'); +assert.strictEqual(aggregate.errors[0].message, 'inner'); class SubError extends Error {} From acb1c08dd51a8005b51a3f4d61461fbc3a3135eb Mon Sep 17 00:00:00 2001 From: James M Snell Date: Fri, 28 Aug 2026 21:13:09 +0000 Subject: [PATCH 03/14] lib: clarify mean in node:bench docs Signed-off-by: James M Snell Assisted-by: Opencode --- doc/api/bench.md | 19 ++++++++++++++++++- test/parallel/test-bench-context-control.js | 19 +++++++++++++++++-- 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/doc/api/bench.md b/doc/api/bench.md index 00faab4a8d2b..4e3c8c890777 100644 --- a/doc/api/bench.md +++ b/doc/api/bench.md @@ -81,6 +81,22 @@ Calling `context.done()` during a measured sample completes the benchmark after that sample. This allows a higher-level tool to treat `samples` as a maximum and implement a dynamic sampling policy. +The number of operations can differ between samples. Summary statistics treat +each sample's `rate` as one equally weighted observation. In particular, +`summary.mean` is the arithmetic mean of the per-sample rates. It is not the +pooled throughput calculated as: + +```text +1_000_000_000 * sum(sample.operations) / sum(sample.duration_ns) +``` + +The two values can differ when sample durations vary because pooled throughput +weights each per-sample rate by its duration. A higher-level tool that varies +batch sizes should choose the aggregation that matches its analysis. It can +calculate pooled throughput from the raw `samples`; operation counts should be +summed as `bigint` values because their total can exceed +`Number.MAX_SAFE_INTEGER` even though each count cannot. + ## Reusable runners The module-level declaration functions use a shared runner and schedule it @@ -621,7 +637,8 @@ A completed benchmark result contains: * `params` {Object} The canonical parameter metadata. * `samples` {Object\[]} The exact measured samples. * `summary` {Object} - * `mean` {number} The arithmetic mean of per-sample rates. + * `mean` {number} The equally weighted arithmetic mean of per-sample rates, + not pooled throughput across all operations and durations. * `median` {number} The median per-sample rate. * `min` {number} The minimum per-sample rate. * `max` {number} The maximum per-sample rate. diff --git a/test/parallel/test-bench-context-control.js b/test/parallel/test-bench-context-control.js index b9adb9f2f9f1..b847e926ffcd 100644 --- a/test/parallel/test-bench-context-control.js +++ b/test/parallel/test-bench-context-control.js @@ -59,10 +59,21 @@ const { createRunner } = require('node:bench'); b.done(); })); + const variableSamples = [ + { __proto__: null, duration_ns: 1_000_000_000n, operations: 1 }, + { __proto__: null, duration_ns: 100_000_000n, operations: 100 }, + ]; + const variableCompletion = runner.bench('variable batch', { + samples: variableSamples.length, + }, common.mustCall((b) => { + b.record(variableSamples[b.index]); + }, variableSamples.length)); + const records = await runner.run().toArray(); - const [controlled, recorded] = await Promise.all([ + const [controlled, recorded, variable] = await Promise.all([ controlledCompletion, recordedCompletion, + variableCompletion, ]); assert.deepStrictEqual(invocations, [ @@ -80,8 +91,12 @@ const { createRunner } = require('node:bench'); assert.strictEqual(recorded.samples.length, 1); assert.deepStrictEqual(recorded.samples[0].detail, { source: 'worker', value: 1n }); + assert.deepStrictEqual(variable.samples.map(({ rate }) => rate), [1, 1000]); + assert.strictEqual(variable.summary.mean, 500.5); + const pooledRate = 1_000_000_000 * 101 / 1_100_000_000; + assert.notStrictEqual(variable.summary.mean, pooledRate); assert.strictEqual( - records.filter(({ type }) => type === 'bench:sample').length, 3); + records.filter(({ type }) => type === 'bench:sample').length, 5); assert.throws(() => closedContext.start(), { code: 'ERR_INVALID_STATE' }); assert.throws(() => closedContext.end(1), { code: 'ERR_INVALID_STATE' }); assert.throws(() => closedContext.record({ From 857701ddf4cf17398e2154503dbe50984e9e1ecf Mon Sep 17 00:00:00 2001 From: James M Snell Date: Fri, 28 Aug 2026 21:23:37 +0000 Subject: [PATCH 04/14] doc: clarify measurement integrity details of node:bench Signed-off-by: James M Snell Assisted-by: Opencode --- doc/api/bench.md | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/doc/api/bench.md b/doc/api/bench.md index 4e3c8c890777..0636f35cbfbb 100644 --- a/doc/api/bench.md +++ b/doc/api/bench.md @@ -38,12 +38,17 @@ suite('URL', () => { params: { input: 'short' }, }, (b) => { const operations = 10_000; + let totalLength = 0; b.start(); for (let i = 0; i < operations; i++) { - new URL(input); + totalLength += new URL(input).href.length; } b.end(operations); + + if (totalLength !== operations * input.length) { + throw new Error('Unexpected URL result'); + } }); }); ``` @@ -77,6 +82,32 @@ system load can all affect results. Keep raw samples when comparing results and investigate noisy or skewed distributions rather than treating a confidence interval as a pass/fail threshold. +### Measurement integrity + +A statistically consistent result does not prove that a benchmark measured the +intended work. An optimizing runtime can remove work whose result is unused or +specialize it more narrowly than the workload being modeled. Framework and loop +overhead can also dominate operations that are too short. To reduce these risks: + +* Make values produced by measured work observable outside the measured + interval, for example by validating an aggregate derived from every result. + Passing them only through unused local computations is insufficient. +* Perform enough operations in each sample to amortize fixed timer reads and + calls to `context.start()` and `context.end()`. If loop bookkeeping is material + relative to one operation, batch multiple operations per iteration and report + the total operation count. +* Inspect raw `samples` for trends that indicate insufficient warmup or + optimization tiering, pauses consistent with garbage collection, and + multimodal distributions. +* Validate surprising results with an independent benchmark shape that performs + the same intended work differently. + +`node:bench` does not force a particular optimization state or infer whether an +engine eliminated work. Such controls and diagnostics are runtime-specific and +heuristic, and do not replace validating the benchmark workload. + +### Dynamic sampling and variable batches + Calling `context.done()` during a measured sample completes the benchmark after that sample. This allows a higher-level tool to treat `samples` as a maximum and implement a dynamic sampling policy. @@ -635,7 +666,8 @@ A completed benchmark result contains: * `column` {number} The source column. * `tags` {string\[]} The inherited canonical tags. * `params` {Object} The canonical parameter metadata. -* `samples` {Object\[]} The exact measured samples. +* `samples` {Object\[]} The exact measured samples in measurement invocation + order. * `summary` {Object} * `mean` {number} The equally weighted arithmetic mean of per-sample rates, not pooled throughput across all operations and durations. From 56c9492969aca8f68a17902ff802977ee49e2259 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Fri, 28 Aug 2026 22:23:39 +0000 Subject: [PATCH 05/14] lib: add `bench:plan` event to `node:bench` Signed-off-by: James M Snell Assisted-by: Opencode --- doc/api/bench.md | 28 ++++++ .../bench_runner/benchmarks_stream.js | 4 + lib/internal/bench_runner/cli.js | 86 ++++++++++++++++--- lib/internal/bench_runner/harness.js | 34 +++++++- .../bench-runner/destroying-reporter.cjs | 2 +- .../load-error-after-declaration.cjs | 11 +++ .../bench-runner/malformed-plan-order.mjs | 34 ++++++++ .../bench-runner/malformed-record.cjs | 33 +++++++ test/fixtures/bench-runner/slow-reporter.cjs | 1 + test/parallel/test-bench-cli.js | 80 +++++++++++++++++ test/parallel/test-bench-filtering.js | 23 ++++- test/parallel/test-bench-harness-errors.js | 6 ++ test/parallel/test-bench-reporters.js | 4 +- test/parallel/test-bench-run-options.js | 30 ++++++- test/parallel/test-bench-run.js | 6 ++ test/parallel/test-bench-stream.js | 42 ++++++++- 16 files changed, 402 insertions(+), 22 deletions(-) create mode 100644 test/fixtures/bench-runner/load-error-after-declaration.cjs create mode 100644 test/fixtures/bench-runner/malformed-plan-order.mjs diff --git a/doc/api/bench.md b/doc/api/bench.md index 0636f35cbfbb..b22f8f5050ce 100644 --- a/doc/api/bench.md +++ b/doc/api/bench.md @@ -594,6 +594,7 @@ both emitted as a named event and made available on the stream as The events are emitted in execution order: +* `'bench:plan'` * `'bench:start'` * `'bench:sample'` * `'bench:complete'` @@ -631,6 +632,33 @@ loading caused the declaration, while `file` identifies the source location of the declaration itself. `parentId` is based on the containing suite's source file and hierarchical name path. +After asynchronous suite declarations settle, an in-process runner emits one +`'bench:plan'` event for every benchmark it collected, in declaration order. +All plans from that runner are emitted before its suite hooks or benchmark +callbacks run. With process isolation, files run in separate children, so plans +for a later file are emitted after an earlier child has completed. With no +isolation, all files share one runner and their plans are emitted before any +benchmark executes. Plan data contains the benchmark-scoped identity, location, +tags, and parameters described in [benchmark result][], together with: + +* `samples` {number} The effective maximum number of measured callback + invocations after run-level overrides. +* `warmup` {number} The effective number of unreported warmup callback + invocations after run-level overrides. +* `timeout` {number|null} The timeout in milliseconds, or `null` when no timeout + is configured. +* `yieldBetweenSamples` {boolean} Whether an event loop turn is scheduled between + sample callbacks. +* `selected` {boolean} Whether the benchmark is eligible to run after applying + `skip`, `only`, and `namePattern` selection. Execution can still be prevented + by a duplicate declaration, suite build, hook, abort, or other runtime failure. +* `skip` {boolean|string} When `selected` is `false`, the explicit skip value or + the selection reason, such as `'only'` or `'name pattern'`. + +The plan contains execution settings known to the runner. Runtime version, +operating system, processor, and other environment metadata are intentionally +left for reporters and higher-level tools to collect. + `'bench:complete'` data contains a [benchmark result][]. A failed result has an additional `error` property and may contain samples recorded before the error. A skipped result has an additional `skip` property and an empty `samples` diff --git a/lib/internal/bench_runner/benchmarks_stream.js b/lib/internal/bench_runner/benchmarks_stream.js index 6d656e0ffdd4..295aa20483ce 100644 --- a/lib/internal/bench_runner/benchmarks_stream.js +++ b/lib/internal/bench_runner/benchmarks_stream.js @@ -238,6 +238,10 @@ class BenchmarksStream extends Readable { return waiter.promise; } + plan(data) { + return this[kEmitMessage]('bench:plan', data); + } + start(data) { return this[kEmitMessage]('bench:start', data); } diff --git a/lib/internal/bench_runner/cli.js b/lib/internal/bench_runner/cli.js index ae33694818b5..c0e07d4a72f4 100644 --- a/lib/internal/bench_runner/cli.js +++ b/lib/internal/bench_runner/cli.js @@ -10,8 +10,12 @@ const { ArrayPrototypePushApply, ArrayPrototypeSome, ArrayPrototypeSort, + NumberIsFinite, NumberIsSafeInteger, ObjectGetOwnPropertyDescriptor, + ObjectGetPrototypeOf, + ObjectPrototype, + ObjectValues, Promise, PromisePrototypeThen, PromiseReject, @@ -46,6 +50,7 @@ const { }, } = require('internal/errors'); const { getOptionValue, getOptionsAsFlagsFromBinding } = require('internal/options'); +const { TIMEOUT_MAX } = require('internal/timers'); const { kEmptyObject } = require('internal/util'); const { validateUint32 } = require('internal/validators'); const { pathToFileURL } = require('internal/url'); @@ -68,6 +73,7 @@ const kBuiltinReporters = new SafeMap([ const kChildAckMessageType = 'node:bench:ack'; const kChildMessageType = 'node:bench:record'; const kEventTypes = new SafeSet([ + 'bench:plan', 'bench:start', 'bench:sample', 'bench:complete', @@ -256,6 +262,8 @@ async function finishReporters(state) { function emitRecord(stream, record) { switch (record.type) { + case 'bench:plan': + return stream.plan(record.data); case 'bench:start': return stream.start(record.data); case 'bench:sample': @@ -311,6 +319,14 @@ function deserializeRecord(record) { }; } +function isStringArray(value) { + if (!ArrayIsArray(value)) return false; + for (let i = 0; i < value.length; i++) { + if (typeof value[i] !== 'string') return false; + } + return true; +} + function validateRecord(record) { if (record === null || typeof record !== 'object' || !kEventTypes.has(record.type) || record.data === null || @@ -323,18 +339,52 @@ function validateRecord(record) { throw new ERR_INVALID_ARG_VALUE( 'benchmark child message', record, 'is not a valid benchmark record'); } - if ((record.type === 'bench:start' || record.type === 'bench:sample' || - record.type === 'bench:complete') && - (typeof record.data.benchId !== 'string' || + if ((record.type === 'bench:plan' || record.type === 'bench:start' || + record.type === 'bench:sample' || record.type === 'bench:complete') && + (typeof record.data.fileRunId !== 'string' || + typeof record.data.benchId !== 'string' || (record.data.parentId !== null && - typeof record.data.parentId !== 'string') || - typeof record.data.name !== 'string' || - !ArrayIsArray(record.data.namePath) || - ArrayPrototypeSome( - record.data.namePath, (name) => typeof name !== 'string'))) { + typeof record.data.parentId !== 'string') || + typeof record.data.name !== 'string' || + !isStringArray(record.data.namePath))) { throw new ERR_INVALID_ARG_VALUE( 'benchmark child message', record, 'is not a valid benchmark record'); } + if (record.type === 'bench:plan') { + const { + samples, + selected, + skip, + timeout, + warmup, + yieldBetweenSamples, + } = record.data; + if (typeof record.data.file !== 'string' || + !NumberIsSafeInteger(record.data.line) || record.data.line < 0 || + !NumberIsSafeInteger(record.data.column) || record.data.column < 0 || + !isStringArray(record.data.tags) || + record.data.params === null || + typeof record.data.params !== 'object' || + ArrayIsArray(record.data.params) || + (ObjectGetPrototypeOf(record.data.params) !== null && + ObjectGetPrototypeOf(record.data.params) !== ObjectPrototype) || + ArrayPrototypeSome(ObjectValues(record.data.params), (value) => + typeof value !== 'string' && typeof value !== 'boolean' && + (typeof value !== 'number' || !NumberIsFinite(value))) || + !NumberIsSafeInteger(samples) || samples <= 0 || + samples > 0xFFFFFFFF || + !NumberIsSafeInteger(warmup) || warmup < 0 || warmup > 0xFFFFFFFF || + (timeout !== null && + (!NumberIsFinite(timeout) || timeout < 0 || timeout > TIMEOUT_MAX)) || + typeof yieldBetweenSamples !== 'boolean' || + typeof selected !== 'boolean' || + (selected && skip !== undefined) || + (!selected && skip !== true && typeof skip !== 'string')) { + throw new ERR_INVALID_ARG_VALUE( + 'benchmark child plan', record.data, + 'is not a valid benchmark plan'); + } + } if (record.type === 'bench:summary') { const { counts, duration_ns, success } = record.data; if (typeof success !== 'boolean' || typeof duration_ns !== 'bigint' || @@ -552,7 +602,9 @@ async function runChild(path, options, scope, onRecord) { stdio: ['inherit', 'pipe', 'pipe', 'ipc'], }); let protocolError; + let plansComplete = false; let recordPending = false; + let summaryReceived = false; const pendingRecords = new SafeSet(); const handleRecord = (record) => { if (protocolError !== undefined) return; @@ -571,7 +623,7 @@ async function runChild(path, options, scope, onRecord) { source?.resume(); }, (error) => { pendingRecords.delete(tracked); - protocolError = error; + protocolError ??= error; child.kill(); }); pendingRecords.add(tracked); @@ -610,10 +662,18 @@ async function runChild(path, options, scope, onRecord) { } recordPending = true; const record = deserializeRecord(validateRecord(message.record)); - record.data.runId = options.runId; - if (record.data.fileRunId !== null) { - record.data.fileRunId = scope.fileRunId; + if (summaryReceived || (record.type === 'bench:plan' && plansComplete)) { + throw new ERR_INVALID_ARG_VALUE( + 'benchmark child message', message, + 'does not have a valid lifecycle sequence'); } + if (record.type !== 'bench:plan' && + record.type !== 'bench:diagnostic') { + plansComplete = true; + } + if (record.type === 'bench:summary') summaryReceived = true; + record.data.runId = options.runId; + record.data.fileRunId = scope.fileRunId; if (record.data.entryFile !== null) { record.data.entryFile = scope.entryFile; } @@ -626,7 +686,7 @@ async function runChild(path, options, scope, onRecord) { })); } } catch (error) { - protocolError = error; + protocolError ??= error; child.kill(); } }); diff --git a/lib/internal/bench_runner/harness.js b/lib/internal/bench_runner/harness.js index fa0a2146d53d..0247500059be 100644 --- a/lib/internal/bench_runner/harness.js +++ b/lib/internal/bench_runner/harness.js @@ -254,7 +254,7 @@ class Harness { if (typeof namePattern === 'string') { nextNamePattern = new RegExp(namePattern); } else if (isRegExp(namePattern)) { - nextNamePattern = namePattern; + nextNamePattern = new RegExp(namePattern); } else { throw new ERR_INVALID_ARG_TYPE( 'options.namePattern', ['string', 'RegExp'], namePattern); @@ -391,6 +391,37 @@ class Harness { }); } + async #emitPlans() { + const benchmarks = []; + this.#walk(this.root, (node) => { + if (node instanceof Bench) ArrayPrototypePush(benchmarks, node); + }); + for (let i = 0; i < benchmarks.length; i++) { + const benchmark = benchmarks[i]; + const skip = this.#getSkip(benchmark); + const data = { + __proto__: null, + ...this.#getRecordScope(benchmark), + benchId: benchmark.benchId, + parentId: benchmark.parentId, + name: benchmark.name, + namePath: ArrayPrototypeSlice(benchmark.namePath), + file: benchmark.loc.file, + line: benchmark.loc.line, + column: benchmark.loc.column, + tags: ArrayPrototypeSlice(benchmark.tags), + params: benchmark.params, + samples: this.samples ?? benchmark.samples, + warmup: this.warmup ?? benchmark.warmup, + timeout: benchmark.timeout === Infinity ? null : benchmark.timeout, + yieldBetweenSamples: this.#yieldBetweenSamples, + selected: skip === null, + }; + if (skip !== null) data.skip = skip; + await this.#waitForStream(this.stream.plan(data)); + } + } + #hasSelectedAncestor(benchmark) { for (let current = benchmark; current !== null; current = current.parent) { if (current.only) return true; @@ -871,6 +902,7 @@ class Harness { this.#fileScopeStorage.disable(); this.#prepare(); this.state = 'running'; + await this.#emitPlans(); await this.#executeSuite(this.root); await this.#finish(startTime); } diff --git a/test/fixtures/bench-runner/destroying-reporter.cjs b/test/fixtures/bench-runner/destroying-reporter.cjs index 9e3c04fb1fb0..8a2a2406f932 100644 --- a/test/fixtures/bench-runner/destroying-reporter.cjs +++ b/test/fixtures/bench-runner/destroying-reporter.cjs @@ -1,7 +1,7 @@ 'use strict'; module.exports = async function* destroyingReporter(source) { - source.once('bench:start', () => { + source.once('bench:plan', () => { source.destroy(new Error('benchmark reporter closed the stream')); }); yield* source; diff --git a/test/fixtures/bench-runner/load-error-after-declaration.cjs b/test/fixtures/bench-runner/load-error-after-declaration.cjs new file mode 100644 index 000000000000..eca2188d41c5 --- /dev/null +++ b/test/fixtures/bench-runner/load-error-after-declaration.cjs @@ -0,0 +1,11 @@ +'use strict'; + +const { bench } = require('node:bench'); + +bench('declared before load error', { samples: 1 }, (b) => { + b.start(); + process.hrtime.bigint(); + b.end(1); +}); + +throw new Error('load failed after declaration'); diff --git a/test/fixtures/bench-runner/malformed-plan-order.mjs b/test/fixtures/bench-runner/malformed-plan-order.mjs new file mode 100644 index 000000000000..80c8cdf4cd8f --- /dev/null +++ b/test/fixtures/bench-runner/malformed-plan-order.mjs @@ -0,0 +1,34 @@ +import { bench } from 'node:bench'; +import common from '../../common/index.js'; + +await new Promise((resolve, reject) => { + const timeout = setTimeout(() => { + reject(new Error('benchmark summary was not acknowledged')); + }, common.platformTimeout(10_000)); + process.on('message', (message) => { + if (message?.type === 'node:bench:ack' && message.id === 0) { + clearTimeout(timeout); + resolve(); + } + }); + process.send?.({ + id: 0, + type: 'node:bench:record', + record: { + type: 'bench:summary', + data: { + counts: { completed: 0, failed: 0, skipped: 0, total: 0 }, + duration_ns: 1n, + entryFile: import.meta.filename, + fileRunId: process.env.NODE_BENCH_FILE_RUN_ID, + file: import.meta.filename, + runId: process.env.NODE_BENCH_RUN_ID, + success: true, + }, + }, + }); +}); + +bench('late plan', { samples: 1 }, () => { + throw new Error('late plan benchmark ran'); +}); diff --git a/test/fixtures/bench-runner/malformed-record.cjs b/test/fixtures/bench-runner/malformed-record.cjs index 909e11877e72..0e903da1a356 100644 --- a/test/fixtures/bench-runner/malformed-record.cjs +++ b/test/fixtures/bench-runner/malformed-record.cjs @@ -4,6 +4,33 @@ const common = require('../../common'); const kind = process.env.NODE_BENCH_MALFORMED_RECORD; const id = kind === 'sequence' ? null : 0; +const benchmarkData = { + __proto__: null, + benchId: 'invalid plan', + column: 1, + entryFile: __filename, + file: __filename, + fileRunId: process.env.NODE_BENCH_FILE_RUN_ID, + line: 1, + name: 'invalid plan', + namePath: ['invalid plan'], + params: {}, + parentId: null, + runId: process.env.NODE_BENCH_RUN_ID, + tags: [], +}; +const plan = { + type: 'bench:plan', + data: { + __proto__: null, + ...benchmarkData, + samples: 1, + selected: true, + timeout: null, + warmup: 0, + yieldBetweenSamples: true, + }, +}; const record = kind === 'summary' ? { type: 'bench:summary', data: { @@ -14,6 +41,12 @@ const record = kind === 'summary' ? { runId: process.env.NODE_BENCH_RUN_ID, success: true, }, +} : kind === 'plan' ? { + ...plan, + data: { + ...plan.data, + samples: 0, + }, } : kind === 'identity' ? { type: 'bench:complete', data: { diff --git a/test/fixtures/bench-runner/slow-reporter.cjs b/test/fixtures/bench-runner/slow-reporter.cjs index b5670d28ea70..7832bd94cd49 100644 --- a/test/fixtures/bench-runner/slow-reporter.cjs +++ b/test/fixtures/bench-runner/slow-reporter.cjs @@ -9,6 +9,7 @@ module.exports = async function* slowReporter(source) { if (++emitted === source.readableHighWaterMark) resolve(); }; for (const type of [ + 'bench:plan', 'bench:start', 'bench:sample', 'bench:complete', diff --git a/test/parallel/test-bench-cli.js b/test/parallel/test-bench-cli.js index 07924acadcfc..6913bb803f06 100644 --- a/test/parallel/test-bench-cli.js +++ b/test/parallel/test-bench-cli.js @@ -91,9 +91,12 @@ for (const { patterns, message } of [ const result = spawnBench(['--bench-reporter=json', basicPattern]); assert.strictEqual(result.status, 0); const records = parseRecords(result); + const plans = records.filter( + ({ type }) => type === 'bench:plan').map(({ data }) => data); const completions = records.filter( ({ type }) => type === 'bench:complete'); + assert.deepStrictEqual(plans.map(({ name }) => name), ['alpha', 'beta']); assert.deepStrictEqual(completions.map(({ data }) => data.name), [ 'alpha', 'beta', @@ -129,10 +132,20 @@ for (const isolation of ['process', 'none']) { ]); assert.strictEqual(result.status, 0); const records = parseRecords(result); + const plans = records.filter( + ({ type }) => type === 'bench:plan').map(({ data }) => data); const completions = records.filter( ({ type }) => type === 'bench:complete').map(({ data }) => data); const summary = records.at(-1).data; + assert.strictEqual(plans.length, 2); + for (const plan of plans) { + const planIndex = records.findIndex(({ type, data }) => + type === 'bench:plan' && data.fileRunId === plan.fileRunId); + const startIndex = records.findIndex(({ type, data }) => + type === 'bench:start' && data.fileRunId === plan.fileRunId); + assert(planIndex < startIndex); + } assert.strictEqual(completions.length, 2); assert.strictEqual(completions[0].benchId, completions[1].benchId); assert.strictEqual(completions[0].runId, completions[1].runId); @@ -249,8 +262,38 @@ for (const isolation of ['process', 'none']) { ]); assert.strictEqual(result.status, 0); const records = parseRecords(result); + const plans = records.filter( + ({ type }) => type === 'bench:plan').map(({ data }) => data); const samples = records.filter(({ type }) => type === 'bench:sample'); assert.deepStrictEqual(samples.map(({ data }) => data.operations), [4, 5]); + assert.deepStrictEqual(plans.map((plan) => ({ + name: plan.name, + samples: plan.samples, + selected: plan.selected, + skip: plan.skip, + timeout: plan.timeout, + warmup: plan.warmup, + yieldBetweenSamples: plan.yieldBetweenSamples, + })), [ + { + name: 'selected', + samples: 2, + selected: true, + skip: undefined, + timeout: null, + warmup: 3, + yieldBetweenSamples: true, + }, + { + name: 'filtered out', + samples: 2, + selected: false, + skip: 'name pattern', + timeout: null, + warmup: 3, + yieldBetweenSamples: true, + }, + ]); const completions = records.filter( ({ type }) => type === 'bench:complete'); @@ -304,6 +347,28 @@ for (const isolation of ['process', 'none']) { assert.strictEqual(records.at(-1).data.success, false); } +for (const isolation of ['process', 'none']) { + const result = spawnBench([ + `--bench-isolation=${isolation}`, + '--bench-reporter=json', + fixtures.path('bench-runner/load-error-after-declaration.cjs'), + ]); + assert.strictEqual(result.status, 1); + const records = parseRecords(result); + assert(records.some(({ type, data }) => + type === 'bench:diagnostic' && + data.message === 'load failed after declaration')); + const plan = records.find(({ type }) => type === 'bench:plan').data; + const completion = records.find( + ({ type }) => type === 'bench:complete').data; + assert.strictEqual(plan.name, 'declared before load error'); + assert.strictEqual(plan.selected, true); + assert.strictEqual(completion.name, plan.name); + assert.strictEqual(completion.error, undefined); + assert.strictEqual(completion.samples.length, 1); + assert.strictEqual(records.at(-1).data.counts.completed, 1); +} + { const result = spawnBench([ '--bench-reporter=json', @@ -399,6 +464,7 @@ for (const { kind, message } of [ { kind: 'sequence', message: /valid record sequence/ }, { kind: 'record', message: /not a valid benchmark record/ }, { kind: 'identity', message: /not a valid benchmark record/ }, + { kind: 'plan', message: /not a valid benchmark plan/ }, { kind: 'summary', message: /not a valid benchmark summary/ }, ]) { const result = spawnBench([ @@ -419,6 +485,20 @@ for (const { kind, message } of [ assert.strictEqual(records.at(-1).data.success, false); } +{ + const result = spawnBench([ + '--bench-reporter=json', + fixtures.path('bench-runner/malformed-plan-order.mjs'), + ]); + assert.strictEqual(result.status, 1); + const records = parseRecords(result); + const diagnostics = records.filter( + ({ type }) => type === 'bench:diagnostic'); + assert(diagnostics.some( + ({ data }) => /valid lifecycle sequence/.test(data.message))); + assert.strictEqual(records.at(-1).data.success, false); +} + for (const { mode, message } of [ { mode: 'code', message: /failed with exit code 2/ }, { mode: 'late', message: /failed with exit code 2/ }, diff --git a/test/parallel/test-bench-filtering.js b/test/parallel/test-bench-filtering.js index 41784c0dafef..39f5368d26a0 100644 --- a/test/parallel/test-bench-filtering.js +++ b/test/parallel/test-bench-filtering.js @@ -26,7 +26,18 @@ suite('selected', { only: true }, () => { bench('only filtered', { samples: 1 }, common.mustNotCall()); const results = []; -const stream = run({ namePattern: /^selected (included|explicitly skipped)$/ }); +const plans = []; +const namePattern = /^selected (included|explicitly skipped)$/; +let patternMutated = false; +const stream = run({ namePattern }); +stream.on('bench:plan', common.mustCall((plan) => { + assert.deepStrictEqual(calls, []); + plans.push(plan); + if (!patternMutated) { + patternMutated = true; + namePattern.compile('only filtered'); + } +}, 4)); stream.on('bench:complete', (result) => results.push(result)); stream.on('end', common.mustCall(() => { assert.deepStrictEqual(calls, ['included']); @@ -37,5 +48,15 @@ stream.on('end', common.mustCall(() => { assert.strictEqual(byName.get('explicitly skipped').skip, true); assert.strictEqual(byName.get('pattern filtered').skip, 'name pattern'); assert.strictEqual(byName.get('only filtered').skip, 'only'); + assert.deepStrictEqual(plans.map(({ name, selected, skip }) => ({ + name, + selected, + skip, + })), [ + { name: 'included', selected: true, skip: undefined }, + { name: 'explicitly skipped', selected: false, skip: true }, + { name: 'pattern filtered', selected: false, skip: 'name pattern' }, + { name: 'only filtered', selected: false, skip: 'only' }, + ]); })); stream.resume(); diff --git a/test/parallel/test-bench-harness-errors.js b/test/parallel/test-bench-harness-errors.js index 3e4ebefd2ebf..a5e01894c640 100644 --- a/test/parallel/test-bench-harness-errors.js +++ b/test/parallel/test-bench-harness-errors.js @@ -22,8 +22,14 @@ async function testSynchronousSuiteFailure() { }); const records = await runner.run().toArray(); await completion; + const plan = records.find( + ({ type }) => type === 'bench:plan').data; const result = records.find( ({ type }) => type === 'bench:complete').data; + assert.strictEqual(plan.name, 'blocked'); + assert.strictEqual(plan.selected, true); + assert.strictEqual( + records.some(({ type }) => type === 'bench:start'), false); assert.strictEqual(result.name, 'blocked'); assert.strictEqual(result.error.message, 'synchronous suite failure'); } diff --git a/test/parallel/test-bench-reporters.js b/test/parallel/test-bench-reporters.js index ed6039d9da3a..44e43bd5be78 100644 --- a/test/parallel/test-bench-reporters.js +++ b/test/parallel/test-bench-reporters.js @@ -27,7 +27,9 @@ bench('json failed', { samples: 1 }, () => { const lines = chunks.join('').trim().split('\n'); const records = lines.map((line) => JSON.parse(line)); - assert.strictEqual(records.length, 6); + assert.strictEqual(records.length, 8); + const plans = records.filter(({ type }) => type === 'bench:plan'); + assert.deepStrictEqual(plans.map(({ data }) => data.selected), [true, true]); const sample = records.find(({ type }) => type === 'bench:sample'); assert.match(sample.data.duration_ns, /^\d+$/); diff --git a/test/parallel/test-bench-run-options.js b/test/parallel/test-bench-run-options.js index e9397908a42c..e04bc15ef335 100644 --- a/test/parallel/test-bench-run-options.js +++ b/test/parallel/test-bench-run-options.js @@ -6,17 +6,43 @@ const assert = require('assert'); const { bench, run } = require('node:bench'); let invocations = 0; -bench('overridden', { samples: 8, warmup: 8 }, (b) => { +const timeout = common.platformTimeout(1000); +bench('overridden', { samples: 8, timeout, warmup: 8 }, (b) => { invocations++; b.start(); process.hrtime.bigint(); b.end(invocations); }); +const plans = []; const samples = []; -const stream = run({ samples: 2, warmup: 3 }); +const types = []; +const stream = run({ + samples: 2, + warmup: 3, + yieldBetweenSamples: false, +}); +stream.on('bench:plan', (plan) => plans.push(plan)); stream.on('bench:sample', ({ operations }) => samples.push(operations)); +stream.on('data', ({ type }) => types.push(type)); stream.on('end', common.mustCall(() => { + assert.strictEqual(plans.length, 1); + assert.deepStrictEqual({ + samples: plans[0].samples, + selected: plans[0].selected, + skip: plans[0].skip, + timeout: plans[0].timeout, + warmup: plans[0].warmup, + yieldBetweenSamples: plans[0].yieldBetweenSamples, + }, { + samples: 2, + selected: true, + skip: undefined, + timeout, + warmup: 3, + yieldBetweenSamples: false, + }); assert.deepStrictEqual(samples, [4, 5]); + assert.deepStrictEqual(types.slice(0, 2), ['bench:plan', 'bench:start']); })); stream.resume(); diff --git a/test/parallel/test-bench-run.js b/test/parallel/test-bench-run.js index 5b7edb4e6418..0e152342bcaa 100644 --- a/test/parallel/test-bench-run.js +++ b/test/parallel/test-bench-run.js @@ -65,7 +65,12 @@ const suiteCompletion = suite('group', { tags: ['Group'] }, async () => { }); const records = []; +const plans = []; const stream = run(); +stream.on('bench:plan', common.mustCall((plan) => { + assert.deepStrictEqual(calls, []); + plans.push(plan.name); +}, 3)); stream.on('data', (record) => records.push(record)); stream.on('end', common.mustCall(() => { assert.strictEqual(active, false); @@ -80,6 +85,7 @@ stream.on('end', common.mustCall(() => { assert.strictEqual(samples.length, 4); assert.strictEqual(completions.length, 3); assert.strictEqual(summaries.length, 1); + assert.deepStrictEqual(plans, ['sync', 'async', 'skipped']); const sync = completions.find(({ data }) => data.name === 'sync').data; assert.strictEqual(sync.error, undefined); diff --git a/test/parallel/test-bench-stream.js b/test/parallel/test-bench-stream.js index 89425e4c079c..11a7e0184a54 100644 --- a/test/parallel/test-bench-stream.js +++ b/test/parallel/test-bench-stream.js @@ -28,7 +28,7 @@ async function testReadableBackpressure() { const iterator = stream[Symbol.asyncIterator](); const first = await iterator.next(); - assert.strictEqual(first.value.type, 'bench:start'); + assert.strictEqual(first.value.type, 'bench:plan'); await setImmediate(); assert(calls < sampleCount); assert(stream.readableLength <= stream.readableHighWaterMark); @@ -43,7 +43,42 @@ async function testReadableBackpressure() { const result = await completion; assert.strictEqual(calls, sampleCount); assert.strictEqual(result.samples.length, sampleCount); - assert.strictEqual(records.length, sampleCount + 3); + assert.strictEqual(records[1].type, 'bench:start'); + assert.strictEqual(records.length, sampleCount + 4); +} + +async function testPlanBackpressure() { + const runner = createRunner({ yieldBetweenSamples: false }); + const benchmarkCount = 32; + const completions = []; + let calls = 0; + for (let i = 0; i < benchmarkCount; i++) { + completions.push(runner.bench(`planned ${i}`, { samples: 1 }, (b) => { + calls++; + recordSample(b); + })); + } + const stream = runner.run(); + const iterator = stream[Symbol.asyncIterator](); + const first = await iterator.next(); + + assert.strictEqual(first.value.type, 'bench:plan'); + await setImmediate(); + assert.strictEqual(calls, 0); + assert(stream.readableLength <= stream.readableHighWaterMark); + const records = [first.value]; + for (;;) { + const next = await iterator.next(); + if (next.done) break; + records.push(next.value); + } + + await Promise.all(completions); + assert.strictEqual(calls, benchmarkCount); + assert.strictEqual( + records.slice(0, benchmarkCount).every(({ type }) => type === 'bench:plan'), + true, + ); } async function testNamedEventsWithoutReading() { @@ -65,7 +100,7 @@ async function testNamedEventsWithoutReading() { assert.strictEqual(calls, sampleCount); assert.strictEqual(result.samples.length, sampleCount); assert.strictEqual(summary.success, true); - assert.strictEqual(stream.readableLength, sampleCount + 3); + assert.strictEqual(stream.readableLength, sampleCount + 4); assert(stream.readableLength > stream.readableHighWaterMark); stream.destroy(); } @@ -283,6 +318,7 @@ async function testRecordOwnership() { (async () => { await testReadableBackpressure(); + await testPlanBackpressure(); await testNamedEventsWithoutReading(); await testCancellationCompletesBenchmarks(); await testDeliveryDoesNotConsumeTimeout(); From c06055133d5a34148706462425da1f29fb49bd12 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Fri, 28 Aug 2026 22:36:07 +0000 Subject: [PATCH 06/14] doc: clarify isolation modes for node:bench Signed-off-by: James M Snell Assisted-by: Opencode --- doc/api/bench.md | 16 ++++++++++++++++ doc/api/cli.md | 4 ++++ 2 files changed, 20 insertions(+) diff --git a/doc/api/bench.md b/doc/api/bench.md index b22f8f5050ce..16833caac541 100644 --- a/doc/api/bench.md +++ b/doc/api/bench.md @@ -179,6 +179,21 @@ they do not corrupt reporter output. has lower startup overhead, but module, heap, and process state carry between files, and user writes share stdout and stderr with reporters. +Worker-thread isolation is not a CLI mode. Each newly constructed {Worker} has a +separate V8 isolate, JavaScript heap, and event loop, typically with lower +startup cost than a child process. Reusing a worker preserves its module and heap +state. Workers also share libuv's process-wide thread pool and can share +process-global native or addon state, so they do not provide the same boundary +as process isolation. + +Higher-level tools can experiment with worker isolation by loading benchmark +code inside a worker, measuring there, transferring structured sample data, and +passing it to [`context.record()`][]. The reported `duration_ns` can exclude +message transport when the worker captures both timestamps. Tools should +identify worker modules and workloads explicitly. They should not stringify +arbitrary functions or closures to move them between isolates, because closures +cannot be reconstructed with their original lexical environment. + Benchmark files passed to `--bench` should declare benchmarks but must not call `run()`. The CLI supports `--bench-name-pattern`, `--bench-samples`, `--bench-warmup`, `--bench-reporter`, and `--bench-reporter-destination`. See @@ -710,6 +725,7 @@ A completed benchmark result contains: interval for the median rate, with `lower` and `upper` properties. * `skewness` {number} The skewness of the scaled rate histogram. +[`context.record()`]: #contextrecordsample [`run()`]: #runoptions [benchmark result]: #benchmark-result [command-line options documentation]: cli.md#--bench diff --git a/doc/api/cli.md b/doc/api/cli.md index 804cc1472007..747e9e0b5d41 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -492,6 +492,10 @@ benchmark runner process. This reduces startup overhead but allows module, heap, and process state to carry between files. User writes to stdout or stderr also share destinations with benchmark reporters in this mode. +The supported modes are `'process'` and `'none'`. Worker-thread isolation is not +a CLI mode. Higher-level tools can implement it using externally measured +samples as described in the [benchmark runner][] documentation. + ### `--bench-name-pattern=pattern` + +* `message` {string} The diagnostic message. +* `options` {Object} + * `level` {string} Either `'info'` or `'warning'`. **Default:** `'info'`. + * `detail` {any} Additional structured-cloneable diagnostic data. With CLI + process isolation, it must also be supported by advanced child process + serialization. +* Returns: {undefined} + +Queues a diagnostic associated with the current benchmark, phase, and sample +index. Multiple diagnostics preserve call order. They are emitted after the +sample callback settles and before that sample's `'bench:sample'` event. Warmup +diagnostics are emitted even though warmup samples are not. Diagnostics queued +before a callback failure are emitted before the failed `'bench:complete'` +event and do not themselves cause the benchmark to fail. If a timeout or abort +wins before the callback settles, queued diagnostics might not be emitted. + +The message and options are validated, and detail is cloned, synchronously. +Calling `diagnostic()` between `context.start()` and `context.end()` therefore +includes that work in the measured duration. Invalid arguments or an +uncloneable detail violate the sample contract. + ### `context.done()` The `node:bench` module supports defining and running JavaScript benchmarks in -the current process. To access it: +the current process, and running one benchmark file in a fresh child process. +To access it: ```mjs import { bench, suite } from 'node:bench'; @@ -497,6 +498,51 @@ for await (const { type, data } of run()) { } ``` +## `runFile(path[, options])` + + + +* `path` {string} The absolute path of one benchmark module. +* `options` {Object} + * `env` {Object} The child process environment. Property values must be + strings or `undefined`. This replaces, rather than extends, the parent + environment. **Default:** A snapshot of `process.env`. + * `execArgv` {string\[]} Node.js command-line options for the child process. + This replaces, rather than extends, inherited options. Benchmark runner + options, positional arguments, and options that select another execution + mode are not allowed. **Default:** Compatible options inherited from the + current process. + * `signal` {AbortSignal} Terminates the child process when aborted. +* Returns: {BenchmarksStream} + +Runs exactly one benchmark module in a fresh child process and returns its +object-mode event stream. `path` is not interpreted as a glob. Unless the signal +is aborted or the stream is destroyed before startup, every call uses a new +child. Input discovery, ordering, concurrency, retries, and multi-file +scheduling remain the caller's responsibility. + +Records use advanced child process serialization, preserving supported +structured values such as `bigint` and errors. Child writes to stdout and stderr +become `'bench:diagnostic'` records. A module loading error, abnormal child exit, +or cancellation also emits an error diagnostic and produces a terminal +`'bench:summary'` whose `success` property is `false`; these execution failures +do not error the stream. If module evaluation fails after declaring benchmarks, +those declarations still run before the unsuccessful summary. + +`env`, effective inherited options, and an explicitly provided `execArgv` are +copied when `runFile()` is called. The runner removes `NODE_OPTIONS`, replaces +IPC-related environment variables, and sets its private child-context, run +identity, and file identity variables, overriding properties with those names +in `env`. Pass child Node.js options through `execArgv`, not `NODE_OPTIONS`. +Standard `child_process` environment propagation still applies, including +`NODE_V8_COVERAGE`, permission-model options, and required z/OS variables. +Aborting `signal` before the child starts produces an `AbortError` diagnostic +without spawning it. Aborting during execution sends `SIGTERM` to the child and +escalates to `SIGKILL` if it does not exit. Destroying the returned stream +follows the same termination procedure. + ## Class: `BenchContext` An instance of `BenchContext` is passed to every benchmark invocation. A new diff --git a/lib/bench.js b/lib/bench.js index 9d1bd24fcebd..96e199773d90 100644 --- a/lib/bench.js +++ b/lib/bench.js @@ -16,6 +16,7 @@ const { const { createRunner, run, + runFile, } = require('internal/bench_runner/runner'); if (process.env.NODE_BENCH_CONTEXT !== 'child' || @@ -33,5 +34,6 @@ ObjectAssign(module.exports, { createRunner, describe: suite, run, + runFile, suite, }); diff --git a/lib/internal/bench_runner/cli.js b/lib/internal/bench_runner/cli.js index 38611e49a991..4b14493e391d 100644 --- a/lib/internal/bench_runner/cli.js +++ b/lib/internal/bench_runner/cli.js @@ -8,13 +8,17 @@ const { ArrayPrototypeJoin, ArrayPrototypePush, ArrayPrototypePushApply, + ArrayPrototypeSlice, ArrayPrototypeSome, ArrayPrototypeSort, + MathMax, NumberIsFinite, NumberIsSafeInteger, ObjectGetOwnPropertyDescriptor, ObjectGetPrototypeOf, + ObjectKeys, ObjectPrototype, + ObjectPrototypeHasOwnProperty, ObjectValues, Promise, PromisePrototypeThen, @@ -27,8 +31,11 @@ const { SafeSet, String, StringPrototypeIndexOf, + StringPrototypeReplaceAll, StringPrototypeSlice, StringPrototypeStartsWith, + StringPrototypeToUpperCase, + SymbolDispose, } = primordials; const { spawn } = require('child_process'); const { createWriteStream, statSync } = require('fs'); @@ -44,19 +51,32 @@ const { } = require('internal/bench_runner/harness'); const { deserializeError, serializeError } = require('internal/error_serdes'); const { + AbortError, codes: { + ERR_INVALID_ARG_TYPE, ERR_INVALID_ARG_VALUE, ERR_INVALID_STATE, }, } = require('internal/errors'); -const { getOptionValue, getOptionsAsFlagsFromBinding } = require('internal/options'); +const { addAbortListener } = require('internal/events/abort_listener'); +const { + getCLIOptionsInfo, + getOptionValue, + getOptionsAsFlagsFromBinding, +} = require('internal/options'); const { TIMEOUT_MAX } = require('internal/timers'); const { kEmptyObject } = require('internal/util'); -const { validateUint32 } = require('internal/validators'); +const { + validateAbortSignal, + validateArray, + validateObject, + validateStringWithoutNullBytes, + validateUint32, +} = require('internal/validators'); const { pathToFileURL } = require('internal/url'); const { pipeline } = require('stream/promises'); -const { once } = require('events'); -const { resolve, sep } = require('path'); +const { isAbsolute, resolve, sep } = require('path'); +const { clearTimeout, setTimeout } = require('timers'); const console = require('internal/console/global'); const esmLoader = require('internal/modules/esm/loader'); @@ -90,6 +110,51 @@ const kFilterArgValues = [ '--bench-warmup', '--experimental-config-file', ]; +const kIncompatibleExecArgv = new SafeSet([ + '--build-sea', + '--build-snapshot', + '--build-snapshot-config', + '--check', + '--completion-bash', + '--eval', + '--experimental-sea-config', + '--help', + '--help-all', + '--input-type', + '--interactive', + '--print', + '--prof-process', + '--run', + '--test', + '--version', + '--v8-options', + '--watch', + '--watch-path', + '-c', + '-e', + '-h', + '-i', + '-p', + '-v', +]); +const kExecArgvWithValue = new SafeSet([ + '--eval', + '--input-type', + '--print', + '--run', + '--watch-path', + '-e', + '-p', +]); +const kIPCEnvironmentVariables = new SafeSet([ + 'NODE_CHANNEL_FD', + 'NODE_CHANNEL_SERIALIZATION_MODE', + 'NODE_BENCH_CONTEXT', + 'NODE_BENCH_FILE_RUN_ID', + 'NODE_BENCH_RUN_ID', + 'NODE_OPTIONS', +]); +const kForceKillDelay = 1_000; function createBenchmarkFileList(patterns, cwd) { if (patterns.length === 0) { @@ -533,6 +598,26 @@ async function loadBenchmarkFiles(files, options, modules, onRecord) { }, true); let summary; for await (const record of stream) { + if (record.type === 'bench:summary' && (process.exitCode ?? 0) !== 0) { + const scope = files.length === 1 ? options.fileScopes[0] : { + __proto__: null, + entryFile: null, + fileRunId: null, + }; + await onRecord({ + __proto__: null, + type: 'bench:diagnostic', + data: { + __proto__: null, + runId: options.runId, + ...scope, + message: `Benchmark process set exit code ${process.exitCode}`, + level: 'error', + file: files.length === 1 ? + resolve(options.cwd, files[0]) : null, + }, + }); + } if (record.type === 'bench:summary' && (loadFailed || (process.exitCode ?? 0) !== 0)) { record.data.success = false; @@ -552,18 +637,62 @@ async function loadBenchmarkFiles(files, options, modules, onRecord) { } function filterExecArgv(arg, index, args) { - return !ArrayPrototypeIncludes(kFilterArgs, arg) && + const name = getOptionName(arg); + return !ArrayPrototypeIncludes(kFilterArgs, name) && !ArrayPrototypeSome(kFilterArgValues, (option) => { - return arg === option || - StringPrototypeStartsWith(arg, `${option}=`) || + return name === option || (option !== '--experimental-config-file' && - index > 0 && args[index - 1] === option); + index > 0 && getOptionName(args[index - 1]) === option); }); } function getOptionName(arg) { const equals = StringPrototypeIndexOf(arg, '='); - return equals === -1 ? arg : StringPrototypeSlice(arg, 0, equals); + const name = equals === -1 ? arg : StringPrototypeSlice(arg, 0, equals); + return StringPrototypeReplaceAll(name, '_', '-'); +} + +function getRunFileOptionName(arg) { + if (StringPrototypeStartsWith(arg, '-') && + !StringPrototypeStartsWith(arg, '--') && arg.length > 2) { + const shortName = StringPrototypeSlice(arg, 0, 2); + if (kIncompatibleExecArgv.has(shortName)) return shortName; + } + return getOptionName(arg); +} + +function filterRunFileExecArgv(arg, index, args) { + if (!filterExecArgv(arg, index, args)) return false; + if (!StringPrototypeStartsWith(arg, '-') || arg === '-' || arg === '--') { + return false; + } + const name = getRunFileOptionName(arg); + if (kIncompatibleExecArgv.has(name)) return false; + if (index > 0) { + const previous = getRunFileOptionName(args[index - 1]); + if (kExecArgvWithValue.has(previous) && + StringPrototypeIndexOf(args[index - 1], '=') === -1) { + return false; + } + } + return true; +} + +function runFileOptionRequiresValue(arg) { + const equals = StringPrototypeIndexOf(arg, '='); + let name = getRunFileOptionName(arg); + const { aliases, options } = getCLIOptionsInfo(); + let info = options.get(name); + if (info === undefined) { + const alias = aliases.get(name); + if (alias !== undefined) info = options.get(alias[0]); + } + if (info === undefined && StringPrototypeStartsWith(name, '--no-')) { + name = `--${StringPrototypeSlice(name, 5)}`; + info = options.get(name); + } + return info !== undefined && info.type >= 3 && + (equals === -1 || equals === arg.length - 1); } const kOptionAliases = new SafeMap([ @@ -572,7 +701,7 @@ const kOptionAliases = new SafeMap([ ['-r', '--require'], ]); -function getChildArgs(path, options) { +function getInheritedChildArgs() { const nodeOptions = getOptionsAsFlagsFromBinding(); const args = ArrayPrototypeFilter(nodeOptions, filterExecArgv); const nodeOptionNames = new SafeSet(); @@ -597,6 +726,12 @@ function getChildArgs(path, options) { ArrayPrototypePushApply(args, unknownExecArgv); // Option serialization omits port 0, which would otherwise become 9229. if (process.debugPort === 0) ArrayPrototypePush(args, '--inspect-port=0'); + return args; +} + +function getChildArgs(path, options) { + const args = options.execArgv === undefined ? + getInheritedChildArgs() : ArrayPrototypeSlice(options.execArgv); ArrayPrototypePush(args, '--bench', '--bench-isolation=none'); if (options.namePatternSource.length > 0) { ArrayPrototypePush( @@ -613,24 +748,100 @@ function getChildArgs(path, options) { } async function runChild(path, options, scope, onRecord) { - const child = spawn(process.execPath, getChildArgs(path, options), { - __proto__: null, - cwd: options.cwd, - env: { + if (options.signal?.aborted) { + return { + __proto__: null, + aborted: true, + error: new AbortError(undefined, { + __proto__: null, + cause: options.signal.reason, + }), + }; + } + const child = spawn( + options.execPath ?? process.execPath, + getChildArgs(path, options), + { __proto__: null, - ...process.env, - NODE_BENCH_CONTEXT: 'child', - NODE_BENCH_FILE_RUN_ID: scope.fileRunId, - NODE_BENCH_RUN_ID: options.runId, + cwd: options.cwd, + env: { + __proto__: null, + ...(options.env ?? process.env), + NODE_BENCH_CONTEXT: 'child', + NODE_BENCH_FILE_RUN_ID: scope.fileRunId, + NODE_BENCH_RUN_ID: options.runId, + }, + serialization: 'advanced', + stdio: ['inherit', 'pipe', 'pipe', 'ipc'], }, - serialization: 'advanced', - stdio: ['inherit', 'pipe', 'pipe', 'ipc'], - }); + ); + let childClosed = false; + let forceKillTimer; + const terminateChild = () => { + if (childClosed) return; + child.kill(); + forceKillTimer ??= setTimeout(() => { + child.kill('SIGKILL'); + }, kForceKillDelay); + }; + const closed = PromiseWithResolvers(); + let closeTracked = false; + let spawnError; + try { + child.once('close', (...status) => closed.resolve(status)); + closeTracked = true; + child.once('error', (error) => { + spawnError ??= error; + terminateChild(); + }); + } catch (error) { + terminateChild(); + if (closeTracked) { + try { + await closed.promise; + } catch { + // Preserve the setup error. + } + } + throw error; + } let protocolError; + let abortError; + let aborted = false; let activeBenchId; let plansComplete = false; let recordPending = false; let summaryReceived = false; + let abortListener; + const closeListener = () => { + terminateChild(); + }; + try { + if (options.signal !== undefined) { + abortListener = addAbortListener(options.signal, () => { + aborted = true; + abortError = new AbortError(undefined, { + __proto__: null, + cause: options.signal.reason, + }); + terminateChild(); + }); + } + options.output?.once('close', closeListener); + if (options.output?.destroyed) closeListener(); + } catch (error) { + terminateChild(); + try { + await closed.promise; + } catch { + // Preserve the setup error. + } + childClosed = true; + if (forceKillTimer !== undefined) clearTimeout(forceKillTimer); + abortListener?.[SymbolDispose](); + options.output?.removeListener('close', closeListener); + throw error; + } const pendingRecords = new SafeSet(); const handleRecord = (record) => { if (protocolError !== undefined) return; @@ -638,7 +849,7 @@ async function runChild(path, options, scope, onRecord) { return onRecord(record); } catch (error) { protocolError = error; - child.kill(); + terminateChild(); } }; const trackPending = (pending, source) => { @@ -649,8 +860,8 @@ async function runChild(path, options, scope, onRecord) { source?.resume(); }, (error) => { pendingRecords.delete(tracked); - protocolError ??= error; - child.kill(); + if (!aborted) protocolError ??= error; + terminateChild(); }); pendingRecords.add(tracked); }; @@ -670,69 +881,99 @@ async function runChild(path, options, scope, onRecord) { }); trackPending(pending, source); }; - child.stdout.setEncoding('utf8'); - child.stderr.setEncoding('utf8'); - child.stdout.on('data', (data) => { - reportOutput(child.stdout, 'stdout', data); - }); - child.stderr.on('data', (data) => { - reportOutput(child.stderr, 'stderr', data); - }); - child.on('message', (message) => { - if (message?.type !== kChildMessageType) return; - try { - if (!NumberIsSafeInteger(message.id) || message.id < 0 || recordPending) { - throw new ERR_INVALID_ARG_VALUE( - 'benchmark child message', message, - 'does not have a valid record sequence'); - } - recordPending = true; - const record = deserializeRecord(validateRecord(message.record)); - const contextDiagnostic = isContextDiagnostic(record); - if (summaryReceived || (record.type === 'bench:plan' && plansComplete) || - (record.type === 'bench:start' && activeBenchId !== undefined) || - ((record.type === 'bench:sample' || contextDiagnostic) && - activeBenchId !== record.data.benchId) || - (record.type === 'bench:complete' && activeBenchId !== undefined && - activeBenchId !== record.data.benchId) || - (record.type === 'bench:summary' && activeBenchId !== undefined)) { - throw new ERR_INVALID_ARG_VALUE( - 'benchmark child message', message, - 'does not have a valid lifecycle sequence'); - } - if (record.type !== 'bench:plan' && - record.type !== 'bench:diagnostic') { - plansComplete = true; - } - if (record.type === 'bench:start') { - activeBenchId = record.data.benchId; - } else if (record.type === 'bench:complete' && - activeBenchId === record.data.benchId) { - activeBenchId = undefined; - } - if (record.type === 'bench:summary') summaryReceived = true; - record.data.runId = options.runId; - record.data.fileRunId = scope.fileRunId; - if (record.data.entryFile !== null) { - record.data.entryFile = scope.entryFile; - } - const pending = handleRecord(record); - if (protocolError === undefined) { - const acknowledged = PromisePrototypeThen( - PromiseResolve(pending), () => sendAck(child, message.id)); - trackPending(PromisePrototypeThen(acknowledged, () => { - recordPending = false; - })); + try { + child.stdout.setEncoding('utf8'); + child.stderr.setEncoding('utf8'); + child.stdout.on('data', (data) => { + reportOutput(child.stdout, 'stdout', data); + }); + child.stderr.on('data', (data) => { + reportOutput(child.stderr, 'stderr', data); + }); + child.on('message', (message) => { + if (message?.type !== kChildMessageType) return; + try { + if (!NumberIsSafeInteger(message.id) || message.id < 0 || recordPending) { + throw new ERR_INVALID_ARG_VALUE( + 'benchmark child message', message, + 'does not have a valid record sequence'); + } + recordPending = true; + const record = deserializeRecord(validateRecord(message.record)); + const contextDiagnostic = isContextDiagnostic(record); + if (summaryReceived || (record.type === 'bench:plan' && plansComplete) || + (record.type === 'bench:start' && activeBenchId !== undefined) || + ((record.type === 'bench:sample' || contextDiagnostic) && + activeBenchId !== record.data.benchId) || + (record.type === 'bench:complete' && activeBenchId !== undefined && + activeBenchId !== record.data.benchId) || + (record.type === 'bench:summary' && activeBenchId !== undefined)) { + throw new ERR_INVALID_ARG_VALUE( + 'benchmark child message', message, + 'does not have a valid lifecycle sequence'); + } + if (record.type !== 'bench:plan' && + record.type !== 'bench:diagnostic') { + plansComplete = true; + } + if (record.type === 'bench:start') { + activeBenchId = record.data.benchId; + } else if (record.type === 'bench:complete' && + activeBenchId === record.data.benchId) { + activeBenchId = undefined; + } + if (record.type === 'bench:summary') summaryReceived = true; + record.data.runId = options.runId; + record.data.fileRunId = scope.fileRunId; + if (record.data.entryFile !== null) { + record.data.entryFile = scope.entryFile; + } + const pending = handleRecord(record); + if (protocolError === undefined) { + const acknowledged = PromisePrototypeThen( + PromiseResolve(pending), () => sendAck(child, message.id)); + trackPending(PromisePrototypeThen(acknowledged, () => { + recordPending = false; + })); + } + } catch (error) { + if (!aborted) protocolError ??= error; + terminateChild(); } - } catch (error) { - protocolError ??= error; - child.kill(); + }); + } catch (error) { + terminateChild(); + try { + await closed.promise; + } catch { + // Preserve the setup error. } - }); - const { 0: code, 1: signal } = await once(child, 'close'); + childClosed = true; + if (forceKillTimer !== undefined) clearTimeout(forceKillTimer); + abortListener?.[SymbolDispose](); + options.output?.removeListener('close', closeListener); + throw error; + } + let status; + try { + status = await closed.promise; + } finally { + childClosed = true; + if (forceKillTimer !== undefined) clearTimeout(forceKillTimer); + abortListener?.[SymbolDispose](); + options.output?.removeListener('close', closeListener); + } + const { 0: code, 1: signal } = status; await SafePromiseAllReturnVoid(ArrayFrom(pendingRecords)); - if (protocolError !== undefined) throw protocolError; - return { __proto__: null, code, signal }; + if (!aborted && spawnError !== undefined) throw spawnError; + if (!aborted && protocolError !== undefined) throw protocolError; + return { + __proto__: null, + aborted, + code, + error: abortError, + signal, + }; } async function runIsolated(files, options, output) { @@ -749,6 +990,13 @@ async function runIsolated(files, options, output) { for (let i = 0; i < files.length; i++) { const path = files[i]; const scope = options.fileScopes[i]; + const observedCounts = { + __proto__: null, + completed: 0, + failed: 0, + skipped: 0, + total: 0, + }; let childSummary; let result; try { @@ -757,6 +1005,16 @@ async function runIsolated(files, options, output) { childSummary = record.data; return; } + if (record.type === 'bench:plan') observedCounts.total++; + if (record.type === 'bench:complete') { + if (ObjectPrototypeHasOwnProperty(record.data, 'error')) { + observedCounts.failed++; + } else if (ObjectPrototypeHasOwnProperty(record.data, 'skip')) { + observedCounts.skipped++; + } else { + observedCounts.completed++; + } + } return emitRecordAndWait(output, record); }); } catch (error) { @@ -774,18 +1032,49 @@ async function runIsolated(files, options, output) { file: path, }, }); + counts.completed += observedCounts.completed; + counts.failed += observedCounts.failed; + counts.skipped += observedCounts.skipped; + counts.total += MathMax( + observedCounts.total, + observedCounts.completed + observedCounts.failed + + observedCounts.skipped); continue; } + if (result.aborted) { + success = false; + await emitRecordAndWait(output, { + __proto__: null, + type: 'bench:diagnostic', + data: { + __proto__: null, + runId: options.runId, + ...scope, + message: result.error.message, + error: result.error, + level: 'error', + file: path, + }, + }); + } if (childSummary !== undefined) { success &&= childSummary.success; counts.completed += childSummary.counts.completed; counts.failed += childSummary.counts.failed; counts.skipped += childSummary.counts.skipped; counts.total += childSummary.counts.total; + } else { + counts.completed += observedCounts.completed; + counts.failed += observedCounts.failed; + counts.skipped += observedCounts.skipped; + counts.total += MathMax( + observedCounts.total, + observedCounts.completed + observedCounts.failed + + observedCounts.skipped); } - if (childSummary === undefined || result.code !== 0 || - result.signal !== null) { + if (!result.aborted && (childSummary === undefined || result.code !== 0 || + result.signal !== null)) { success = false; if (childSummary === undefined || childSummary.success) { const status = result.signal === null ? @@ -812,7 +1101,8 @@ async function runIsolated(files, options, output) { runId: options.runId, fileRunId: scope?.fileRunId ?? null, entryFile: scope?.entryFile ?? null, - success: success && (process.exitCode ?? 0) === 0, + success: success && (options.useProcessExitCode === false || + (process.exitCode ?? 0) === 0), counts, duration_ns: hrtime() - start, file: files.length === 1 ? resolve(options.cwd, files[0]) : null, @@ -825,6 +1115,124 @@ async function runIsolated(files, options, output) { return summary; } +function runFile(path, options = kEmptyObject) { + validateStringWithoutNullBytes(path, 'path'); + if (!isAbsolute(path)) { + throw new ERR_INVALID_ARG_VALUE('path', path, 'must be an absolute path'); + } + const file = resolve(path); + validateObject(options, 'options'); + const { + env = process.env, + execArgv, + signal, + } = options; + let childExecArgv; + if (execArgv !== undefined) { + validateArray(execArgv, 'options.execArgv'); + childExecArgv = []; + const length = execArgv.length; + for (let i = 0; i < length; i++) { + const arg = execArgv[i]; + validateStringWithoutNullBytes(arg, `options.execArgv[${i}]`); + if (!StringPrototypeStartsWith(arg, '-') || arg === '-' || arg === '--') { + throw new ERR_INVALID_ARG_VALUE( + `options.execArgv[${i}]`, arg, + 'must be a Node.js command-line option'); + } + if (kIncompatibleExecArgv.has(getRunFileOptionName(arg))) { + throw new ERR_INVALID_ARG_VALUE( + `options.execArgv[${i}]`, arg, + 'is not compatible with benchmark execution'); + } + if (runFileOptionRequiresValue(arg)) { + const equals = StringPrototypeIndexOf(arg, '='); + if (equals !== -1) { + throw new ERR_INVALID_ARG_VALUE( + `options.execArgv[${i}]`, arg, + 'must include a non-empty value'); + } + if (i + 1 >= length) { + throw new ERR_INVALID_ARG_VALUE( + `options.execArgv[${i}]`, arg, + 'must be followed by a value'); + } + ArrayPrototypePush(childExecArgv, arg); + i++; + const value = execArgv[i]; + validateStringWithoutNullBytes(value, `options.execArgv[${i}]`); + if (value.length === 0) { + throw new ERR_INVALID_ARG_VALUE( + `options.execArgv[${i}]`, value, + 'must not be empty'); + } + ArrayPrototypePush(childExecArgv, value); + continue; + } + if (!StringPrototypeStartsWith(arg, '--') && + StringPrototypeIndexOf(arg, '=') !== -1) { + throw new ERR_INVALID_ARG_VALUE( + `options.execArgv[${i}]`, arg, + 'must not use = with a short option'); + } + ArrayPrototypePush(childExecArgv, arg); + } + if (ArrayPrototypeFilter(childExecArgv, filterExecArgv).length !== + childExecArgv.length) { + throw new ERR_INVALID_ARG_VALUE( + 'options.execArgv', childExecArgv, + 'must not contain benchmark runner options'); + } + } else { + childExecArgv = ArrayPrototypeFilter( + getInheritedChildArgs(), filterRunFileExecArgv); + } + validateObject(env, 'options.env'); + const childEnv = { __proto__: null }; + const envKeys = ObjectKeys(env); + for (let i = 0; i < envKeys.length; i++) { + const key = envKeys[i]; + validateStringWithoutNullBytes(key, 'options.env key'); + const value = env[key]; + if (value === undefined) continue; + validateStringWithoutNullBytes(value, `options.env.${key}`); + if (kIPCEnvironmentVariables.has(StringPrototypeToUpperCase(key))) continue; + childEnv[key] = value; + } + validateAbortSignal(signal, 'options.signal'); + if (signal !== undefined && + (typeof signal.addEventListener !== 'function' || + typeof signal.removeEventListener !== 'function')) { + throw new ERR_INVALID_ARG_TYPE('options.signal', 'AbortSignal', signal); + } + + const runId = createRunId(); + const scope = { + __proto__: null, + entryFile: file, + fileRunId: createRunId(), + }; + const output = new BenchmarksStream(); + const runOptions = { + __proto__: null, + cwd: process.cwd(), + env: childEnv, + execPath: process.execPath, + execArgv: childExecArgv, + fileScopes: [scope], + namePatternSource: '', + output, + runId, + signal, + useProcessExitCode: false, + }; + const execution = PromisePrototypeThen(PromiseResolve(), () => + (output.destroyed ? undefined : runIsolated([file], runOptions, output))); + PromisePrototypeThen(execution, () => output.end(), + (error) => output.destroy(error)); + return output; +} + async function run(patterns) { const options = parseCommandLine(); const files = options.isChild ? @@ -883,4 +1291,4 @@ async function run(patterns) { return summary; } -module.exports = { run }; +module.exports = { run, runFile }; diff --git a/lib/internal/bench_runner/runner.js b/lib/internal/bench_runner/runner.js index fc8595d2a35d..c48a0a5339fa 100644 --- a/lib/internal/bench_runner/runner.js +++ b/lib/internal/bench_runner/runner.js @@ -10,7 +10,12 @@ function run(options = kEmptyObject) { return runBenchmarks(options); } +function runFile(path, options = kEmptyObject) { + return require('internal/bench_runner/cli').runFile(path, options); +} + module.exports = { createRunner, run, + runFile, }; diff --git a/test/fixtures/bench-runner/run-file-blocked.cjs b/test/fixtures/bench-runner/run-file-blocked.cjs new file mode 100644 index 000000000000..9298d0dccc4b --- /dev/null +++ b/test/fixtures/bench-runner/run-file-blocked.cjs @@ -0,0 +1,7 @@ +'use strict'; + +const { bench } = require('node:bench'); + +bench('blocked run file', { samples: 1 }, async () => { + await new Promise(() => {}); +}); diff --git a/test/fixtures/bench-runner/run-file-lingering.cjs b/test/fixtures/bench-runner/run-file-lingering.cjs new file mode 100644 index 000000000000..954bc108d01d --- /dev/null +++ b/test/fixtures/bench-runner/run-file-lingering.cjs @@ -0,0 +1,11 @@ +'use strict'; + +const { bench } = require('node:bench'); + +setInterval(() => {}, 1_000); +if (process.platform !== 'win32') process.on('SIGTERM', () => {}); +process.on('disconnect', () => process.stdout.write('child disconnected\n')); + +bench('lingering run file', { samples: 1 }, (b) => { + b.record({ duration_ns: 1n, operations: 1 }); +}); diff --git a/test/fixtures/bench-runner/run-file-partial.cjs b/test/fixtures/bench-runner/run-file-partial.cjs new file mode 100644 index 000000000000..9e3019e94e62 --- /dev/null +++ b/test/fixtures/bench-runner/run-file-partial.cjs @@ -0,0 +1,11 @@ +'use strict'; + +const { bench } = require('node:bench'); + +bench('completed before abort', { samples: 1 }, (b) => { + b.record({ duration_ns: 1n, operations: 1 }); +}); + +bench('aborted run file', { samples: 1 }, async () => { + await new Promise(() => {}); +}); diff --git a/test/fixtures/bench-runner/run-file.cjs b/test/fixtures/bench-runner/run-file.cjs new file mode 100644 index 000000000000..382af4cc42f1 --- /dev/null +++ b/test/fixtures/bench-runner/run-file.cjs @@ -0,0 +1,21 @@ +'use strict'; + +const { bench } = require('node:bench'); + +bench('run file', { + samples: 1, + params: { + context: process.env.NODE_BENCH_CONTEXT, + exposed: typeof globalThis.gc === 'function', + value: process.env.NODE_BENCH_RUN_FILE ?? 'unset', + }, +}, (b) => { + b.record({ + duration_ns: 1n, + operations: 1, + detail: { + execArgv: process.execArgv, + pid: process.pid, + }, + }); +}); diff --git a/test/parallel/test-bench-cli.js b/test/parallel/test-bench-cli.js index bd67b4b45bdb..d0f9629ad9a3 100644 --- a/test/parallel/test-bench-cli.js +++ b/test/parallel/test-bench-cli.js @@ -709,6 +709,8 @@ if (common.hasInspector) { ]); assert.strictEqual(result.status, 1); const records = parseRecords(result); + assert(records.some(({ type, data }) => + type === 'bench:diagnostic' && /set exit code/.test(data.message))); assert.strictEqual(records.at(-1).data.success, false); } diff --git a/test/parallel/test-bench-run-file.js b/test/parallel/test-bench-run-file.js new file mode 100644 index 000000000000..eccbf2e72960 --- /dev/null +++ b/test/parallel/test-bench-run-file.js @@ -0,0 +1,318 @@ +// Flags: --no-warnings +'use strict'; + +const common = require('../common'); +const fixtures = require('../common/fixtures'); +const assert = require('assert'); +const { spawnSync } = require('child_process'); +const { once } = require('events'); +const path = require('path'); +const { runFile } = require('node:bench'); + +const fixture = fixtures.path('bench-runner/run-file.cjs'); + +assert.throws(() => runFile(null), { code: 'ERR_INVALID_ARG_TYPE' }); +assert.throws(() => runFile('relative.cjs'), { code: 'ERR_INVALID_ARG_VALUE' }); +assert.throws(() => runFile(fixture, null), { code: 'ERR_INVALID_ARG_TYPE' }); +assert.throws(() => runFile(fixture, { execArgv: null }), { + code: 'ERR_INVALID_ARG_TYPE', +}); +assert.throws(() => runFile(fixture, { execArgv: [1] }), { + code: 'ERR_INVALID_ARG_TYPE', +}); +assert.throws(() => runFile(fixture, { execArgv: ['--bench'] }), { + code: 'ERR_INVALID_ARG_VALUE', +}); +assert.throws(() => runFile(fixture, { execArgv: [fixture] }), { + code: 'ERR_INVALID_ARG_VALUE', +}); +assert.throws(() => runFile(fixture, { execArgv: ['-e', '0'] }), { + code: 'ERR_INVALID_ARG_VALUE', +}); +assert.throws(() => runFile(fixture, { execArgv: ['--require'] }), { + code: 'ERR_INVALID_ARG_VALUE', +}); +assert.throws(() => runFile(fixture, { execArgv: ['--require='] }), { + code: 'ERR_INVALID_ARG_VALUE', +}); +assert.throws(() => runFile(fixture, { execArgv: [`-r=${fixture}`] }), { + code: 'ERR_INVALID_ARG_VALUE', +}); +assert.throws(() => runFile(fixture, { execArgv: ['--prof-process'] }), { + code: 'ERR_INVALID_ARG_VALUE', +}); +assert.throws(() => runFile(fixture, { execArgv: ['--prof_process'] }), { + code: 'ERR_INVALID_ARG_VALUE', +}); +assert.throws(() => runFile(fixture, { + execArgv: ['--bench_name_pattern=run'], +}), { code: 'ERR_INVALID_ARG_VALUE' }); +assert.throws(() => runFile(`${fixture}\0`), { + code: 'ERR_INVALID_ARG_VALUE', +}); +assert.throws(() => runFile(fixture, { env: null }), { + code: 'ERR_INVALID_ARG_TYPE', +}); +assert.throws(() => runFile(fixture, { env: { INVALID: 1 } }), { + code: 'ERR_INVALID_ARG_TYPE', +}); +assert.throws(() => runFile(fixture, { env: { INVALID: 'x\0' } }), { + code: 'ERR_INVALID_ARG_VALUE', +}); +assert.throws(() => runFile(fixture, { env: { NODE_CHANNEL_FD: 1 } }), { + code: 'ERR_INVALID_ARG_TYPE', +}); +assert.throws(() => runFile(fixture, { signal: {} }), { + code: 'ERR_INVALID_ARG_TYPE', +}); +assert.throws(() => runFile(fixture, { signal: { aborted: false } }), { + code: 'ERR_INVALID_ARG_TYPE', +}); + +async function testRunFile() { + const execArgv = ['--expose-gc', '-r', 'fs']; + const env = { + __proto__: null, + ...process.env, + NODE_BENCH_CONTEXT: 'not-a-child', + NODE_BENCH_RUN_FILE: 'original', + NODE_CHANNEL_FD: '999', + NODE_CHANNEL_SERIALIZATION_MODE: 'json', + }; + const stream = runFile(fixture, { env, execArgv }); + execArgv.length = 0; + env.NODE_BENCH_RUN_FILE = 'mutated'; + const records = await stream.toArray(); + const plan = records.find(({ type }) => type === 'bench:plan').data; + const result = records.find(({ type }) => type === 'bench:complete').data; + const summary = records.at(-1).data; + + assert.strictEqual(records.at(-1).type, 'bench:summary'); + assert.strictEqual(plan.params.context, 'child'); + assert.strictEqual(plan.params.exposed, true); + assert.strictEqual(plan.params.value, 'original'); + assert.strictEqual(result.error, undefined); + assert.strictEqual(result.samples.length, 1); + assert.strictEqual(typeof result.samples[0].duration_ns, 'bigint'); + assert.notStrictEqual(result.samples[0].detail.pid, process.pid); + assert(result.samples[0].detail.execArgv.includes('--expose-gc')); + assert(result.samples[0].detail.execArgv.includes('-r')); + assert.strictEqual(summary.success, true); + assert.strictEqual(summary.file, fixture); + assert.strictEqual(summary.entryFile, fixture); + assert.strictEqual(summary.fileRunId, result.fileRunId); + assert.strictEqual(summary.runId, result.runId); + assert.strictEqual(records.filter( + ({ type }) => type === 'bench:summary').length, 1); +} + +async function testConcurrentCalls() { + const [cjsRecords, esmRecords] = await Promise.all([ + runFile(fixtures.path('bench-runner/a.cjs')).toArray(), + runFile(fixtures.path('bench-runner/b.mjs')).toArray(), + ]); + const cjsResult = cjsRecords.find( + ({ type }) => type === 'bench:complete').data; + const esmResult = esmRecords.find( + ({ type }) => type === 'bench:complete').data; + assert.strictEqual(cjsResult.name, 'alpha'); + assert.strictEqual(esmResult.name, 'beta'); + assert.notStrictEqual(cjsResult.params.pid, esmResult.params.pid); + assert.notStrictEqual(cjsResult.runId, esmResult.runId); +} + +async function testLoadFailure() { + const missing = path.resolve(fixtures.fixturesDir, 'does-not-exist.cjs'); + const records = await runFile(missing).toArray(); + const diagnostics = records.filter( + ({ type }) => type === 'bench:diagnostic'); + assert(diagnostics.some( + ({ data }) => data.level === 'error' && /failed with exit code/.test( + data.message))); + assert.strictEqual(records.some( + ({ type }) => type === 'bench:complete'), false); + assert.strictEqual(records.at(-1).type, 'bench:summary'); + assert.strictEqual(records.at(-1).data.success, false); +} + +async function testEvaluationFailure() { + const records = await runFile(fixtures.path( + 'bench-runner/load-error-after-declaration.cjs')).toArray(); + assert(records.some(({ type, data }) => + type === 'bench:diagnostic' && + data.message === 'load failed after declaration')); + const result = records.find( + ({ type }) => type === 'bench:complete').data; + assert.strictEqual(result.name, 'declared before load error'); + assert.strictEqual(result.error, undefined); + assert.strictEqual(records.at(-1).data.counts.completed, 1); + assert.strictEqual(records.at(-1).data.success, false); +} + +async function testStructuredError() { + const records = await runFile( + fixtures.path('bench-runner/error.cjs')).toArray(); + const result = records.find( + ({ type }) => type === 'bench:complete').data; + assert.strictEqual(result.error.code, 'ERR_BENCHMARK_FIXTURE'); + assert.deepStrictEqual(result.error.cause, { value: 42n }); + assert.match(result.error.stack, /error\.cjs/); + assert.strictEqual(records.at(-1).data.success, false); +} + +async function testAbort() { + const controller = new AbortController(); + const reason = new Error('cancel run file'); + const stream = runFile( + fixtures.path('bench-runner/run-file-blocked.cjs'), + { signal: controller.signal }); + stream.once('bench:start', common.mustCall(() => controller.abort(reason))); + const records = await stream.toArray(); + const diagnostic = records.find(({ type, data }) => + type === 'bench:diagnostic' && data.error?.code === 'ABORT_ERR').data; + assert.strictEqual(diagnostic.error.cause.message, reason.message); + assert.strictEqual(records.some( + ({ type }) => type === 'bench:complete'), false); + assert.strictEqual(records.at(-1).type, 'bench:summary'); + assert.strictEqual(records.at(-1).data.success, false); + assert.deepStrictEqual(records.at(-1).data.counts, { + __proto__: null, + completed: 0, + failed: 0, + skipped: 0, + total: 1, + }); +} + +async function testPartialAbort() { + const controller = new AbortController(); + const stream = runFile( + fixtures.path('bench-runner/run-file-partial.cjs'), + { signal: controller.signal }); + stream.on('bench:start', ({ name }) => { + if (name === 'aborted run file') controller.abort(); + }); + const records = await stream.toArray(); + const summary = records.at(-1).data; + assert.strictEqual(records.filter( + ({ type }) => type === 'bench:complete').length, 1); + assert.deepStrictEqual(summary.counts, { + __proto__: null, + completed: 1, + failed: 0, + skipped: 0, + total: 2, + }); + assert.strictEqual(summary.success, false); +} + +async function testPostSummaryAbort() { + const controller = new AbortController(); + const stream = runFile( + fixtures.path('bench-runner/run-file-lingering.cjs'), + { signal: controller.signal }); + stream.on('bench:diagnostic', common.mustCall(({ message, stream }) => { + if (stream === 'stdout' && /child disconnected/.test(message)) { + controller.abort(new Error('stop lingering child')); + } + }, 2)); + const records = await stream.toArray(); + assert(records.some(({ type, data }) => + type === 'bench:diagnostic' && data.error?.code === 'ABORT_ERR')); + assert.strictEqual(records.at(-1).type, 'bench:summary'); + assert.strictEqual(records.at(-1).data.counts.completed, 1); + assert.strictEqual(records.at(-1).data.success, false); +} + +async function testExitCode() { + const records = await runFile( + fixtures.path('bench-runner/exit-code.cjs')).toArray(); + assert(records.some(({ type, data }) => + type === 'bench:diagnostic' && + /set exit code/.test(data.message))); + assert.strictEqual(records.at(-1).data.success, false); +} + +async function testParentExitCode() { + process.exitCode = 42; + try { + const records = await runFile(fixture).toArray(); + assert.strictEqual(records.at(-1).data.success, true); + } finally { + process.exitCode = undefined; + } +} + +async function testDefaultExecArgvSnapshot() { + const stream = runFile(fixture); + process.execArgv.push('--require=/does/not/exist.cjs'); + try { + const records = await stream.toArray(); + assert.strictEqual(records.at(-1).data.success, true); + } finally { + process.execArgv.pop(); + } +} + +async function testExecPathSnapshot() { + const stream = runFile(fixture); + const execPath = process.execPath; + process.execPath = '/does/not/exist'; + try { + const records = await stream.toArray(); + assert.strictEqual(records.at(-1).data.success, true); + } finally { + process.execPath = execPath; + } +} + +function testEvalParent() { + const script = ` + require('node:bench').runFile(${JSON.stringify(fixture)}) + .on('bench:summary', (summary) => console.log(summary.success)); + `; + const result = spawnSync(process.execPath, [ + '--no-warnings', + '-e', + script, + ], { encoding: 'utf8' }); + assert.strictEqual(result.status, 0, result.stderr); + assert.strictEqual(result.stdout, 'true\n'); +} + +async function testPreAborted() { + const records = await runFile(fixture, { + signal: AbortSignal.abort(new Error('already cancelled')), + }).toArray(); + assert.strictEqual(records.some(({ type }) => type === 'bench:start'), false); + assert.strictEqual(records.find(({ type }) => + type === 'bench:diagnostic').data.error.code, 'ABORT_ERR'); + assert.strictEqual(records.at(-1).type, 'bench:summary'); + assert.strictEqual(records.at(-1).data.success, false); +} + +async function testDestroy() { + const stream = runFile( + fixtures.path('bench-runner/run-file-blocked.cjs')); + stream.once('bench:start', common.mustCall(() => stream.destroy())); + await once(stream, 'close'); + assert.strictEqual(stream.destroyed, true); +} + +(async () => { + await testRunFile(); + await testConcurrentCalls(); + await testLoadFailure(); + await testEvaluationFailure(); + await testStructuredError(); + await testAbort(); + await testPartialAbort(); + await testPostSummaryAbort(); + await testExitCode(); + await testParentExitCode(); + await testDefaultExecArgvSnapshot(); + await testExecPathSnapshot(); + await testPreAborted(); + await testDestroy(); + testEvalParent(); +})().then(common.mustCall()); From d6a6edd2c7f4da9769c36ed46eba84e8fd45a629 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 29 Aug 2026 14:36:15 +0000 Subject: [PATCH 10/14] lib: have runFile honor permissions and accept URL/Buffer paths Signed-off-by: James M Snell Assisted-by: Opencode --- doc/api/bench.md | 24 +++++--- lib/internal/bench_runner/cli.js | 24 ++++++-- test/parallel/test-bench-run-file.js | 91 ++++++++++++++++++++++++++-- 3 files changed, 120 insertions(+), 19 deletions(-) diff --git a/doc/api/bench.md b/doc/api/bench.md index 2eb6eb6a3822..96a5aade5a41 100644 --- a/doc/api/bench.md +++ b/doc/api/bench.md @@ -504,7 +504,7 @@ for await (const { type, data } of run()) { added: REPLACEME --> -* `path` {string} The absolute path of one benchmark module. +* `path` {string|Buffer|URL} The path of one benchmark module. * `options` {Object} * `env` {Object} The child process environment. Property values must be strings or `undefined`. This replaces, rather than extends, the parent @@ -518,18 +518,22 @@ added: REPLACEME * Returns: {BenchmarksStream} Runs exactly one benchmark module in a fresh child process and returns its -object-mode event stream. `path` is not interpreted as a glob. Unless the signal -is aborted or the stream is destroyed before startup, every call uses a new -child. Input discovery, ordering, concurrency, retries, and multi-file -scheduling remain the caller's responsibility. +object-mode event stream. A relative `path` is resolved from the current working +directory when `runFile()` is called. `path` is not interpreted as a glob. +Unless the signal is aborted or the stream is destroyed before startup, every +call uses a new child. Input discovery, ordering, concurrency, retries, and +multi-file scheduling remain the caller's responsibility. + +When the Permission Model is enabled, the caller must have file system read +access to `path` and permission to create child processes. Records use advanced child process serialization, preserving supported structured values such as `bigint` and errors. Child writes to stdout and stderr -become `'bench:diagnostic'` records. A module loading error, abnormal child exit, -or cancellation also emits an error diagnostic and produces a terminal -`'bench:summary'` whose `success` property is `false`; these execution failures -do not error the stream. If module evaluation fails after declaring benchmarks, -those declarations still run before the unsuccessful summary. +become `'bench:diagnostic'` records. A permission failure, module loading error, +abnormal child exit, or cancellation also emits an error diagnostic and produces +a terminal `'bench:summary'` whose `success` property is `false`; these execution +failures do not error the stream. If module evaluation fails after declaring +benchmarks, those declarations still run before the unsuccessful summary. `env`, effective inherited options, and an explicitly provided `execArgv` are copied when `runFile()` is called. The runner removes `NODE_OPTIONS`, replaces diff --git a/lib/internal/bench_runner/cli.js b/lib/internal/bench_runner/cli.js index 4b14493e391d..8e3d62257b62 100644 --- a/lib/internal/bench_runner/cli.js +++ b/lib/internal/bench_runner/cli.js @@ -36,10 +36,14 @@ const { StringPrototypeStartsWith, StringPrototypeToUpperCase, SymbolDispose, + uncurryThis, } = primordials; +const { Buffer } = require('buffer'); +const BufferToString = uncurryThis(Buffer.prototype.toString); const { spawn } = require('child_process'); const { createWriteStream, statSync } = require('fs'); const { Glob } = require('internal/fs/glob'); +const { getValidatedPath } = require('internal/fs/utils'); const { BenchmarksStream, } = require('internal/bench_runner/benchmarks_stream'); @@ -53,6 +57,7 @@ const { deserializeError, serializeError } = require('internal/error_serdes'); const { AbortError, codes: { + ERR_ACCESS_DENIED, ERR_INVALID_ARG_TYPE, ERR_INVALID_ARG_VALUE, ERR_INVALID_STATE, @@ -64,6 +69,7 @@ const { getOptionValue, getOptionsAsFlagsFromBinding, } = require('internal/options'); +const permission = require('internal/process/permission'); const { TIMEOUT_MAX } = require('internal/timers'); const { kEmptyObject } = require('internal/util'); const { @@ -75,7 +81,7 @@ const { } = require('internal/validators'); const { pathToFileURL } = require('internal/url'); const { pipeline } = require('stream/promises'); -const { isAbsolute, resolve, sep } = require('path'); +const { resolve, sep } = require('path'); const { clearTimeout, setTimeout } = require('timers'); const console = require('internal/console/global'); @@ -758,6 +764,16 @@ async function runChild(path, options, scope, onRecord) { }), }; } + const resource = resolve(options.cwd, path); + if (permission.isEnabled() && + !permission.has('fs.read', resource) && + !permission.isAuditMode()) { + throw new ERR_ACCESS_DENIED( + 'Access to this API has been restricted. Use --allow-fs-read to manage permissions.', + 'FileSystemRead', + resource, + ); + } const child = spawn( options.execPath ?? process.execPath, getChildArgs(path, options), @@ -1116,10 +1132,8 @@ async function runIsolated(files, options, output) { } function runFile(path, options = kEmptyObject) { - validateStringWithoutNullBytes(path, 'path'); - if (!isAbsolute(path)) { - throw new ERR_INVALID_ARG_VALUE('path', path, 'must be an absolute path'); - } + path = getValidatedPath(path); + if (typeof path !== 'string') path = BufferToString(path); const file = resolve(path); validateObject(options, 'options'); const { diff --git a/test/parallel/test-bench-run-file.js b/test/parallel/test-bench-run-file.js index eccbf2e72960..600f1a212ae5 100644 --- a/test/parallel/test-bench-run-file.js +++ b/test/parallel/test-bench-run-file.js @@ -10,9 +10,9 @@ const path = require('path'); const { runFile } = require('node:bench'); const fixture = fixtures.path('bench-runner/run-file.cjs'); +const relativeFixture = path.relative(process.cwd(), fixture); assert.throws(() => runFile(null), { code: 'ERR_INVALID_ARG_TYPE' }); -assert.throws(() => runFile('relative.cjs'), { code: 'ERR_INVALID_ARG_VALUE' }); assert.throws(() => runFile(fixture, null), { code: 'ERR_INVALID_ARG_TYPE' }); assert.throws(() => runFile(fixture, { execArgv: null }), { code: 'ERR_INVALID_ARG_TYPE', @@ -50,6 +50,9 @@ assert.throws(() => runFile(fixture, { assert.throws(() => runFile(`${fixture}\0`), { code: 'ERR_INVALID_ARG_VALUE', }); +assert.throws(() => runFile(Buffer.from(`${fixture}\0`)), { + code: 'ERR_INVALID_ARG_VALUE', +}); assert.throws(() => runFile(fixture, { env: null }), { code: 'ERR_INVALID_ARG_TYPE', }); @@ -79,7 +82,7 @@ async function testRunFile() { NODE_CHANNEL_FD: '999', NODE_CHANNEL_SERIALIZATION_MODE: 'json', }; - const stream = runFile(fixture, { env, execArgv }); + const stream = runFile(relativeFixture, { env, execArgv }); execArgv.length = 0; env.NODE_BENCH_RUN_FILE = 'mutated'; const records = await stream.toArray(); @@ -108,8 +111,11 @@ async function testRunFile() { async function testConcurrentCalls() { const [cjsRecords, esmRecords] = await Promise.all([ - runFile(fixtures.path('bench-runner/a.cjs')).toArray(), - runFile(fixtures.path('bench-runner/b.mjs')).toArray(), + runFile(fixtures.fileURL('bench-runner/a.cjs')).toArray(), + runFile(Buffer.from(path.relative( + process.cwd(), + fixtures.path('bench-runner/b.mjs'), + ))).toArray(), ]); const cjsResult = cjsRecords.find( ({ type }) => type === 'bench:complete').data; @@ -119,6 +125,14 @@ async function testConcurrentCalls() { assert.strictEqual(esmResult.name, 'beta'); assert.notStrictEqual(cjsResult.params.pid, esmResult.params.pid); assert.notStrictEqual(cjsResult.runId, esmResult.runId); + assert.strictEqual( + cjsRecords.at(-1).data.file, + fixtures.path('bench-runner/a.cjs'), + ); + assert.strictEqual( + esmRecords.at(-1).data.file, + fixtures.path('bench-runner/b.mjs'), + ); } async function testLoadFailure() { @@ -280,6 +294,74 @@ function testEvalParent() { assert.strictEqual(result.stdout, 'true\n'); } +function testPermissions() { + const fsReadScript = ` + const assert = require('assert'); + const { runFile } = require('node:bench'); + const { pathToFileURL } = require('url'); + const target = ${JSON.stringify(fixture)}; + assert.strictEqual(process.permission.has('child'), true); + assert.strictEqual(process.permission.has('fs.read', target), false); + Promise.all([ + target, + Buffer.from(target), + pathToFileURL(target), + ].map(async (input) => { + const records = await runFile(input).toArray(); + const diagnostic = records.find(({ type, data }) => + type === 'bench:diagnostic' && + data.error?.code === 'ERR_ACCESS_DENIED'); + assert.strictEqual(diagnostic.data.error.permission, 'FileSystemRead'); + assert.strictEqual(diagnostic.data.error.resource, target); + assert.strictEqual(records.at(-1).type, 'bench:summary'); + assert.strictEqual(records.at(-1).data.success, false); + })).catch((error) => { + console.error(error); + process.exitCode = 1; + }); + `; + const result = spawnSync(process.execPath, [ + '--no-warnings', + '--permission', + '--allow-child-process', + '-e', + fsReadScript, + ], { encoding: 'utf8' }); + assert.strictEqual(result.status, 0, result.stderr); + + const childProcessScript = ` + const assert = require('assert'); + const { runFile } = require('node:bench'); + const target = ${JSON.stringify(fixture)}; + assert.strictEqual(process.permission.has('fs.read', target), true); + assert.strictEqual(process.permission.has('child'), false); + runFile(target).toArray().then((records) => { + const diagnostic = records.find(({ type, data }) => + type === 'bench:diagnostic' && + data.error?.code === 'ERR_ACCESS_DENIED'); + assert.strictEqual(diagnostic.data.error.permission, 'ChildProcess'); + assert.strictEqual(diagnostic.data.error.resource, process.execPath); + assert.strictEqual(records.at(-1).type, 'bench:summary'); + assert.strictEqual(records.at(-1).data.success, false); + }).catch((error) => { + console.error(error); + process.exitCode = 1; + }); + `; + const childProcessResult = spawnSync(process.execPath, [ + '--no-warnings', + '--permission', + `--allow-fs-read=${fixture}`, + '-e', + childProcessScript, + ], { encoding: 'utf8' }); + assert.strictEqual( + childProcessResult.status, + 0, + childProcessResult.stderr, + ); +} + async function testPreAborted() { const records = await runFile(fixture, { signal: AbortSignal.abort(new Error('already cancelled')), @@ -315,4 +397,5 @@ async function testDestroy() { await testPreAborted(); await testDestroy(); testEvalParent(); + testPermissions(); })().then(common.mustCall()); From c79c6a5d37dedc9b71a399ee05181db836d9b748 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 29 Aug 2026 15:11:16 +0000 Subject: [PATCH 11/14] lib: improve diagnostic message support Support serializable context.diagnostic messages and optionally listen to diagnostic_channel messages Signed-off-by: James M Snell Assisted-by: Opencode --- doc/api/bench.md | 25 +++- doc/api/cli.md | 4 +- doc/node.1 | 1 + lib/internal/bench_runner/benchmark.js | 48 ++++++- lib/internal/bench_runner/cli.js | 5 +- lib/internal/bench_runner/harness.js | 36 ++++++ test/fixtures/bench-runner/diagnostic.cjs | 16 ++- test/parallel/test-bench-cli.js | 9 +- test/parallel/test-bench-context-errors.js | 8 +- .../test-bench-diagnostic-channels.js | 119 ++++++++++++++++++ test/parallel/test-bench-diagnostics.js | 10 +- test/parallel/test-bench-validation.js | 4 + 12 files changed, 253 insertions(+), 32 deletions(-) create mode 100644 test/parallel/test-bench-diagnostic-channels.js diff --git a/doc/api/bench.md b/doc/api/bench.md index 96a5aade5a41..842e8a59264b 100644 --- a/doc/api/bench.md +++ b/doc/api/bench.md @@ -316,6 +316,9 @@ added: REPLACEME * `name` {string} The benchmark name. **Default:** The `name` property of `fn`, or `''` when `fn` has no name. * `options` {Object} + * `diagnosticChannels` {Array} String diagnostics channel names, deduplicated + and inherited from containing suites by union. Symbol values in the array + are silently ignored. **Default:** `[]`. * `only` {boolean} When any benchmark or containing suite has `only` set, benchmarks without `only` in their hierarchy are skipped. **Default:** `false`. @@ -346,6 +349,12 @@ samples, but their samples are discarded. An exception, rejection, timeout, abort, missing timing call, or duplicate timing call stops the current benchmark. Later benchmarks continue to run. +For each warmup and measured callback, the runner subscribes to the configured +diagnostics channels. Each publication queues a context diagnostic whose +`message` is `{ name, message }`, containing the string channel name and the +published message. Subscriptions are removed when the callback settles or is +aborted. + A timeout or abort cannot interrupt synchronous JavaScript and does not forcibly cancel asynchronous work that ignores `context.signal`. @@ -388,6 +397,9 @@ added: REPLACEME * `name` {string} The suite name. **Default:** The `name` property of `fn`, or `''` when `fn` has no name. * `options` {Object} + * `diagnosticChannels` {Array} String diagnostics channel names inherited by + nested suites and benchmarks. Symbol values in the array are silently + ignored. **Default:** `[]`. * `only` {boolean} Selects all benchmarks nested in this suite. **Default:** `false`. * `skip` {boolean|string} Skips all benchmarks nested in this suite. @@ -663,7 +675,8 @@ message transport from the duration. `record()` is mutually exclusive with added: REPLACEME --> -* `message` {string} The diagnostic message. +* `message` {any} A structured-cloneable diagnostic value. With CLI process + isolation, it must also be supported by advanced child process serialization. * `options` {Object} * `level` {string} Either `'info'` or `'warning'`. **Default:** `'info'`. * `detail` {any} Additional structured-cloneable diagnostic data. With CLI @@ -679,10 +692,10 @@ before a callback failure are emitted before the failed `'bench:complete'` event and do not themselves cause the benchmark to fail. If a timeout or abort wins before the callback settles, queued diagnostics might not be emitted. -The message and options are validated, and detail is cloned, synchronously. -Calling `diagnostic()` between `context.start()` and `context.end()` therefore -includes that work in the measured duration. Invalid arguments or an -uncloneable detail violate the sample contract. +The message and detail are cloned synchronously. Options are also validated +synchronously. Calling `diagnostic()` between `context.start()` and +`context.end()` therefore includes that work in the measured duration. Invalid +arguments or an uncloneable message or detail violate the sample contract. ### `context.done()` @@ -751,6 +764,8 @@ isolation, all files share one runner and their plans are emitted before any benchmark executes. Plan data contains the benchmark-scoped identity, location, tags, and parameters described in [benchmark result][], together with: +* `diagnosticChannels` {string\[]} The inherited string channel names + subscribed to during each callback. * `samples` {number} The effective maximum number of measured callback invocations after run-level overrides. * `warmup` {number} The effective number of unreported warmup callback diff --git a/doc/api/cli.md b/doc/api/cli.md index 747e9e0b5d41..2805a526de24 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -492,9 +492,7 @@ benchmark runner process. This reduces startup overhead but allows module, heap, and process state to carry between files. User writes to stdout or stderr also share destinations with benchmark reporters in this mode. -The supported modes are `'process'` and `'none'`. Worker-thread isolation is not -a CLI mode. Higher-level tools can implement it using externally measured -samples as described in the [benchmark runner][] documentation. +The supported modes are `'process'` and `'none'`. ### `--bench-name-pattern=pattern` diff --git a/doc/node.1 b/doc/node.1 index 7307ebf4abf3..0a52e0140b76 100644 --- a/doc/node.1 +++ b/doc/node.1 @@ -304,6 +304,7 @@ When \fBmode\fR is \fB'none'\fR, all matching files and benchmarks run serially benchmark runner process. This reduces startup overhead but allows module, heap, and process state to carry between files. User writes to stdout or stderr also share destinations with benchmark reporters in this mode. +The supported modes are \fB'process'\fR and \fB'none'\fR. . .It Fl -bench-name-pattern Ns = Ns Ar pattern Only runs benchmarks whose full hierarchical name matches the JavaScript diff --git a/lib/internal/bench_runner/benchmark.js b/lib/internal/bench_runner/benchmark.js index 93fe34595708..375f8e6b635f 100644 --- a/lib/internal/bench_runner/benchmark.js +++ b/lib/internal/bench_runner/benchmark.js @@ -48,6 +48,7 @@ const { structuredClone } = require('internal/worker/js_transferable'); const { bigint: hrtime } = process.hrtime; const kDefaultSamples = 30; const kDefaultWarmup = 0; +const kEmptyDiagnosticChannels = ObjectFreeze([]); const kEmptyNamePath = ObjectFreeze([]); const kEmptyParams = ObjectFreeze({ __proto__: null }); const kEmptyTags = ObjectFreeze([]); @@ -82,6 +83,33 @@ function canonicalizeTags(tags, parentTags = kEmptyTags) { return ObjectFreeze(result); } +function canonicalizeDiagnosticChannels( + diagnosticChannels, + parentDiagnosticChannels = kEmptyDiagnosticChannels, +) { + if (diagnosticChannels === undefined) return parentDiagnosticChannels; + if (!ArrayIsArray(diagnosticChannels)) { + throw new ERR_INVALID_ARG_TYPE( + 'options.diagnosticChannels', 'Array', diagnosticChannels); + } + + const result = ArrayPrototypeSlice(parentDiagnosticChannels); + const seen = new SafeSet(parentDiagnosticChannels); + for (let i = 0; i < diagnosticChannels.length; i++) { + const name = diagnosticChannels[i]; + if (typeof name === 'symbol') continue; + if (typeof name !== 'string') { + throw new ERR_INVALID_ARG_TYPE( + `options.diagnosticChannels[${i}]`, ['string', 'symbol'], name); + } + if (!seen.has(name)) { + seen.add(name); + ArrayPrototypePush(result, name); + } + } + return ObjectFreeze(result); +} + function canonicalizeParams(params) { if (params === undefined) return kEmptyParams; validateObject(params, 'options.params'); @@ -106,15 +134,17 @@ function canonicalizeParams(params) { return ObjectFreeze(result); } -function validateNodeOptions(options, parentTags) { +function validateNodeOptions(options, parentTags, parentDiagnosticChannels) { validateObject(options, 'options'); - const { only = false, skip, tags } = options; + const { diagnosticChannels, only = false, skip, tags } = options; if (typeof only !== 'boolean') { throw new ERR_INVALID_ARG_TYPE('options.only', 'boolean', only); } validateSkip(skip); return { __proto__: null, + diagnosticChannels: canonicalizeDiagnosticChannels( + diagnosticChannels, parentDiagnosticChannels), only, skip, tags: canonicalizeTags(tags, parentTags), @@ -157,7 +187,10 @@ class Suite extends AsyncResource { constructor(harness, parent, name, options, fn, loc, isRoot = false) { super('BenchSuite'); const validated = validateNodeOptions( - options, parent?.tags ?? kEmptyTags); + options, + parent?.tags ?? kEmptyTags, + parent?.diagnosticChannels ?? kEmptyDiagnosticChannels, + ); this.harness = harness; this.parent = parent; @@ -174,6 +207,7 @@ class Suite extends AsyncResource { this.namePath, ]); this.parentId = isRoot || parent.isRoot ? null : parent.suiteId; + this.diagnosticChannels = validated.diagnosticChannels; this.only = validated.only; this.skip = validated.skip; this.tags = validated.tags; @@ -195,7 +229,8 @@ class Suite extends AsyncResource { class Bench extends AsyncResource { constructor(harness, parent, name, options, fn, loc) { super('Benchmark'); - const validated = validateNodeOptions(options, parent.tags); + const validated = validateNodeOptions( + options, parent.tags, parent.diagnosticChannels); const { params, samples = kDefaultSamples, @@ -216,6 +251,7 @@ class Bench extends AsyncResource { this.name = name; this.fn = fn; this.loc = createLocation(loc, harness.entryFile); + this.diagnosticChannels = validated.diagnosticChannels; this.only = validated.only; this.skip = validated.skip; this.tags = validated.tags; @@ -275,7 +311,7 @@ class BenchContext { throw new ERR_INVALID_STATE('benchmark sample is no longer active'); } try { - validateString(message, 'message'); + const clonedMessage = structuredClone(message); validateObject(options, 'options'); const { detail, level = 'info' } = options; validateString(level, 'options.level'); @@ -283,7 +319,7 @@ class BenchContext { throw new ERR_INVALID_ARG_VALUE( 'options.level', level, "must be 'info' or 'warning'"); } - const diagnostic = { __proto__: null, level, message }; + const diagnostic = { __proto__: null, level, message: clonedMessage }; if (detail !== undefined) diagnostic.detail = structuredClone(detail); this.#onDiagnostic(diagnostic); } catch (error) { diff --git a/lib/internal/bench_runner/cli.js b/lib/internal/bench_runner/cli.js index 8e3d62257b62..0cc4dc5210a1 100644 --- a/lib/internal/bench_runner/cli.js +++ b/lib/internal/bench_runner/cli.js @@ -440,7 +440,8 @@ function validateRecord(record) { warmup, yieldBetweenSamples, } = record.data; - if (typeof record.data.file !== 'string' || + if (!isStringArray(record.data.diagnosticChannels) || + typeof record.data.file !== 'string' || !NumberIsSafeInteger(record.data.line) || record.data.line < 0 || !NumberIsSafeInteger(record.data.column) || record.data.column < 0 || !isStringArray(record.data.tags) || @@ -471,7 +472,7 @@ function validateRecord(record) { record.data.phase !== 'measurement') || !NumberIsSafeInteger(record.data.index) || record.data.index < 0 || record.data.index > 0xFFFFFFFF || - typeof record.data.message !== 'string' || + !ObjectPrototypeHasOwnProperty(record.data, 'message') || (record.data.level !== 'info' && record.data.level !== 'warning') || typeof record.data.file !== 'string' || !NumberIsSafeInteger(record.data.line) || record.data.line < 0 || diff --git a/lib/internal/bench_runner/harness.js b/lib/internal/bench_runner/harness.js index f3e28bc01b1a..c18f439f6719 100644 --- a/lib/internal/bench_runner/harness.js +++ b/lib/internal/bench_runner/harness.js @@ -24,6 +24,10 @@ const { const { getCallerLocation } = internalBinding('util'); const { exitCodes: { kGenericUserError } } = internalBinding('errors'); const { AsyncLocalStorage } = require('async_hooks'); +const { + subscribe: subscribeToChannel, + unsubscribe: unsubscribeFromChannel, +} = require('diagnostics_channel'); const { AbortController } = require('internal/abort_controller'); const { AbortError, @@ -406,6 +410,7 @@ class Harness { parentId: benchmark.parentId, name: benchmark.name, namePath: ArrayPrototypeSlice(benchmark.namePath), + diagnosticChannels: ArrayPrototypeSlice(benchmark.diagnosticChannels), file: benchmark.loc.file, line: benchmark.loc.line, column: benchmark.loc.column, @@ -720,9 +725,37 @@ class Harness { const context = new BenchContext( benchmark, signal, phase, index, (diagnostic) => ArrayPrototypePush(diagnostics, diagnostic)); + const channels = benchmark.diagnosticChannels; + let abortSubscription; + let diagnosticError; + let diagnosticFailed = false; + let subscribed = 0; + const onMessage = (message, name) => { + if (signal.aborted || diagnosticFailed) return; + try { + context.diagnostic({ __proto__: null, name, message }); + } catch (error) { + diagnosticError = error; + diagnosticFailed = true; + } + }; + const unsubscribe = () => { + while (subscribed > 0) { + subscribed--; + unsubscribeFromChannel(channels[subscribed], onMessage); + } + }; try { + for (let i = 0; i < channels.length; i++) { + subscribeToChannel(channels[i], onMessage); + subscribed++; + } + if (subscribed > 0) { + abortSubscription = addAbortListener(signal, unsubscribe); + } await this.#invoke( benchmark, benchmark, benchmark.fn, [context]); + if (diagnosticFailed) throw diagnosticError; const { done, sample } = context.finish(); return { __proto__: null, @@ -739,6 +772,9 @@ class Harness { error, failed: true, }; + } finally { + abortSubscription?.[SymbolDispose](); + unsubscribe(); } } diff --git a/test/fixtures/bench-runner/diagnostic.cjs b/test/fixtures/bench-runner/diagnostic.cjs index 018b8a31de42..9e413e943dd7 100644 --- a/test/fixtures/bench-runner/diagnostic.cjs +++ b/test/fixtures/bench-runner/diagnostic.cjs @@ -1,11 +1,17 @@ 'use strict'; const { bench } = require('node:bench'); +const { channel } = require('diagnostics_channel'); -bench('diagnostic relay', { samples: 1 }, (b) => { - b.diagnostic('relayed warning', { - detail: { value: 42n }, - level: 'warning', - }); +const channelName = 'node:bench:test:diagnostic'; +const diagnosticChannel = channel(channelName); + +bench('diagnostic relay', { + diagnosticChannels: [channelName], + samples: 1, +}, (b) => { + const message = { value: 42n }; + diagnosticChannel.publish(message); + message.value = 0n; b.record({ duration_ns: 1n, operations: 1 }); }); diff --git a/test/parallel/test-bench-cli.js b/test/parallel/test-bench-cli.js index d0f9629ad9a3..21090814f867 100644 --- a/test/parallel/test-bench-cli.js +++ b/test/parallel/test-bench-cli.js @@ -399,11 +399,14 @@ for (const isolation of ['process', 'none']) { ({ type }) => type === 'bench:diagnostic').data; const completion = records.find( ({ type }) => type === 'bench:complete').data; - assert.strictEqual(diagnostic.message, 'relayed warning'); - assert.strictEqual(diagnostic.level, 'warning'); + assert.deepStrictEqual(diagnostic.message, { + name: 'node:bench:test:diagnostic', + message: { value: '42' }, + }); + assert.strictEqual(diagnostic.level, 'info'); assert.strictEqual(diagnostic.phase, 'measurement'); assert.strictEqual(diagnostic.index, 0); - assert.deepStrictEqual(diagnostic.detail, { value: '42' }); + assert.strictEqual(diagnostic.detail, undefined); assert.strictEqual(diagnostic.benchId, completion.benchId); assert.strictEqual(diagnostic.fileRunId, completion.fileRunId); assert.strictEqual(completion.error, undefined); diff --git a/test/parallel/test-bench-context-errors.js b/test/parallel/test-bench-context-errors.js index ad2be4aa1274..9ba104443ad6 100644 --- a/test/parallel/test-bench-context-errors.js +++ b/test/parallel/test-bench-context-errors.js @@ -59,8 +59,8 @@ runner.bench('reentrant record', { samples: 1 }, (b) => { runner.bench('uncloneable detail', { samples: 1 }, (b) => { b.record({ duration_ns: 1n, operations: 1, detail: () => {} }); }); -runner.bench('invalid diagnostic message', { samples: 1 }, (b) => { - b.diagnostic(1); +runner.bench('uncloneable diagnostic message', { samples: 1 }, (b) => { + b.diagnostic(() => {}); }); runner.bench('invalid diagnostic level', { samples: 1 }, (b) => { b.diagnostic('invalid', { level: 'error' }); @@ -113,8 +113,8 @@ runner.bench('caught diagnostic violation', { samples: 1 }, 'ERR_INVALID_STATE'); assert.strictEqual(byName.get('uncloneable detail').error.name, 'DataCloneError'); - assert.strictEqual(byName.get('invalid diagnostic message').error.code, - 'ERR_INVALID_ARG_TYPE'); + assert.strictEqual(byName.get('uncloneable diagnostic message').error.name, + 'DataCloneError'); assert.strictEqual(byName.get('invalid diagnostic level').error.code, 'ERR_INVALID_ARG_VALUE'); assert.strictEqual(byName.get('uncloneable diagnostic detail').error.name, diff --git a/test/parallel/test-bench-diagnostic-channels.js b/test/parallel/test-bench-diagnostic-channels.js new file mode 100644 index 000000000000..9e1bb2a07024 --- /dev/null +++ b/test/parallel/test-bench-diagnostic-channels.js @@ -0,0 +1,119 @@ +// Flags: --no-warnings +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const dc = require('diagnostics_channel'); +const { createRunner } = require('node:bench'); + +const prefix = `node:bench:test:${process.pid}`; +const inheritedName = `${prefix}:inherited`; +const nestedName = `${prefix}:nested`; +const benchmarkName = `${prefix}:benchmark`; +const unlistedName = `${prefix}:unlisted`; +const symbolName = Symbol(`${prefix}:symbol`); +const inheritedChannel = dc.channel(inheritedName); +const nestedChannel = dc.channel(nestedName); +const benchmarkChannel = dc.channel(benchmarkName); +const unlistedChannel = dc.channel(unlistedName); +const symbolChannel = dc.channel(symbolName); + +function recordSample(context) { + context.record({ duration_ns: 1n, operations: 1 }); +} + +async function testCapture() { + const runner = createRunner({ yieldBetweenSamples: false }); + runner.suite('outer', { + diagnosticChannels: [inheritedName, symbolName, inheritedName], + }, common.mustCall(() => { + runner.suite('inner', { + diagnosticChannels: [nestedName], + }, common.mustCall(() => { + runner.bench('captured', { + diagnosticChannels: [benchmarkName, nestedName], + samples: 1, + }, common.mustCall((context) => { + const message = { value: 1 }; + inheritedChannel.publish(message); + message.value = 0; + nestedChannel.publish({ value: 2 }); + benchmarkChannel.publish({ value: 3 }); + unlistedChannel.publish({ value: 4 }); + symbolChannel.publish({ value: 5 }); + recordSample(context); + })); + })); + })); + runner.bench('not captured', { samples: 1 }, common.mustCall((context) => { + inheritedChannel.publish({ value: 6 }); + recordSample(context); + })); + + const records = await runner.run().toArray(); + const plan = records.find( + ({ type, data }) => type === 'bench:plan' && data.name === 'captured').data; + assert.deepStrictEqual(plan.diagnosticChannels, [ + inheritedName, + nestedName, + benchmarkName, + ]); + const diagnostics = records.filter( + ({ type }) => type === 'bench:diagnostic').map(({ data }) => data); + assert.deepStrictEqual(diagnostics.map(({ message }) => message), [ + { name: inheritedName, message: { value: 1 } }, + { name: nestedName, message: { value: 2 } }, + { name: benchmarkName, message: { value: 3 } }, + ]); + assert(diagnostics.every(({ level }) => level === 'info')); + assert.strictEqual(inheritedChannel.hasSubscribers, false); + assert.strictEqual(nestedChannel.hasSubscribers, false); + assert.strictEqual(benchmarkChannel.hasSubscribers, false); + assert.strictEqual(symbolChannel.hasSubscribers, false); +} + +async function testUncloneableMessage() { + const name = `${prefix}:uncloneable`; + const channel = dc.channel(name); + const runner = createRunner({ yieldBetweenSamples: false }); + runner.bench('uncloneable', { + diagnosticChannels: [name], + samples: 1, + }, common.mustCall((context) => { + channel.publish(() => {}); + recordSample(context); + })); + const records = await runner.run().toArray(); + const result = records.find( + ({ type }) => type === 'bench:complete').data; + assert.strictEqual(result.error.name, 'DataCloneError'); + assert.strictEqual(channel.hasSubscribers, false); +} + +async function testAbortCleanup() { + const name = `${prefix}:abort`; + const channel = dc.channel(name); + const controller = new AbortController(); + const runner = createRunner({ yieldBetweenSamples: false }); + runner.bench('abort', { + diagnosticChannels: [name], + samples: 1, + signal: controller.signal, + }, common.mustCall((context) => { + controller.abort(new Error('stop')); + assert.strictEqual(channel.hasSubscribers, false); + channel.publish({ ignored: true }); + recordSample(context); + })); + const records = await runner.run().toArray(); + assert.strictEqual( + records.some(({ type }) => type === 'bench:diagnostic'), + false, + ); +} + +(async () => { + await testCapture(); + await testUncloneableMessage(); + await testAbortCleanup(); +})().then(common.mustCall()); diff --git a/test/parallel/test-bench-diagnostics.js b/test/parallel/test-bench-diagnostics.js index 9f8facbcb16f..9e33fd56323f 100644 --- a/test/parallel/test-bench-diagnostics.js +++ b/test/parallel/test-bench-diagnostics.js @@ -99,12 +99,14 @@ async function testAfterEachFailurePrecedence() { }, common.mustCall((b) => { finalContext = b; const detail = { index: b.index }; + const message = { index: b.index, phase: b.phase }; const level = b.phase === 'warmup' ? 'info' : 'warning'; - assert.strictEqual(b.diagnostic(`${b.phase} ${b.index}`, { + assert.strictEqual(b.diagnostic(message, { detail, level, }), undefined); detail.index = -1; + message.index = -1; recordSample(b); }, 3)); const expectedError = new Error('benchmark failed'); @@ -135,9 +137,9 @@ async function testAfterEachFailurePrecedence() { assert.strictEqual(diagnostics.length, 4); assert.strictEqual(namedDiagnostics.length, diagnostics.length); assert.deepStrictEqual(diagnostics.map(({ message }) => message), [ - 'warmup 0', - 'measurement 0', - 'measurement 1', + { index: 0, phase: 'warmup' }, + { index: 0, phase: 'measurement' }, + { index: 1, phase: 'measurement' }, 'before failure', ]); assert.deepStrictEqual(diagnostics.map(({ phase, index, level }) => ({ diff --git a/test/parallel/test-bench-validation.js b/test/parallel/test-bench-validation.js index 7071dd2d915f..fa7a579aa60c 100644 --- a/test/parallel/test-bench-validation.js +++ b/test/parallel/test-bench-validation.js @@ -34,6 +34,10 @@ assert.throws(() => bench('name', { timeout: -1 }, noop), { code: 'ERR_OUT_OF_RANGE' }); assert.throws(() => bench('name', { signal: {} }, noop), { code: 'ERR_INVALID_ARG_TYPE' }); +assert.throws(() => bench('name', { diagnosticChannels: 'channel' }, noop), + { code: 'ERR_INVALID_ARG_TYPE' }); +assert.throws(() => bench('name', { diagnosticChannels: [1] }, noop), + { code: 'ERR_INVALID_ARG_TYPE' }); assert.throws(() => bench('name', { tags: 'fast' }, noop), { code: 'ERR_INVALID_ARG_TYPE' }); assert.throws(() => bench('name', { tags: [''] }, noop), From b50499a40b4660f8a0f4dd7d283e7078f90f78b5 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 29 Aug 2026 19:22:02 +0000 Subject: [PATCH 12/14] lib,benchmark: address multiple review issues Refs: https://github.com/nodejs/node/pull/65606#discussion_r3887305271 Refs: https://github.com/nodejs/node/pull/65606#discussion_r3887305272 Refs: https://github.com/nodejs/node/pull/65606#discussion_r3887305275 --- benchmark/README.md | 6 +- benchmark/_node-bench-analysis.js | 41 ++++++++++--- benchmark/compare-node-bench.js | 6 +- doc/api/bench.md | 5 ++ .../writing-and-running-benchmarks.md | 5 +- lib/internal/bench_runner/harness.js | 34 ++++++++++- test/parallel/test-bench-errors.js | 40 +++++++++---- .../test-benchmark-node-bench-tools.js | 57 +++++++++++++++++-- 8 files changed, 164 insertions(+), 30 deletions(-) diff --git a/benchmark/README.md b/benchmark/README.md index d2d1ac6a0f7c..78a55fba453f 100644 --- a/benchmark/README.md +++ b/benchmark/README.md @@ -80,9 +80,9 @@ Rscript benchmark/compare.R < compare-node-bench.csv Pass `--analyze` to run the same Welch analysis inline. `--max-regression N` implies `--analyze` and makes the command fail only when the Holm-Bonferroni -adjusted p-value is below 0.05 and the full 95% confidence interval is worse -than `-N%`. Requiring both conditions prevents a noisy point estimate from -failing a regression gate. +adjusted one-sided p-value against the `N%` threshold is below 0.05 and the full +95% confidence interval is worse than `-N%`. Requiring both conditions prevents +a noisy point estimate from failing a regression gate. ```console ./node benchmark/compare-node-bench.js \ diff --git a/benchmark/_node-bench-analysis.js b/benchmark/_node-bench-analysis.js index 5fa3c34d3976..bf0f7402b791 100644 --- a/benchmark/_node-bench-analysis.js +++ b/benchmark/_node-bench-analysis.js @@ -31,8 +31,19 @@ function holmAdjust(pValues) { return adjusted; } +function thresholdPValue(oldRates, newHistogram, scale, maxRegression) { + const factor = 1 - maxRegression / 100; + if (factor <= 0) return 1; + const thresholdHistogram = createRateHistogram( + oldRates.map((rate) => rate * factor), scale, 3); + const result = thresholdHistogram.welchTest(newHistogram); + if (Number.isNaN(result.pValue)) return 1; + return result.tStatistic > 0 ? + result.pValue / 2 : 1 - result.pValue / 2; +} + function isRegressionFailure(row, maxRegression) { - return row.pAdjusted < 0.05 && + return row.pThresholdAdjusted < 0.05 && row.improvement + row.ci95 < -maxRegression; } @@ -80,7 +91,7 @@ function analyzeCompare(samples, scale, maxRegression) { result.confidenceInterval.lower) / 2; return (half / (oldMean * scale)) * 100; }; - rows.push({ + const row = { ci95: ciPercent(w95), ci99: ciPercent(w99), ci999: ciPercent(w999), @@ -88,14 +99,24 @@ function analyzeCompare(samples, scale, maxRegression) { name, pValue: Number.isNaN(w95.pValue) ? 1 : w95.pValue, stars, - }); + }; + if (maxRegression !== undefined) { + row.pThreshold = thresholdPValue( + oldRates, newHistogram, scale, maxRegression); + } + rows.push(row); } const adjusted = holmAdjust(rows.map(({ pValue }) => pValue)); + const thresholdAdjusted = maxRegression === undefined ? null : + holmAdjust(rows.map(({ pThreshold }) => pThreshold)); let underpowered = 0; for (let index = 0; index < rows.length; index++) { const row = rows[index]; row.pAdjusted = adjusted[index]; + if (thresholdAdjusted !== null) { + row.pThresholdAdjusted = thresholdAdjusted[index]; + } row.inconclusive = maxRegression > 0 && row.stars.trim() === '' && row.ci95 > maxRegression; @@ -146,8 +167,13 @@ function analyzeCompare(samples, scale, maxRegression) { `After Holm-Bonferroni correction across ${rows.length} comparison` + `${rows.length === 1 ? '' : 's'}, ${significant} remain` + `${significant === 1 ? 's' : ''} significant at 5%.`, - '--max-regression uses the corrected values.', ); + if (maxRegression !== undefined) { + output.push( + `For --max-regression, one-sided p-values against the ` + + `${maxRegression}% threshold were corrected separately.`, + ); + } if (maxRegression > 0 && underpowered > 0) { output.push(''); @@ -159,21 +185,22 @@ function analyzeCompare(samples, scale, maxRegression) { ); } - const failures = maxRegression > 0 ? + const failures = maxRegression !== undefined ? rows.filter((row) => isRegressionFailure(row, maxRegression)) : []; if (failures.length > 0) { output.push(''); output.push( `FAIL: ${failures.length} benchmark${failures.length === 1 ? '' : 's'}` + ` regressed by more than ${maxRegression}% (the 95% interval excludes ` + - `the threshold and significance is family-wise corrected across ` + + `the threshold and its one-sided test is family-wise corrected across ` + `${rows.length} comparisons):`, ); for (const failure of failures) { output.push( ` ${failure.name} ${failure.improvement.toFixed(2)}% ` + `(95% CI up to ${(failure.improvement + failure.ci95).toFixed(2)}%, ` + - `adjusted p=${failure.pAdjusted.toExponential(2)})`, + `adjusted threshold p=` + + `${failure.pThresholdAdjusted.toExponential(2)})`, ); } } diff --git a/benchmark/compare-node-bench.js b/benchmark/compare-node-bench.js index d2bc6071bed7..295f15f0fdc1 100644 --- a/benchmark/compare-node-bench.js +++ b/benchmark/compare-node-bench.js @@ -36,9 +36,10 @@ async function main() { const runs = parseInteger(cli.optional.runs, 30, '--runs', 1); const warmup = parseInteger(cli.optional.warmup, 0, '--warmup', 0); const scale = parseInteger(cli.optional.scale, 1000, '--scale', 1); + const hasMaxRegression = cli.optional['max-regression'] !== undefined; const maxRegression = parseNumber( cli.optional['max-regression'], 0, '--max-regression', 0); - const analyze = !!cli.optional.analyze || maxRegression > 0; + const analyze = !!cli.optional.analyze || hasMaxRegression; const options = { namePattern: cli.optional['name-pattern'], nodeArgs: cli.optional['node-arg'], @@ -96,7 +97,8 @@ async function main() { } if (analyze) { - const result = analyzeCompare(rows, scale, maxRegression); + const result = analyzeCompare( + rows, scale, hasMaxRegression ? maxRegression : undefined); process.stdout.write(result.output); if (result.failed) process.exitCode = 1; return; diff --git a/doc/api/bench.md b/doc/api/bench.md index 842e8a59264b..b25503830f2c 100644 --- a/doc/api/bench.md +++ b/doc/api/bench.md @@ -349,6 +349,11 @@ samples, but their samples are discarded. An exception, rejection, timeout, abort, missing timing call, or duplicate timing call stops the current benchmark. Later benchmarks continue to run. +After a timeout or abort, the runner briefly waits for asynchronous benchmark +work to settle before continuing. If it remains pending, all later benchmarks +that were selected to run fail without running so that their measurements +cannot overlap with that work. + For each warmup and measured callback, the runner subscribes to the configured diagnostics channels. Each publication queues a context diagnostic whose `message` is `{ name, message }`, containing the string channel name and the diff --git a/doc/contributing/writing-and-running-benchmarks.md b/doc/contributing/writing-and-running-benchmarks.md index 26d42dc17594..a4c8e20244cd 100644 --- a/doc/contributing/writing-and-running-benchmarks.md +++ b/doc/contributing/writing-and-running-benchmarks.md @@ -721,8 +721,9 @@ Both parallel tools support inline analysis. `scatter-node-bench.js --analyze` uses the same `--xaxis`, `--category`, and `--no-chart` interface described for `scatter.js`. `compare-node-bench.js --analyze` performs Welch's t-test, while `--max-regression N` adds a corrected regression gate. The gate requires both a -Holm-Bonferroni-adjusted p-value below 0.05 and a 95% confidence interval lying -entirely beyond `-N%`; the point estimate alone cannot fail the command. +Holm-Bonferroni-adjusted one-sided p-value against the `N%` threshold below 0.05 +and a 95% confidence interval lying entirely beyond `-N%`; the point estimate +alone cannot fail the command. Scatter analysis reduces aggregated configurations to one value per outer process and uses disjoint process sets for consecutive Mann-Whitney comparisons so configurations sharing a process are not treated as independent samples. diff --git a/lib/internal/bench_runner/harness.js b/lib/internal/bench_runner/harness.js index c18f439f6719..985440634217 100644 --- a/lib/internal/bench_runner/harness.js +++ b/lib/internal/bench_runner/harness.js @@ -64,6 +64,7 @@ const { } = require('internal/bench_runner/benchmarks_stream'); const { bigint: hrtime } = process.hrtime; +const kCancellationGracePeriod = 100; const kHookNames = ['after', 'afterEach', 'before', 'beforeEach']; const kIsCliRunner = getOptionValue('--bench'); let nextRunId = 0; @@ -76,6 +77,20 @@ function eventLoopTurn() { return new Promise((resolve) => setImmediate(resolve)); } +async function settlesPromptly(work) { + const timeout = PromiseWithResolvers(); + const timer = setTimeout( + () => timeout.resolve(false), kCancellationGracePeriod); + try { + return await SafePromiseRace([ + PromisePrototypeThen(work, () => true, () => true), + timeout.promise, + ]); + } finally { + clearTimeout(timer); + } +} + function createAbortError(signal) { return new AbortError(undefined, { __proto__: null, cause: signal.reason }); } @@ -86,6 +101,7 @@ function createTimeoutError(benchmark) { } class Harness { + #abortReason = null; #autoRun; #buildPromises = []; #duplicateErrors = new SafeMap(); @@ -581,6 +597,14 @@ class Harness { } async #executeSuite(suite) { + if (this.#abortReason !== null) { + await this.#completeSubtree(suite, this.#abortReason); + suite.finished = true; + suite.completion.resolve(); + if (!suite.isRoot) suite.emitDestroy(); + return; + } + if (suite.buildError !== null) { await this.#diagnostic(suite.buildError, suite.loc, 'error', suite); await this.#completeSubtree(suite, suite.buildError); @@ -614,7 +638,7 @@ class Harness { } } - if (active) { + if (active && this.#abortReason === null) { const failure = await this.#runSuiteHooks(suite, 'after'); if (failure !== null) { await this.#diagnostic( @@ -711,6 +735,13 @@ class Harness { } catch { // Preserve the error that stopped benchmark execution. } + if (!await settlesPromptly(work) && this.#abortReason === null) { + this.#abortReason = new AbortError( + 'The benchmark run was aborted because asynchronous work did not ' + + 'settle after cancellation', + { __proto__: null, cause: error }, + ); + } throw error; } finally { if (timer !== undefined) clearTimeout(timer); @@ -833,6 +864,7 @@ class Harness { return; } + if (this.#abortReason !== null) forcedError = this.#abortReason; if (forcedError !== undefined) { this.success = false; this.counts.failed++; diff --git a/test/parallel/test-bench-errors.js b/test/parallel/test-bench-errors.js index b0774965a26f..891048d23881 100644 --- a/test/parallel/test-bench-errors.js +++ b/test/parallel/test-bench-errors.js @@ -27,14 +27,21 @@ bench('invalid operations', options, (b) => { bench('throws', options, () => { throw new Error('benchmark failure'); }); -bench('timeout', { samples: 1, timeout: 10 }, async () => { - await new Promise(() => {}); -}); +let lateTimeoutActive = false; bench('late timeout', { samples: 1, timeout: 5 }, async (b) => { - b.start(); - await setTimeout(30); - b.end(1); + lateTimeoutActive = true; + try { + b.start(); + await setTimeout(30); + b.end(1); + } finally { + lateTimeoutActive = false; + } }); +bench('after late timeout', options, common.mustCall((b) => { + assert.strictEqual(lateTimeoutActive, false); + complete(b); +})); const signal = AbortSignal.abort(new Error('stop')); bench('aborted', { samples: 1, signal }, () => {}); @@ -48,6 +55,11 @@ function complete(b) { bench('duplicate', { samples: 1, params: { value: 1 } }, complete); bench('duplicate', { samples: 1, params: { value: 1 } }, complete); bench('continues', options, complete); +bench('timeout', { samples: 1, timeout: 10 }, async () => { + await new Promise(() => {}); +}); +bench('after unsettled timeout', options, common.mustNotCall()); +bench.skip('skipped after unsettled timeout', options, common.mustNotCall()); const completions = []; const sampleNames = []; @@ -57,13 +69,13 @@ stream.on('bench:complete', (result) => completions.push(result)); stream.on('bench:sample', (sample) => sampleNames.push(sample.name)); stream.on('bench:summary', (result) => { summary = result; }); stream.on('end', common.mustCall(() => { - assert.strictEqual(completions.length, 13); + assert.strictEqual(completions.length, 16); assert.deepStrictEqual(summary.counts, { __proto__: null, - completed: 2, - failed: 11, - skipped: 0, - total: 13, + completed: 3, + failed: 12, + skipped: 1, + total: 16, }); assert.strictEqual(summary.success, false); @@ -92,12 +104,18 @@ stream.on('end', common.mustCall(() => { 'ERR_OPERATION_FAILED'); assert.strictEqual(byName.get('late timeout')[0].error.code, 'ERR_OPERATION_FAILED'); + assert.strictEqual(byName.get('after late timeout')[0].error, undefined); assert.strictEqual(byName.get('aborted')[0].error.code, 'ABORT_ERR'); const duplicates = byName.get('duplicate'); assert.strictEqual(duplicates[0].error, undefined); assert.match(duplicates[1].error.message, /duplicate benchmark identity/); assert.strictEqual(byName.get('continues')[0].error, undefined); + const unsettled = byName.get('after unsettled timeout')[0].error; + assert.strictEqual(unsettled.code, 'ABORT_ERR'); + assert.strictEqual(unsettled.cause.code, 'ERR_OPERATION_FAILED'); + assert.strictEqual( + byName.get('skipped after unsettled timeout')[0].skip, true); setTimeout(40).then(common.mustCall(() => { assert.strictEqual(sampleNames.includes('late timeout'), false); })); diff --git a/test/parallel/test-benchmark-node-bench-tools.js b/test/parallel/test-benchmark-node-bench-tools.js index 90dd02028de3..3d248b45c0e3 100644 --- a/test/parallel/test-benchmark-node-bench-tools.js +++ b/test/parallel/test-benchmark-node-bench-tools.js @@ -9,6 +9,7 @@ const path = require('path'); const fixtures = require('../common/fixtures'); const tmpdir = require('../common/tmpdir'); const { + analyzeCompare, analyzeScatter, holmAdjust, isRegressionFailure, @@ -27,18 +28,51 @@ assert.deepStrictEqual(holmAdjust([0.01, 0.03, 0.04]), [0.03, 0.06, 0.06]); assert.strictEqual(isRegressionFailure({ ci95: 3, improvement: -12, - pAdjusted: 0.01, + pThresholdAdjusted: 0.01, }, 10), false); assert.strictEqual(isRegressionFailure({ ci95: 1, improvement: -12, - pAdjusted: 0.06, + pThresholdAdjusted: 0.06, }, 10), false); assert.strictEqual(isRegressionFailure({ ci95: 1, improvement: -12, - pAdjusted: 0.01, + pThresholdAdjusted: 0.01, }, 10), true); + +function comparisonSamples(oldRates, newRates) { + const create = (binary, rate) => ({ + binary, + configuration: '', + identity: 'comparison', + name: 'comparison', + rate, + }); + return [ + ...oldRates.map((rate) => create('old', rate)), + ...newRates.map((rate) => create('new', rate)), + ]; +} + +{ + const result = analyzeCompare(comparisonSamples( + [98, 99, 100, 100, 101, 102], + [48, 49, 50, 50, 51, 52], + ), 1000, 0); + assert.strictEqual(result.failed, true); + assert.strictEqual(result.rows[0].pThresholdAdjusted < 0.05, true); +} + +{ + const result = analyzeCompare(comparisonSamples( + [98, 99, 100, 100, 101, 102], + [93, 94, 95, 95, 96, 97], + ), 1000, 10); + assert.strictEqual(result.rows[0].pAdjusted < 0.05, true); + assert.strictEqual(result.rows[0].pThresholdAdjusted > 0.05, true); + assert.strictEqual(result.failed, false); +} assert.throws( () => analyzeScatter([{ observation: 0, @@ -221,7 +255,22 @@ function run(script, args, options = undefined) { assert.strictEqual(result.stderr, ''); assert.match(result.stdout, /confidence\s+improvement\s+accuracy/); assert.match(result.stdout, /Holm-Bonferroni correction/); - assert.match(result.stdout, /--max-regression uses the corrected values/); + assert.match(result.stdout, /one-sided p-values against the 100% threshold/); + assert.doesNotMatch(result.stdout, /"binary","filename"/); +} + +{ + const result = run(compare, [ + '--old', process.execPath, + '--new', process.execPath, + '--runs', '1', + '--max-regression', '0', + '--', fixtures.path('bench-runner/tools-no-params.cjs'), + ]); + assert.strictEqual(result.status, 0, result.stderr); + assert.strictEqual(result.stderr, ''); + assert.match(result.stdout, /confidence\s+improvement\s+accuracy/); + assert.match(result.stdout, /one-sided p-values against the 0% threshold/); assert.doesNotMatch(result.stdout, /"binary","filename"/); } From 3dd38e69ef0ed6fdfb0f267e15a861009802c147 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Thu, 3 Sep 2026 18:25:38 +0000 Subject: [PATCH 13/14] lib: fixup node:bench handling of --require option Signed-off-by: James M Snell --- lib/internal/bench_runner/cli.js | 4 +++- test/parallel/test-bench-cli.js | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/internal/bench_runner/cli.js b/lib/internal/bench_runner/cli.js index 0cc4dc5210a1..5eaf0a317147 100644 --- a/lib/internal/bench_runner/cli.js +++ b/lib/internal/bench_runner/cli.js @@ -649,7 +649,9 @@ function filterExecArgv(arg, index, args) { !ArrayPrototypeSome(kFilterArgValues, (option) => { return name === option || (option !== '--experimental-config-file' && - index > 0 && getOptionName(args[index - 1]) === option); + index > 0 && + StringPrototypeIndexOf(args[index - 1], '=') === -1 && + getOptionName(args[index - 1]) === option); }); } diff --git a/test/parallel/test-bench-cli.js b/test/parallel/test-bench-cli.js index 21090814f867..a2ac88eefbcf 100644 --- a/test/parallel/test-bench-cli.js +++ b/test/parallel/test-bench-cli.js @@ -570,9 +570,9 @@ for (const mode of ['callback', 'throw']) { { const result = spawnBench([ - '--stack-trace-limit=17', - '--random-seed=17', '--bench-reporter=json', + '--random-seed=17', + '--stack-trace-limit=17', fixtures.path('bench-runner/v8-option.cjs'), ]); assert.strictEqual(result.status, 0); From 09785df763be9fd0936f59cb6905f2f672a45a6e Mon Sep 17 00:00:00 2001 From: James M Snell Date: Thu, 3 Sep 2026 19:40:01 +0000 Subject: [PATCH 14/14] test: expand test coverage of node:bench Signed-off-by: James M Snell Assisted-by: Opencode --- .../bench-runner/malformed-record.cjs | 12 ++++ test/fixtures/bench-runner/run-file.cjs | 1 + test/parallel/test-bench-cli.js | 18 ++++++ test/parallel/test-bench-errors.js | 9 ++- test/parallel/test-bench-harness-errors.js | 45 ++++++++++++--- test/parallel/test-bench-run-file.js | 37 ++++++++++-- test/parallel/test-bench-stream.js | 56 ++++++++++++++++++- 7 files changed, 161 insertions(+), 17 deletions(-) diff --git a/test/fixtures/bench-runner/malformed-record.cjs b/test/fixtures/bench-runner/malformed-record.cjs index 61de7d9b567a..3550694532f5 100644 --- a/test/fixtures/bench-runner/malformed-record.cjs +++ b/test/fixtures/bench-runner/malformed-record.cjs @@ -41,12 +41,24 @@ const record = kind === 'summary' ? { runId: process.env.NODE_BENCH_RUN_ID, success: true, }, +} : kind === 'name-path' ? { + ...plan, + data: { + ...plan.data, + namePath: [1], + }, } : kind === 'plan' ? { ...plan, data: { ...plan.data, samples: 0, }, +} : kind === 'timeout' ? { + ...plan, + data: { + ...plan.data, + timeout: -1, + }, } : kind === 'identity' ? { type: 'bench:complete', data: { diff --git a/test/fixtures/bench-runner/run-file.cjs b/test/fixtures/bench-runner/run-file.cjs index 382af4cc42f1..436665074b6b 100644 --- a/test/fixtures/bench-runner/run-file.cjs +++ b/test/fixtures/bench-runner/run-file.cjs @@ -7,6 +7,7 @@ bench('run file', { params: { context: process.env.NODE_BENCH_CONTEXT, exposed: typeof globalThis.gc === 'function', + omitted: process.env.NODE_BENCH_OMITTED === undefined, value: process.env.NODE_BENCH_RUN_FILE ?? 'unset', }, }, (b) => { diff --git a/test/parallel/test-bench-cli.js b/test/parallel/test-bench-cli.js index a2ac88eefbcf..8fd05375bb07 100644 --- a/test/parallel/test-bench-cli.js +++ b/test/parallel/test-bench-cli.js @@ -488,7 +488,9 @@ for (const { kind, message } of [ { kind: 'sequence', message: /valid record sequence/ }, { kind: 'record', message: /not a valid benchmark record/ }, { kind: 'identity', message: /not a valid benchmark record/ }, + { kind: 'name-path', message: /not a valid benchmark record/ }, { kind: 'plan', message: /not a valid benchmark plan/ }, + { kind: 'timeout', message: /not a valid benchmark plan/ }, { kind: 'diagnostic', message: /not a valid benchmark diagnostic/ }, { kind: 'diagnostic-order', message: /valid lifecycle sequence/ }, { kind: 'summary', message: /not a valid benchmark summary/ }, @@ -525,6 +527,22 @@ for (const { kind, message } of [ assert.strictEqual(records.at(-1).data.success, false); } +{ + const result = spawnBench([ + '--bench-isolation=none', + '--bench-reporter=json', + fixtures.path('bench-runner/a.cjs'), + fixtures.path('bench-runner/exit-code.cjs'), + ]); + assert.strictEqual(result.status, 1); + const records = parseRecords(result); + const diagnostic = records.find(({ type, data }) => + type === 'bench:diagnostic' && /set exit code/.test(data.message)).data; + assert.strictEqual(diagnostic.entryFile, null); + assert.strictEqual(diagnostic.fileRunId, null); + assert.strictEqual(diagnostic.file, null); +} + for (const { mode, message } of [ { mode: 'code', message: /failed with exit code 2/ }, { mode: 'late', message: /failed with exit code 2/ }, diff --git a/test/parallel/test-bench-errors.js b/test/parallel/test-bench-errors.js index 891048d23881..2124720fee63 100644 --- a/test/parallel/test-bench-errors.js +++ b/test/parallel/test-bench-errors.js @@ -3,7 +3,7 @@ const common = require('../common'); const assert = require('assert'); -const { bench, run } = require('node:bench'); +const { bench, run, suite } = require('node:bench'); const { setTimeout } = require('timers/promises'); const options = { samples: 1 }; @@ -58,8 +58,11 @@ bench('continues', options, complete); bench('timeout', { samples: 1, timeout: 10 }, async () => { await new Promise(() => {}); }); -bench('after unsettled timeout', options, common.mustNotCall()); -bench.skip('skipped after unsettled timeout', options, common.mustNotCall()); +const suiteCompletion = suite('after unsettled timeout suite', () => { + bench('after unsettled timeout', options, common.mustNotCall()); + bench.skip('skipped after unsettled timeout', options, common.mustNotCall()); +}); +suiteCompletion.then(common.mustCall()); const completions = []; const sampleNames = []; diff --git a/test/parallel/test-bench-harness-errors.js b/test/parallel/test-bench-harness-errors.js index a5e01894c640..1cb0b505689f 100644 --- a/test/parallel/test-bench-harness-errors.js +++ b/test/parallel/test-bench-harness-errors.js @@ -62,17 +62,20 @@ async function testRunSignal() { async function testRunSignalAfterSample() { const runner = createRunner({ yieldBetweenSamples: false }); const controller = new AbortController(); + const reason = new Error('sample aborted'); const completion = runner.bench('aborted after sample', { - samples: 1, - }, (b) => { - complete(b); - controller.abort(new Error('sample aborted')); - }); - await runner.run({ signal: controller.signal }).toArray(); + samples: 2, + }, complete); + const stream = runner.run({ signal: controller.signal }); + stream.once('bench:sample', common.mustCall(() => { + controller.abort(reason); + })); + const records = await stream.toArray(); const result = await completion; - await setImmediate(); assert.strictEqual(result.error.code, 'ABORT_ERR'); - assert.strictEqual(result.error.cause.message, 'sample aborted'); + assert.strictEqual(result.error.cause, reason); + assert.strictEqual(result.samples.length, 1); + assert.strictEqual(records.at(-1).data.success, false); } async function testStringNamePattern() { @@ -93,12 +96,15 @@ async function testStringNamePattern() { async function testTopLevelRecovery() { const runner = createRunner({ yieldBetweenSamples: false }); - runner.bench('listener failure', { samples: 1 }, complete); + const suiteCompletion = runner.suite('nested', () => { + runner.bench('listener failure', { samples: 1 }, complete); + }); const stream = runner.run(); const failure = new Error(); failure.message = undefined; stream.on('bench:start', common.mustCall(() => { throw failure; })); const records = await stream.toArray(); + await suiteCompletion; const diagnostic = records.find( ({ type }) => type === 'bench:diagnostic').data; const summary = records.find(({ type }) => type === 'bench:summary').data; @@ -110,10 +116,31 @@ async function testTopLevelRecovery() { assert.strictEqual(summary.success, false); } +async function testRepeatedReportingFailure() { + const runner = createRunner({ yieldBetweenSamples: false }); + const original = new Error('start listener failed'); + const diagnostic = new Error('diagnostic listener failed'); + const summary = new Error('summary listener failed'); + const completion = runner.bench('reporting failures', { + samples: 1, + }, complete); + const stream = runner.run(); + stream.on('bench:start', common.mustCall(() => { throw original; })); + stream.on('bench:diagnostic', common.mustCall(() => { + throw diagnostic; + }, 2)); + stream.on('bench:summary', common.mustCall(() => { throw summary; })); + const ended = new Promise((resolve) => stream.once('end', resolve)); + stream.resume(); + await ended; + assert.strictEqual((await completion).error, original); +} + (async () => { await testSynchronousSuiteFailure(); await testRunSignal(); await testRunSignalAfterSample(); await testStringNamePattern(); await testTopLevelRecovery(); + await testRepeatedReportingFailure(); })().then(common.mustCall()); diff --git a/test/parallel/test-bench-run-file.js b/test/parallel/test-bench-run-file.js index 600f1a212ae5..ec0265ab162c 100644 --- a/test/parallel/test-bench-run-file.js +++ b/test/parallel/test-bench-run-file.js @@ -29,9 +29,15 @@ assert.throws(() => runFile(fixture, { execArgv: [fixture] }), { assert.throws(() => runFile(fixture, { execArgv: ['-e', '0'] }), { code: 'ERR_INVALID_ARG_VALUE', }); +assert.throws(() => runFile(fixture, { execArgv: ['-e0'] }), { + code: 'ERR_INVALID_ARG_VALUE', +}); assert.throws(() => runFile(fixture, { execArgv: ['--require'] }), { code: 'ERR_INVALID_ARG_VALUE', }); +assert.throws(() => runFile(fixture, { execArgv: ['--require', ''] }), { + code: 'ERR_INVALID_ARG_VALUE', +}); assert.throws(() => runFile(fixture, { execArgv: ['--require='] }), { code: 'ERR_INVALID_ARG_VALUE', }); @@ -73,11 +79,12 @@ assert.throws(() => runFile(fixture, { signal: { aborted: false } }), { }); async function testRunFile() { - const execArgv = ['--expose-gc', '-r', 'fs']; + const execArgv = ['--expose-gc', '--no-warnings', '-r', 'fs']; const env = { __proto__: null, ...process.env, NODE_BENCH_CONTEXT: 'not-a-child', + NODE_BENCH_OMITTED: undefined, NODE_BENCH_RUN_FILE: 'original', NODE_CHANNEL_FD: '999', NODE_CHANNEL_SERIALIZATION_MODE: 'json', @@ -93,12 +100,14 @@ async function testRunFile() { assert.strictEqual(records.at(-1).type, 'bench:summary'); assert.strictEqual(plan.params.context, 'child'); assert.strictEqual(plan.params.exposed, true); + assert.strictEqual(plan.params.omitted, true); assert.strictEqual(plan.params.value, 'original'); assert.strictEqual(result.error, undefined); assert.strictEqual(result.samples.length, 1); assert.strictEqual(typeof result.samples[0].duration_ns, 'bigint'); assert.notStrictEqual(result.samples[0].detail.pid, process.pid); assert(result.samples[0].detail.execArgv.includes('--expose-gc')); + assert(result.samples[0].detail.execArgv.includes('--no-warnings')); assert(result.samples[0].detail.execArgv.includes('-r')); assert.strictEqual(summary.success, true); assert.strictEqual(summary.file, fixture); @@ -258,13 +267,16 @@ async function testParentExitCode() { } async function testDefaultExecArgvSnapshot() { - const stream = runFile(fixture); - process.execArgv.push('--require=/does/not/exist.cjs'); + const original = Array.from(process.execArgv); try { + process.execArgv.push('--bench', '--eval', 'throw new Error()'); + const stream = runFile(fixture); + process.execArgv.push('--require=/does/not/exist.cjs'); const records = await stream.toArray(); assert.strictEqual(records.at(-1).data.success, true); } finally { - process.execArgv.pop(); + process.execArgv.length = 0; + process.execArgv.push(...original); } } @@ -280,6 +292,22 @@ async function testExecPathSnapshot() { } } +async function testSpawnFailure() { + const execPath = process.execPath; + let stream; + try { + process.execPath = fixtures.path('does-not-exist-node'); + stream = runFile(fixture); + } finally { + process.execPath = execPath; + } + const records = await stream.toArray(); + const diagnostic = records.find( + ({ type }) => type === 'bench:diagnostic').data; + assert.strictEqual(diagnostic.error.code, 'ENOENT'); + assert.strictEqual(records.at(-1).data.success, false); +} + function testEvalParent() { const script = ` require('node:bench').runFile(${JSON.stringify(fixture)}) @@ -394,6 +422,7 @@ async function testDestroy() { await testParentExitCode(); await testDefaultExecArgvSnapshot(); await testExecPathSnapshot(); + await testSpawnFailure(); await testPreAborted(); await testDestroy(); testEvalParent(); diff --git a/test/parallel/test-bench-stream.js b/test/parallel/test-bench-stream.js index 11a7e0184a54..9a9eff7b2369 100644 --- a/test/parallel/test-bench-stream.js +++ b/test/parallel/test-bench-stream.js @@ -81,6 +81,34 @@ async function testPlanBackpressure() { ); } +async function testDestroyWhileBlocked() { + const runner = createRunner({ yieldBetweenSamples: false }); + const completions = []; + for (let i = 0; i < 32; i++) { + completions.push(runner.bench(`destroyed ${i}`, { + samples: 1, + }, recordSample)); + } + const stream = runner.run(); + const unblocked = stream.waitForDrain(); + const iterator = stream[Symbol.asyncIterator](); + await iterator.next(); + await unblocked; + for (let i = 0; i < 100 && + stream.readableLength < stream.readableHighWaterMark; i++) { + await setImmediate(); + } + assert.strictEqual(stream.readableLength, stream.readableHighWaterMark); + + const draining = stream.waitForDrain(); + const closed = new Promise((resolve) => stream.once('close', resolve)); + stream.destroy(); + await assert.rejects(draining, { code: 'ERR_INVALID_STATE' }); + await assert.rejects(stream.waitForDrain(), { code: 'ERR_INVALID_STATE' }); + await closed; + await Promise.all(completions); +} + async function testNamedEventsWithoutReading() { const runner = createRunner(); const sampleCount = 64; @@ -192,7 +220,13 @@ async function testRecordOwnership() { expectedError.context = { note: 'preserved', callback() {}, + get accessor() { return 'value'; }, + }; + expectedError.customContext = { + __proto__: {}, + note: 'preserved', }; + expectedError.arrayPayload = [new WeakMap()]; const innerError = new Error('inner failure'); innerError.code = 'ERR_INNER'; const aggregate = new AggregateError([innerError], 'aggregate failure'); @@ -217,6 +251,16 @@ async function testRecordOwnership() { const caused = runner.bench('owned cause', { samples: 1 }, () => { throw causedError; }); + const throwingNameError = new Error('throwing name'); + Object.defineProperty(throwingNameError, 'name', { + configurable: true, + get() { throw new Error('name getter'); }, + }); + const throwingName = runner.bench('throwing name', { + samples: 1, + }, () => { + throw throwingNameError; + }); const thrownValue = new WeakMap(); const uncloneable = runner.bench('uncloneable error', { samples: 1, @@ -265,6 +309,7 @@ async function testRecordOwnership() { const measuredResult = await measured; const failedResult = await failed; await caused; + const throwingNameResult = await throwingName; const uncloneableResult = await uncloneable; const trappedResult = await trapped; const afterTrapResult = await afterTrap; @@ -278,6 +323,8 @@ async function testRecordOwnership() { ({ name }) => name === 'owned error'); const streamCaused = streamResults.find( ({ name }) => name === 'owned cause'); + const streamThrowingName = streamResults.find( + ({ name }) => name === 'throwing name'); const streamSummary = records.find( ({ type }) => type === 'bench:summary').data; @@ -299,12 +346,18 @@ async function testRecordOwnership() { assert.strictEqual(streamFailed.error.cause, streamFailed.error); assert.strictEqual(streamFailed.error.context.note, 'preserved'); assert.strictEqual(streamFailed.error.context.callback, undefined); + assert.strictEqual(streamFailed.error.context.accessor, undefined); + assert.strictEqual(streamFailed.error.customContext.note, 'preserved'); + assert.deepStrictEqual(streamFailed.error.arrayPayload, [undefined]); assert.strictEqual(failedResult.error, expectedError); assert.strictEqual(failedResult.error.code, 'ERR_EXPECTED'); assert.strictEqual(failedResult.error.cause, failedResult.error); assert.strictEqual(uncloneableResult.error, thrownValue); assert.strictEqual(trappedResult.error, proxyError); assert.strictEqual(afterTrapResult.error, undefined); + assert.strictEqual(throwingNameResult.error, throwingNameError); + assert.strictEqual(streamThrowingName.error.name, 'Error'); + assert.strictEqual(streamThrowingName.error.message, 'throwing name'); assert(streamCaused.error.cause instanceof AggregateError); assert.strictEqual(streamCaused.error.cause.name, 'AggregateError'); assert.strictEqual(streamCaused.error.cause.code, 'ERR_AGGREGATE'); @@ -313,12 +366,13 @@ async function testRecordOwnership() { streamCaused.error.references.get('self'), streamCaused.error); assert.strictEqual(streamCaused.error.members.has(streamCaused.error), true); assert.notStrictEqual(eventSummary, streamSummary); - assert.strictEqual(streamSummary.counts.total, 6); + assert.strictEqual(streamSummary.counts.total, 7); } (async () => { await testReadableBackpressure(); await testPlanBackpressure(); + await testDestroyWhileBlocked(); await testNamedEventsWithoutReading(); await testCancellationCompletesBenchmarks(); await testDeliveryDoesNotConsumeTimeout();