Skip to content

benchmark: add --csv option to compare.js with --analyze #65922

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jasnell wants to merge 1 commit into
base: main
Choose a base branch
from
+123 −42
Open

benchmark: add --csv option to compare.js with --analyze #65922

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
102 changes: 60 additions & 42 deletions benchmark/compare.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict';

const { spawn, fork } = require('node:child_process');
const { closeSync, openSync, writeSync } = require('node:fs');
const { inspect } = require('util');
const path = require('path');
const CLI = require('./_cli.js');
Expand All @@ -27,7 +28,9 @@ const cli = new CLI(`usage: ./node compare.js [options] [--] <category> ...
--no-progress don't show benchmark progress indicator
--analyze perform statistical analysis after benchmarks
complete (Welch's t-test, effect size) instead
of printing csv output
of printing csv output to stdout
--csv filename write csv output to filename (can be combined
with --analyze). Use - to write to stdout.
--scale 1000 rate-to-integer multiplier for histogram
precision when using --analyze (default: 1000)
--max-regression N exit with code 1 if any statistically
Expand Down Expand Up @@ -60,6 +63,16 @@ if (benchmarks.length === 0) {
return;
}

const cvsToStdout = cli.optional.csv === '-';
const csvFd = cli.optional.csv === undefined || cvsToStdout ?
null :
openSync(cli.optional.csv, 'w');
const outputCsv = !analyze || csvFd !== null || cvsToStdout;

function writeCsv(line) {
writeSync(csvFd || process.stdout.fd, `${line}\n`);
}

// When --analyze is set, collect results for statistical analysis.
const results = analyze ? new Map() : null;

Expand All @@ -78,17 +91,19 @@ for (const filename of benchmarks) {
}
// queue.length = binary.length * runs * benchmarks.length

// Print csv header (unless analyzing inline).
if (!analyze) {
console.log('"binary","filename","configuration","rate","time"');
// Print csv header unless only analyzing inline.
if (outputCsv) {
writeCsv('"binary","filename","configuration","rate","time"');
}

const kStartOfQueue = 0;

const showProgress = !cli.optional['no-progress'];
const showProgress = !cli.optional['no-progress'] && !cvsToStdout;
let progress;
if (showProgress) {
progress = new BenchmarkProgress(queue, benchmarks, { analyze });
progress = new BenchmarkProgress(queue, benchmarks, {
analyze: analyze || csvFd !== null,
});
progress.startQueue(kStartOfQueue);
}

Expand Down Expand Up @@ -126,11 +141,13 @@ if (showProgress) {
results.set(name, { old: [], new: [] });
}
results.get(name)[job.binary].push(data.rate);
} else {
}

if (outputCsv) {
// Escape quotes (") for correct csv formatting
conf = conf.replace(/"/g, '""');
console.log(`"${job.binary}","${job.filename}","${conf}",` +
`${data.rate},${data.time}`);
const csvConf = conf.replace(/"/g, '""');
writeCsv(`"${job.binary}","${job.filename}","${csvConf}",` +
`${data.rate},${data.time}`);
}
if (showProgress) {
// One item in the subqueue has been completed.
Expand All @@ -153,8 +170,9 @@ if (showProgress) {
// If there are more benchmarks execute the next
if (i + 1 < queue.length) {
recursive(i + 1);
} else if (analyze) {
printAnalysis(results, scale, maxRegression);
} else {
if (csvFd !== null) closeSync(csvFd);
if (analyze) printAnalysis(results, scale, maxRegression);
}
});
})(kStartOfQueue);
Expand Down Expand Up @@ -261,41 +279,41 @@ function printAnalysis(results, scale, maxRegression) {
const pad = (s, n) => s + ' '.repeat(Math.max(0, n - s.length));
const rpad = (s, n) => ' '.repeat(Math.max(0, n - s.length)) + s;

console.log(`${pad('', maxNameLen)} confidence` +
` improvement accuracy (*) (**) (***)`);
writeSync(process.stdout.fd, `${pad('', maxNameLen)} confidence` +
` improvement accuracy (*) (**) (***)\n`);

for (const row of rows) {
const imp = `${row.improvement >= 0 ? '+' : ''}${row.improvement.toFixed(2)} %`;
console.log(
`${pad(row.name, maxNameLen)} ${pad(row.stars, 10)}` +
writeSync(process.stdout.fd,
`${pad(row.name, maxNameLen)} ${pad(row.stars, 10)}` +
` ${rpad(imp, 11)}` +
` ±${row.ci95.toFixed(2)}%` +
` ±${row.ci99.toFixed(2)}%` +
` ±${row.ci999.toFixed(2)}%` +
`${row.inconclusive ? ' (inconclusive)' : ''}`,
`${row.inconclusive ? ' (inconclusive)' : ''}\n`,
);
}

if (skipped > 0) {
console.log('');
console.log(
`Note: ${skipped} configuration${skipped === 1 ? ' was' : 's were'}` +
writeSync(process.stdout.fd, '\n');
writeSync(process.stdout.fd,
`Note: ${skipped} configuration${skipped === 1 ? ' was' : 's were'}` +
` skipped because Welch's t-test requires at least 2 samples per` +
` binary. Use --runs 2 or higher.`,
` binary. Use --runs 2 or higher.\n`,
);
}

// --- Bar chart visualization ---
printChart(rows, maxNameLen);

console.log('');
console.log(
`Rates were scaled by ${scale}x into HdrHistogram (3 significant figures).\n` +
`Use --scale to adjust precision if needed.\n`,
writeSync(process.stdout.fd, '\n');
writeSync(process.stdout.fd,
`Rates were scaled by ${scale}x into HdrHistogram (3 significant figures).\n` +
`Use --scale to adjust precision if needed.\n\n`,
);
const anyFamilyWise = rows.filter((r) => r.pAdjusted < 0.05).length;
console.log(
`Be aware that when doing many comparisons the risk of a false-positive\n` +
writeSync(process.stdout.fd,
`Be aware that when doing many comparisons the risk of a false-positive\n` +
`result increases. In this case, there are ${rows.length} comparisons, ` +
`you can thus\nexpect the following amount of false-positive results:\n` +
` ${(rows.length * 0.05).toFixed(2)} false positives, when considering ` +
Expand All @@ -307,19 +325,19 @@ function printAnalysis(results, scale, maxRegression) {
`\nThe stars above are per-benchmark and uncorrected. Adjusting for the ` +
`size of\nthis comparison set (Holm-Bonferroni), ${anyFamilyWise} ` +
`comparison${anyFamilyWise === 1 ? '' : 's'} remain${anyFamilyWise === 1 ? 's' : ''} ` +
`significant at 5%.\n--max-regression uses the corrected values.`,
`significant at 5%.\n--max-regression uses the corrected values.\n`,
);

// Gate: exit with error if any regression is shown to exceed the limit.
if (maxRegression > 0) {
if (underpowered > 0) {
console.log('');
console.log(
`Note: ${underpowered} of ${rows.length} comparison` +
writeSync(process.stdout.fd, '\n');
writeSync(process.stdout.fd,
`Note: ${underpowered} of ${rows.length} comparison` +
`${rows.length === 1 ? '' : 's'} could not resolve an effect as ` +
`small as ${maxRegression}%, and are marked (inconclusive). They are ` +
`not\nevidence of no regression -- the samples are too noisy to tell. ` +
`Raise --runs,\nor pin cores with --set CPUSET, to narrow them.`,
`Raise --runs,\nor pin cores with --set CPUSET, to narrow them.\n`,
);
}

Expand All @@ -340,18 +358,18 @@ function printAnalysis(results, scale, maxRegression) {
);

if (failures.length > 0) {
console.log('');
console.log(
`FAIL: ${failures.length} benchmark${failures.length === 1 ? '' : 's'}` +
writeSync(process.stdout.fd, '\n');
writeSync(process.stdout.fd,
`FAIL: ${failures.length} benchmark${failures.length === 1 ? '' : 's'}` +
` regressed by more than ${maxRegression}%` +
` (interval excludes the threshold,\n` +
`family-wise corrected across ${rows.length} comparisons):`,
`family-wise corrected across ${rows.length} comparisons):\n`,
);
for (const f of failures) {
console.log(
` ${f.name} ${f.improvement.toFixed(2)}% ` +
writeSync(process.stdout.fd,
` ${f.name} ${f.improvement.toFixed(2)}% ` +
`(95% CI up to ${(f.improvement + f.ci95).toFixed(2)}%, ` +
`adjusted p=${f.pAdjusted.toExponential(2)})`,
`adjusted p=${f.pAdjusted.toExponential(2)})\n`,
);
}
process.exitCode = 1;
Expand Down Expand Up @@ -388,8 +406,8 @@ function printChart(rows, maxNameLen) {
axisCenter +
' '.repeat(Math.max(0, halfWidth - Math.ceil(axisCenter.length / 2) - axisRight.length)) +
axisRight;
console.log('');
console.log(leftLabel);
writeSync(process.stdout.fd, '\n');
writeSync(process.stdout.fd, `${leftLabel}\n`);

for (const row of rows) {
const imp = row.improvement;
Expand Down Expand Up @@ -421,6 +439,6 @@ function printChart(rows, maxNameLen) {

const label = `${row.improvement >= 0 ? '+' : ''}${row.improvement.toFixed(2)}%`;
const sig = row.stars.trim();
console.log(`${pad(row.name, maxNameLen)} ${chars.join('')} ${label} ${sig}`);
writeSync(process.stdout.fd, `${pad(row.name, maxNameLen)} ${chars.join('')} ${label} ${sig}\n`);
}
}
17 changes: 17 additions & 0 deletions doc/contributing/writing-and-running-benchmarks.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,8 @@ module, you can use the `--filter` option:_
--set variable=value set benchmark variable (can be repeated)
--no-progress don't show benchmark progress indicator
--analyze perform statistical analysis inline (no R needed)
--csv filename write csv output to filename (can be combined
with --analyze)
--scale 1000 rate multiplier for --analyze precision
--max-regression N exit with code 1 if any significant regression
exceeds N% (implies --analyze)
Expand All @@ -429,6 +431,14 @@ The simplest way to get statistical results is to pass `--analyze`:
node benchmark/compare.js --old ./node-main --new ./node-pr-5134 --analyze string_decoder
```

Use `--csv` to retain the raw benchmark results. If you pass both `--csv` and
`--analyze`, both the raw results and the analysis are printed:

```bash
node benchmark/compare.js --old ./node-main --new ./node-pr-5134 \
--analyze --csv compare-pr-5134.csv string_decoder
```

This runs the benchmarks and prints the analysis directly:

```console
Expand All @@ -438,6 +448,13 @@ string_decoder/string-decoder.js n=2500000 chunkLen=16 inLen=128 encoding='utf8'
...
```

Use `-csv -` to output the raw results to stdout with the analysis.

```bash
node benchmark/compare.js --old ./node-main --new ./node-pr-5134 \
--analyze --csv - string_decoder
```

The `--analyze` mode uses the histogram API's `welchTest()` method to perform
the same Welch's t-test that the R script uses. Benchmark rates are scaled to
integers for the histogram (controlled by `--scale`, default 1000). With the
Expand Down
46 changes: 46 additions & 0 deletions test/parallel/test-benchmark-compare.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
'use strict';

require('../common');

const assert = require('node:assert');
const { spawnSyncAndExitWithoutError } = require('../common/child_process');
const { readFileSync } = require('node:fs');
const path = require('node:path');
const tmpdir = require('../common/tmpdir');

const compare = path.resolve(__dirname, '../../benchmark/compare.js');

tmpdir.refresh();

const csv = tmpdir.resolve('compare.csv');
spawnSyncAndExitWithoutError(process.execPath, [
compare,
'--old', process.execPath,
'--new', process.execPath,
'--runs', '1',
'--filter', 'buffer-compare-offset.js',
'--set', 'method=offset',
'--set', 'size=16',
'--set', 'n=1',
'--no-progress',
'--analyze',
'--csv', csv,
'buffers',
], {
encoding: 'utf8',
timeout: 30_000,
}, {
stderr: '',
stdout(stdout) {
assert.match(stdout, /confidence\s+improvement\s+accuracy/);
assert.doesNotMatch(stdout, /"binary","filename"/);
},
});

const lines = readFileSync(csv, 'utf8').trim().split('\n');
const filename = path.join('buffers', 'buffer-compare-offset.js');
assert.strictEqual(lines[0],
'"binary","filename","configuration","rate","time"');
assert.strictEqual(lines.length, 3);
assert(lines[1].startsWith(`"old","${filename}",`));
assert(lines[2].startsWith(`"new","${filename}",`));
Loading