Skip to content
Open
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
6 changes: 3 additions & 3 deletions benchmark/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
41 changes: 34 additions & 7 deletions benchmark/_node-bench-analysis.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -80,22 +91,32 @@ 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),
improvement,
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;
Expand Down Expand Up @@ -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('');
Expand All @@ -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)})`,
);
}
}
Expand Down
6 changes: 4 additions & 2 deletions benchmark/compare-node-bench.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
Expand Down Expand Up @@ -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;
Expand Down
Loading
Loading