fix(ci): compare integrated benchmarks on the same runner (#15228)
Compare integrated benchmark samples against main measured on the same runner. Keep absolute minimum timings in Bencher for historical tracking without letting cross-machine historical alerts overwrite the PR regression check. Require at least nine successful samples per target. Fail when the median slowdown and retained sample separation both exceed 5% of main's median or 2 ms, whichever is larger. Trim up to 10% from each tail. Report other median slowdowns above that tolerance as inconclusive. This conservative noise guard is not a statistical confidence interval; smaller or noisy regressions can pass, and sequential measurements remain vulnerable to runner drift. Run the comparison from trusted default-branch code in the privileged comment workflow, publish the report before enforcing the result, and preserve the existing check name on the triggering PR commit. Keep the peer-heavy Rust-versus-TypeScript assertion unchanged. Cover regressions, shared machine slowdowns, outliers, invalid data, all scenario and engine combinations, command names, CLI exit codes, and the duration-dependent tolerance with Node tests. Keep the explanation in the existing benchmark documentation.
This commit is contained in:
1 parent
222513ac46
commit
bd4d526530
5 files changed
+320
-32
No files matched your search
@@ -0,0 +1,82 @@
|
||||
import { readFile } from 'node:fs/promises'
|
||||
import { join, resolve } from 'node:path'
|
||||
import { pathToFileURL } from 'node:url'
|
||||
|
||||
export const scenarios = [
|
||||
'ISOLATED_FRESH_RESTORE_COLD_CACHE_COLD_STORE',
|
||||
'ISOLATED_FRESH_RESTORE_HOT_CACHE_HOT_STORE',
|
||||
'ISOLATED_REPEAT_INSTALL_HOT_CACHE_HOT_STORE',
|
||||
'ISOLATED_REPEAT_INSTALL_COLD_CACHE_HOT_STORE',
|
||||
'ISOLATED_FRESH_INSTALL_COLD_CACHE_COLD_STORE',
|
||||
'ISOLATED_FRESH_INSTALL_HOT_CACHE_HOT_STORE',
|
||||
'ISOLATED_FRESH_INSTALL_COLD_CACHE_HOT_STORE',
|
||||
'ISOLATED_FRESH_RESOLVE_HOT_CACHE_OFFLINE',
|
||||
'ISOLATED_PEER_HEAVY_RESOLVE_HOT_CACHE_OFFLINE',
|
||||
'ISOLATED_LINKED_WORKSPACE_RESOLVE_HOT_CACHE_OFFLINE',
|
||||
'ISOLATED_FRESH_RESTORE_COLD_CACHE_COLD_STORE_COLD_PNPR',
|
||||
'GVS_FRESH_RESTORE_HOT_CACHE_HOT_STORE',
|
||||
]
|
||||
|
||||
export function compare (report, engine) {
|
||||
const head = samples(report, `${engine}@HEAD`)
|
||||
const base = samples(report, `${engine}@main`)
|
||||
const headMedian = median(head)
|
||||
const baseMedian = median(base)
|
||||
const tolerance = Math.max(baseMedian * 0.05, 0.002)
|
||||
const ratio = headMedian / baseMedian
|
||||
// Trim at most 10% of each tail. This is an empirical noise guard, not a
|
||||
// confidence interval: hyperfine measures commands sequentially.
|
||||
const headLow = head[Math.floor(head.length / 10)]
|
||||
const baseHigh = base[base.length - 1 - Math.floor(base.length / 10)]
|
||||
const status = headMedian <= baseMedian + tolerance ? 'Within tolerance' : headLow > baseHigh + tolerance
|
||||
? 'Regression' : 'Inconclusive (overlapping samples)'
|
||||
return { head: headMedian, base: baseMedian, ratio, tolerance, status }
|
||||
}
|
||||
|
||||
function samples (report, name) {
|
||||
const matches = report.results.filter(result => (result.command_name ?? result.command) === name)
|
||||
if (matches.length !== 1) throw new Error(`Expected one result for ${name}`)
|
||||
const { times, exit_codes: exitCodes } = matches[0]
|
||||
if (!Array.isArray(times) || times.length < 9 || times.some(time => !Number.isFinite(time) || time <= 0)) {
|
||||
throw new Error(`Expected at least 9 positive finite samples for ${name}`)
|
||||
}
|
||||
if (!Array.isArray(exitCodes) || exitCodes.length !== times.length || exitCodes.some(code => code !== 0)) {
|
||||
throw new Error(`Missing or unsuccessful exit codes for ${name}`)
|
||||
}
|
||||
return [...times].sort((a, b) => a - b)
|
||||
}
|
||||
|
||||
function median (values) {
|
||||
const middle = Math.floor(values.length / 2)
|
||||
return values.length % 2 ? values[middle] : (values[middle - 1] + values[middle]) / 2
|
||||
}
|
||||
|
||||
export async function renderComparison (directory) {
|
||||
const lines = [
|
||||
'### Same-run regression check',
|
||||
'',
|
||||
'HEAD and main are measured on the same runner. The tolerance is 5% of the main median or 2 ms, whichever is larger. A regression requires both the median slowdown and the gap between the fastest retained HEAD sample and slowest retained main sample to exceed this tolerance, after trimming up to 10% from each tail. This conservative noise guard is not a statistical confidence interval.',
|
||||
'',
|
||||
'Inconclusive rows do not fail the check. They remain visible for performance review. Bencher retains absolute minimum timings for historical tracking; historical alerts do not gate this PR. Sequential measurements can still be affected by runner drift.',
|
||||
'',
|
||||
'| Scenario | Engine | main median | HEAD median | Change | Tolerance | Result |',
|
||||
'| --- | --- | ---: | ---: | ---: | ---: | --- |',
|
||||
]
|
||||
let failed = false
|
||||
for (const scenario of scenarios) {
|
||||
const report = JSON.parse(await readFile(join(directory, `BENCHMARK_REPORT_${scenario}.json`), 'utf8'))
|
||||
const engines = scenario.includes('PEER_HEAVY') || scenario.includes('LINKED_WORKSPACE') ? ['pacquet'] : ['pacquet', 'pnpr']
|
||||
for (const engine of engines) {
|
||||
const result = compare(report, engine)
|
||||
failed ||= result.status === 'Regression'
|
||||
lines.push(`| ${scenario.toLowerCase().replaceAll('_', '-')} | ${engine} | ${(result.base * 1000).toFixed(2)} ms | ${(result.head * 1000).toFixed(2)} ms | ${((result.ratio - 1) * 100).toFixed(1)}% | ${(result.tolerance * 1000).toFixed(2)} ms | ${result.status} |`)
|
||||
}
|
||||
}
|
||||
return { markdown: lines.join('\n') + '\n', failed }
|
||||
}
|
||||
|
||||
if (process.argv[1] && import.meta.url === pathToFileURL(resolve(process.argv[1])).href) {
|
||||
const { markdown, failed } = await renderComparison(process.argv[2])
|
||||
console.log(markdown)
|
||||
process.exitCode = failed ? 1 : 0
|
||||
}
|
||||
@@ -0,0 +1,116 @@
|
||||
import assert from 'node:assert/strict'
|
||||
import { spawnSync } from 'node:child_process'
|
||||
import { mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { fileURLToPath } from 'node:url'
|
||||
import { test } from 'node:test'
|
||||
import { compare, renderComparison, scenarios } from './compare-integrated-benchmarks.mjs'
|
||||
|
||||
function command (name, times) {
|
||||
return { command: name, times, exit_codes: times.map(() => 0) }
|
||||
}
|
||||
|
||||
function report (head, base) {
|
||||
return { results: [command('pacquet@HEAD', head), command('pacquet@main', base)] }
|
||||
}
|
||||
|
||||
const stable = Array(10).fill(1)
|
||||
|
||||
test('a slower machine affects both revisions without causing an alert', () => {
|
||||
assert.equal(compare(report(stable.map(x => x * 3.1), stable.map(x => x * 3)), 'pacquet').status, 'Within tolerance')
|
||||
})
|
||||
|
||||
test('a consistent regression fails, an improvement passes', () => {
|
||||
assert.equal(compare(report(Array(9).fill(1.3), Array(9).fill(1)), 'pacquet').status, 'Regression')
|
||||
assert.equal(compare(report(stable, stable.map(x => x * 1.3)), 'pacquet').status, 'Within tolerance')
|
||||
})
|
||||
|
||||
test('multi-second scenarios catch consistent slowdowns below 20%', () => {
|
||||
const result = compare(report(Array(10).fill(3.18), Array(10).fill(3)), 'pacquet')
|
||||
assert.equal(result.status, 'Regression')
|
||||
assert.equal(result.tolerance, 3 * 0.05)
|
||||
})
|
||||
|
||||
test('short scenarios tolerate up to 2 ms but still catch larger slowdowns', () => {
|
||||
const base = Array(10).fill(0.008)
|
||||
assert.equal(compare(report(Array(10).fill(0.009), base), 'pacquet').status, 'Within tolerance')
|
||||
assert.equal(compare(report(Array(10).fill(0.010), base), 'pacquet').status, 'Within tolerance')
|
||||
const result = compare(report(Array(10).fill(0.011), base), 'pacquet')
|
||||
assert.equal(result.status, 'Regression')
|
||||
assert.equal(result.tolerance, 0.002)
|
||||
})
|
||||
|
||||
test('exactly 5% is tolerated when the relative threshold is larger', () => {
|
||||
assert.equal(compare(report(Array(10).fill(1.05), stable), 'pacquet').status, 'Within tolerance')
|
||||
})
|
||||
|
||||
test('a noisy slowdown is explicitly inconclusive', () => {
|
||||
assert.equal(compare(report([1, 1, 1.3, 1.3, 1.3, 1.3, 1.4, 1.4, 2, 3], stable), 'pacquet').status, 'Inconclusive (overlapping samples)')
|
||||
})
|
||||
|
||||
test('isolated outliers neither create nor conceal a regression', () => {
|
||||
assert.equal(compare(report([...stable.slice(1), 10], stable), 'pacquet').status, 'Within tolerance')
|
||||
assert.equal(compare(report([0.5, ...Array(9).fill(1.3)], [...stable.slice(1), 10]), 'pacquet').status, 'Regression')
|
||||
})
|
||||
|
||||
test('missing, duplicate, failed, undersampled and invalid data fail closed', () => {
|
||||
const valid = report(stable, stable)
|
||||
const invalid = [
|
||||
{ results: [] },
|
||||
{ results: [...valid.results, valid.results[0]] },
|
||||
report(stable.slice(0, 8), stable),
|
||||
report([...stable.slice(1), NaN], stable),
|
||||
report([...stable.slice(1), 0], stable),
|
||||
{ results: [{ ...valid.results[0], exit_codes: [0] }, valid.results[1]] },
|
||||
{ results: [{ ...valid.results[0], exit_codes: [1, ...Array(9).fill(0)] }, valid.results[1]] },
|
||||
]
|
||||
for (const input of invalid) assert.throws(() => compare(input, 'pacquet'))
|
||||
})
|
||||
|
||||
test('all scenarios and both engines are checked, and missing reports fail', async () => {
|
||||
const directory = await mkdtemp(join(tmpdir(), 'benchmark-comparison-'))
|
||||
try {
|
||||
for (const scenario of scenarios) {
|
||||
const input = report(stable, stable)
|
||||
if (!scenario.includes('PEER_HEAVY') && !scenario.includes('LINKED_WORKSPACE')) {
|
||||
input.results.push(command('pnpr@HEAD', Array(10).fill(1.4)), command('pnpr@main', stable))
|
||||
}
|
||||
await writeFile(join(directory, `BENCHMARK_REPORT_${scenario}.json`), JSON.stringify(input))
|
||||
}
|
||||
const result = await renderComparison(directory)
|
||||
assert.equal(result.failed, true)
|
||||
assert.equal(result.markdown.match(/\| Regression \|/g).length, 10)
|
||||
assert.equal(result.markdown.match(/\| Within tolerance \|/g).length, 12)
|
||||
const cli = new URL('./compare-integrated-benchmarks.mjs', import.meta.url)
|
||||
const run = spawnSync(process.execPath, [fileURLToPath(cli), directory], { encoding: 'utf8' })
|
||||
assert.equal(run.status, 1)
|
||||
assert.match(run.stdout, /Regression/)
|
||||
assert.equal(run.stderr, '')
|
||||
for (const scenario of scenarios) {
|
||||
const input = { results: ['pacquet', 'pnpr'].flatMap(engine => [command(`${engine}@HEAD`, stable), command(`${engine}@main`, stable)]) }
|
||||
await writeFile(join(directory, `BENCHMARK_REPORT_${scenario}.json`), JSON.stringify(input))
|
||||
}
|
||||
assert.equal(spawnSync(process.execPath, [fileURLToPath(cli), directory]).status, 0)
|
||||
await rm(join(directory, `BENCHMARK_REPORT_${scenarios[0]}.json`))
|
||||
await assert.rejects(renderComparison(directory), /ENOENT/)
|
||||
} finally {
|
||||
await rm(directory, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
test('comparison and Bencher uploads cover every matrix scenario', async () => {
|
||||
const workflow = await readFile(new URL('../workflows/pacquet-integrated-benchmark.yml', import.meta.url), 'utf8')
|
||||
const tags = [...workflow.matchAll(/'([A-Z_]+):[a-z][^']+'/g)].map(match => match[1])
|
||||
const matrix = workflow.match(/ scenario:\n((?: - [^\n]+\n)+)/)[1]
|
||||
const matrixTags = [...matrix.matchAll(/- ([a-z][\w.-]+)/g)].map(match => match[1]
|
||||
.replace('-linker.', '_').replaceAll(/[.-]/g, '_').toUpperCase())
|
||||
assert.deepEqual([...new Set(tags)].sort(), [...matrixTags].sort())
|
||||
assert.deepEqual([...scenarios].sort(), [...matrixTags].sort())
|
||||
})
|
||||
|
||||
test('hyperfine command names identify targets when commands are shell scripts', () => {
|
||||
const input = report(stable, stable)
|
||||
input.results = input.results.map(result => ({ ...result, command_name: result.command, command: '/tmp/run.sh' }))
|
||||
assert.equal(compare(input, 'pacquet').status, 'Within tolerance')
|
||||
})
|
||||
Reference in new issue
Block a user