Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions benchmark/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ function allow() {

class Benchmark {
constructor(fn, configs, options = {}) {
this.setup = options.setup;
this.teardown = options.teardown;
// Used to make sure a benchmark only start a timer once
this._started = false;

Expand Down Expand Up @@ -62,6 +64,9 @@ class Benchmark {
if (process.env.NODE_RUN_BENCHMARK_FN !== undefined) {
fn(this.config);
} else {
if (this.setup) {
this.setup();
}
// _run will use fork() to create a new process for each configuration
// combination.
this._run();
Expand Down Expand Up @@ -234,6 +239,8 @@ class Benchmark {

if (queueIndex + 1 < this.queue.length) {
recursive(queueIndex + 1);
} else if (this.teardown) {
this.teardown();
}
});
};
Expand Down
17 changes: 11 additions & 6 deletions benchmark/module/module-require.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,22 @@ const common = require('../common.js');
const tmpdir = require('../../test/common/tmpdir');
const benchmarkDirectory = tmpdir.resolve('nodejs-benchmark-module');

const t = 1e4;

const bench = common.createBenchmark(main, {
type: ['.js', '.json', 'dir'],
n: [1e4],
n: [t],
}, {
setup() {
tmpdir.refresh();
Copy link
Member

Choose a reason for hiding this comment

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

Is it exactly equal to the previous behaviour? Your documentation states:

A function to be run once by the main "controller"
process before any benchmark child processes are spawned.

However, this should run once per configuration, as it does on function main(). Either your documentation is wrong, or the setup and teardown shouldn't work like that; instead, it should be called once per configuration

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a log to createEntryPoint

Current output

$ node benchmark/module/module-require.js
called createEntryPoint
module/module-require.js n=10000 type=".js": 9,197.64355525931
called createEntryPoint
module/module-require.js n=10000 type=".json": 19,692.5488212058
called createEntryPoint
module/module-require.js n=10000 type="dir": 8,185.132800456524

My Pr

$ node benchmark/module/module-require.js
called setup
called createEntryPoint
module/module-require.js n=10000 type=".js": 9,507.804393764822
module/module-require.js n=10000 type=".json": 18,387.40575971879
module/module-require.js n=10000 type="dir": 7,686.528215393394
called teardown

However, this should run once per configuration, as it does on function main(). Either your documentation is wrong, or the setup and teardown shouldn't work like that; instead, it should be called once per configuration

It runs once per configuration, and the documentation is also accurate. Here is my reason to say that. createConfig creates a controller, source code reads the config. So the controller is the one doing everything;

  createBenchmark(fn, configs, options) {
    return new Benchmark(fn, configs, options);
  }

which is why I said

A function to be run once by the main "controller"

Copy link
Contributor Author

@0hmX 0hmX Jul 27, 2025

Choose a reason for hiding this comment

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

@RafaelGSS ping

Copy link
Member

Choose a reason for hiding this comment

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

So they aren't the same. Most of the time the teardown is necessary after each configuration

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@RafaelGSS, thanks for the explanation.

Sorry for being dumb and ignoring your previous comment about the per configuration. I understand now. But now I am unsure which part of the codebase I should look at or how the API for this new feature will look. If you have any ideas, please share them. Else, I will close this PR a few days from now.

Thanks again for your time!

Copy link
Member

Choose a reason for hiding this comment

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

I think that's why this feature doesn't exist. We fork the process for every configuration, so there's no way to "wrap" a setup and teardown to each configuration as functions aren't passed across environments -- That's why people usually do it in the main.

createEntryPoint(t);
},
teardown() {
tmpdir.refresh();
},
});

function main({ type, n }) {
tmpdir.refresh();
createEntryPoint(n);

switch (type) {
case '.js':
measureJSFile(n);
Expand All @@ -24,8 +31,6 @@ function main({ type, n }) {
case 'dir':
measureDir(n);
}

tmpdir.refresh();
}

function measureJSFile(n) {
Expand Down
15 changes: 15 additions & 0 deletions doc/contributing/writing-and-running-benchmarks.md
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,14 @@ The arguments of `createBenchmark` are:
Each configuration is a property with an array of possible values.
The configuration values can only be strings or numbers.
* `options` {Object} The benchmark options. Supported options:
* `setup` {Function} A function to be run once by the main "controller"
process before any benchmark child processes are spawned. Ideal for
preparing the environment for the entire benchmark suite.

* `teardown` {Function} A function to be run once by the main "controller"
process after all benchmark child processes have completed. Ideal for
cleaning up the environment.

* `flags` {Array} Contains node-specific command line flags to pass to
the child process.

Expand Down Expand Up @@ -585,6 +593,13 @@ const configs = {
const options = {
// Add --expose-internals in order to require internal modules in main
flags: ['--zero-fill-buffers'],
// Setup and teardown functions are run in the parent process, once.
setup() {
console.log('Running setup.');
},
teardown() {
console.log('Running teardown.');
},
};

// `main` and `configs` are required, `options` is optional.
Expand Down
Loading