From fc4501f5f09adaecf5df0efeeedf1f366356bc19 Mon Sep 17 00:00:00 2001 From: Marc Rousavy Date: Sat, 5 Sep 2026 16:34:05 +0300 Subject: [PATCH] perf: share calibrated work across fresh base and head processes --- .github/PERFORMANCE.md | 12 +- .github/workflows/performance.yml | 9 +- apps/benchmark/README.md | 7 +- .../benchmark/src/benchmarks/BenchmarkApp.tsx | 35 +++- apps/benchmark/src/benchmarks/batch.ts | 6 +- apps/benchmark/src/benchmarks/calibration.ts | 6 +- apps/benchmark/src/benchmarks/runner.ts | 111 +++++++----- apps/benchmark/src/benchmarks/types.ts | 14 +- scripts/performance/comparison.test.ts | 15 ++ scripts/performance/comparison.ts | 37 +++- scripts/performance/isolated-cases.ts | 4 +- scripts/performance/publish.test.ts | 13 ++ scripts/performance/publish.ts | 8 +- scripts/performance/receive.ts | 15 +- scripts/performance/report-validation.test.ts | 38 ++++ scripts/performance/report.ts | 8 + scripts/performance/run-device.ts | 62 ++++++- scripts/performance/run-sequence.test.ts | 166 ++++++++++++++++++ scripts/performance/run-sequence.ts | 44 ++++- scripts/performance/runner.test.ts | 92 +++++++--- scripts/performance/schema.ts | 52 ++++-- scripts/performance/validate-report.ts | 52 +++++- 22 files changed, 663 insertions(+), 143 deletions(-) create mode 100644 scripts/performance/run-sequence.test.ts diff --git a/.github/PERFORMANCE.md b/.github/PERFORMANCE.md index 84ec6fceb..c279e5e62 100644 --- a/.github/PERFORMANCE.md +++ b/.github/PERFORMANCE.md @@ -6,8 +6,10 @@ order for the second pair. Each case gets a fresh app process; installation, startup, transport and process restarts are outside timing. There is no automatic third pair. Manual reruns are retained as identifiable workflow attempts. -Each case records twenty ordered batch averages after five warmup batches. -Calibration targets 150 ms of timed work; this target does not establish steady +A separate base calibration process chooses one per-case iteration and chunk +plan. Both revisions then use that plan for five warmup batches and twenty ordered +measurements in fresh processes. Incompatible counts fail rather than silently +shortening head work. Calibration targets 150 ms of timed work; this target does not establish steady state or erase drift. Allocation-heavy cases sum bounded timed chunks with explicit cleanup outside timing. Raw `iterations`, `chunkIterations` and ordered `samplesNsPerOp` describe the work; timed sample milliseconds are @@ -27,12 +29,14 @@ failures still fail CI. Turning observed differences into a regression gate need empirical validation on unchanged commits and intentional slowdowns on each unchanged suite/testbed. No Promise case is permanently exempt. Scheduled/manual runs with the same base and head SHA measure baseline variation explicitly. -Changed benchmark definitions require a new baseline and are not compared. +Changed benchmark definitions run two head-only measurements as a new baseline, +without executing the old base app or publishing an invented paired baseline. +This also handles the first rollout of a new runner protocol. ## Artifacts and publishing The canonical artifact is `performance-report-`: raw JSON for every -base/head process plus `performance-report.json` with repository, revisions, +measured process and discarded calibration plan, plus `performance-report.json` with repository, revisions, workflow run and attempt provenance. Artifacts remain available for 30 days. The PR comment links its exact immutable artifact ID; downloads require GitHub access. An agent can inspect the JSON instead of scraping the rendered table. diff --git a/.github/workflows/performance.yml b/.github/workflows/performance.yml index 9f08aaa38..39af7153d 100644 --- a/.github/workflows/performance.yml +++ b/.github/workflows/performance.yml @@ -370,15 +370,20 @@ jobs: PR_NUMBER: ${{ needs.prepare.outputs.pr_number }} run: | mkdir -p performance-report/raw/android performance-report/raw/ios + cmp artifacts/android/suite.json artifacts/ios/suite.json bun scripts/performance/report.ts \ + --suite artifacts/android/suite.json \ --output performance-report/performance-report.json \ --repository "$REPOSITORY" \ --event-name "${{ github.event_name }}" \ --pull-request "$PR_NUMBER" \ --base-sha "${{ needs.prepare.outputs.base_sha }}" \ --head-sha "${{ needs.prepare.outputs.head_sha }}" - cp artifacts/android/base-*.json artifacts/android/head-*.json performance-report/raw/android/ - cp artifacts/ios/base-*.json artifacts/ios/head-*.json performance-report/raw/ios/ + for PLATFORM in android ios; do + for RESULT in artifacts/"$PLATFORM"/{base,head,calibration}-*.json; do + if [[ -f "$RESULT" ]]; then cp "$RESULT" performance-report/raw/"$PLATFORM"/; fi + done + done - name: Upload aggregate report if: needs.prepare.outputs.relevant == 'true' diff --git a/apps/benchmark/README.md b/apps/benchmark/README.md index 53b775364..eef4f16db 100644 --- a/apps/benchmark/README.md +++ b/apps/benchmark/README.md @@ -61,9 +61,10 @@ Local runs do not upload results. Each metric targets 150 ms of timed work per sample (roughly 100–200 ms), using round iteration counts with two significant digits, such as -1,500,000 or 24,000. Calibration can grow or shrink the count and is rechecked -after five warmup batches. That count is then frozen for twenty measured samples; -slow samples are retained, not discarded or adaptively shortened. +1,500,000 or 24,000. Calibration chooses the count in a separate process that is discarded. The +measurement process performs five warmup batches and twenty samples using those +fixed counts. In paired runs, both revisions share the base-derived count and +chunk size for each case. Slow samples are retained without shortening the work. Allocation-heavy cases split a sample into bounded chunks, collecting garbage after each chunk and yielding for native cleanup at most every four chunks, diff --git a/apps/benchmark/src/benchmarks/BenchmarkApp.tsx b/apps/benchmark/src/benchmarks/BenchmarkApp.tsx index 6132d935b..87410fba1 100644 --- a/apps/benchmark/src/benchmarks/BenchmarkApp.tsx +++ b/apps/benchmark/src/benchmarks/BenchmarkApp.tsx @@ -1,3 +1,4 @@ +import { calibrateBenchmarkDefinitions } from './runner' import * as React from 'react' import { StyleSheet, Text, View } from 'react-native' import { @@ -23,6 +24,12 @@ function isRunConfiguration( if (value == null || typeof value !== 'object') return false const candidate = value as Partial return ( + (candidate.calibration === undefined || candidate.calibration === true) && + (candidate.work === undefined || + (candidate.work != null && + typeof candidate.work.id === 'string' && + Number.isSafeInteger(candidate.work.iterations) && + Number.isSafeInteger(candidate.work.chunkIterations))) && typeof candidate.runId === 'string' && typeof candidate.reverse === 'boolean' && typeof candidate.commitSha === 'string' && @@ -87,17 +94,35 @@ async function run(): Promise { ) if (selected.length === 0) throw new Error('Requested benchmark index is outside the suite.') - const metrics = await runBenchmarkDefinitions(selected, { - ...RUNNER_OPTIONS, - reverse: configuration.reverse, - }) + const runner = configuration.calibration + ? { ...RUNNER_OPTIONS, warmupCount: 0, sampleCount: 0 } + : RUNNER_OPTIONS + const metrics = configuration.calibration + ? ( + await calibrateBenchmarkDefinitions( + selected, + runner.targetBatchDurationMs + ) + ).map((work, index) => ({ + ...work, + version: selected[index]!.version, + family: selected[index]!.family, + implementation: selected[index]!.implementation, + samplesNsPerOp: [], + checksum: 0, + })) + : await runBenchmarkDefinitions( + selected, + { ...runner, reverse: configuration.reverse }, + configuration.work == null ? [] : [configuration.work] + ) return { schemaVersion: 1, suiteVersion: 1, configuration, environment, - runner: RUNNER_OPTIONS, + runner, startedAt, durationMs: performance.now() - start, metrics, diff --git a/apps/benchmark/src/benchmarks/batch.ts b/apps/benchmark/src/benchmarks/batch.ts index 7e8943d04..666158d7b 100644 --- a/apps/benchmark/src/benchmarks/batch.ts +++ b/apps/benchmark/src/benchmarks/batch.ts @@ -20,12 +20,12 @@ export const benchmarkRuntime: BenchmarkRuntime = { export async function executeBatch( definition: BenchmarkDefinition, iterations: number, - runtime: BenchmarkRuntime -): Promise<{ durationMs: number; checksum: number }> { - const chunkIterations = Math.min( + runtime: BenchmarkRuntime, + chunkIterations = Math.min( iterations, definition.maxChunkIterations ?? iterations ) +): Promise<{ durationMs: number; checksum: number }> { if (!Number.isSafeInteger(chunkIterations) || chunkIterations < 1) { throw new Error(`Benchmark ${definition.id} has an invalid chunk size.`) } diff --git a/apps/benchmark/src/benchmarks/calibration.ts b/apps/benchmark/src/benchmarks/calibration.ts index 2a622bb7e..9ed397ffe 100644 --- a/apps/benchmark/src/benchmarks/calibration.ts +++ b/apps/benchmark/src/benchmarks/calibration.ts @@ -20,7 +20,7 @@ export async function calibrateIterations( if (!Number.isFinite(durationMs) || durationMs < 0) { throw new Error('Calibration requires a finite, non-negative duration.') } - // Aim inside the requested 100–200 ms window, leaving room for drift. + // Choose a practical batch duration; this does not establish steady state. if (durationMs >= targetMs * 0.8 && durationMs <= targetMs * 1.2) { if (++confirmations === 2) return iterations continue @@ -39,5 +39,7 @@ export async function calibrateIterations( maximum ) } - throw new Error('Batch duration did not stabilize during calibration.') + throw new Error( + 'Calibration could not reach the target duration within its step limit.' + ) } diff --git a/apps/benchmark/src/benchmarks/runner.ts b/apps/benchmark/src/benchmarks/runner.ts index f215963e8..d49b95e3f 100644 --- a/apps/benchmark/src/benchmarks/runner.ts +++ b/apps/benchmark/src/benchmarks/runner.ts @@ -1,84 +1,101 @@ import { median } from './statistics' -import { calibrateIterations, roundIterations } from './calibration' +import { calibrateIterations } from './calibration' import { benchmarkRuntime, executeBatch, type BenchmarkRuntime } from './batch' import type { BenchmarkDefinition, BenchmarkMetric, BenchmarkRunnerOptions, + BenchmarkWork, } from './types' +export async function calibrateBenchmarkDefinitions( + definitions: readonly BenchmarkDefinition[], + targetBatchDurationMs: number, + runtime: BenchmarkRuntime = benchmarkRuntime +): Promise { + const work: BenchmarkWork[] = [] + for (const definition of definitions) { + const iterations = await calibrateIterations( + async (count) => + (await executeBatch(definition, count, runtime)).durationMs, + targetBatchDurationMs, + definition.initialIterations, + definition.maxIterations + ) + work.push({ + id: definition.id, + iterations, + chunkIterations: Math.min( + iterations, + definition.maxChunkIterations ?? iterations + ), + }) + } + return work +} + export async function runBenchmarkDefinitions( definitions: readonly BenchmarkDefinition[], options: BenchmarkRunnerOptions, + work: readonly BenchmarkWork[], runtime: BenchmarkRuntime = benchmarkRuntime ): Promise { const ordered = options.reverse ? [...definitions].reverse() : [...definitions] const metrics: BenchmarkMetric[] = [] - for (const definition of ordered) { - let iterations = definition.initialIterations ?? 1_000 - let checksum = 0 - for (let attempt = 0; attempt < 3; attempt++) { - iterations = await calibrateIterations( - async (count) => - (await executeBatch(definition, count, runtime)).durationMs, - options.targetBatchDurationMs, - iterations, - definition.maxIterations + const plan = work.find((entry) => entry.id === definition.id) + if ( + plan == null || + !Number.isSafeInteger(plan.iterations) || + plan.iterations < 1 || + plan.iterations > (definition.maxIterations ?? 100_000_000) || + !Number.isSafeInteger(plan.chunkIterations) || + plan.chunkIterations < 1 || + plan.chunkIterations > plan.iterations || + plan.chunkIterations > (definition.maxChunkIterations ?? plan.iterations) + ) { + throw new Error( + `Missing or incompatible work counts for ${definition.id}.` ) - checksum = 0 - const warmupDurations: number[] = [] - for (let index = 0; index < options.warmupCount; index++) { - const warmup = await executeBatch(definition, iterations, runtime) - checksum += warmup.checksum - warmupDurations.push(warmup.durationMs) - } - const warmupMedian = median(warmupDurations) - if ( - warmupMedian >= options.targetBatchDurationMs * (2 / 3) && - warmupMedian <= options.targetBatchDurationMs * (4 / 3) - ) - break - if (attempt === 2) { - throw new Error( - `Benchmark ${definition.id} did not stabilize after warmup.` + } + let checksum = 0 + for (let index = 0; index < options.warmupCount; index++) { + checksum += ( + await executeBatch( + definition, + plan.iterations, + runtime, + plan.chunkIterations ) - } - iterations = roundIterations( - (iterations * options.targetBatchDurationMs) / warmupMedian, - definition.maxIterations ?? 100_000_000 - ) + ).checksum } - - // Freeze the count for all measured samples. Do not discard slow samples - // or tune iterations from measured results: that would bias the comparison. - const samplesNsPerOp = new Array(options.sampleCount) + // The same plan is used for both binaries. Preserve every ordered sample, + // even if head is much slower than the calibration target. + const samplesNsPerOp: number[] = [] for (let index = 0; index < options.sampleCount; index++) { - const sample = await executeBatch(definition, iterations, runtime) + const sample = await executeBatch( + definition, + plan.iterations, + runtime, + plan.chunkIterations + ) checksum += sample.checksum - samplesNsPerOp[index] = (sample.durationMs * 1_000_000) / iterations + samplesNsPerOp.push((sample.durationMs * 1_000_000) / plan.iterations) } - const metric: BenchmarkMetric = { - id: definition.id, + ...plan, version: definition.version, family: definition.family, implementation: definition.implementation, - iterations, - chunkIterations: Math.min( - iterations, - definition.maxChunkIterations ?? iterations - ), samplesNsPerOp, checksum, } metrics.push(metric) console.info( - `[NitroBenchmark] ${metric.id}: ${median(metric.samplesNsPerOp).toFixed(2)} ns/op; ${iterations} ops/sample, chunks of ${metric.chunkIterations}, median timed batch ${((median(metric.samplesNsPerOp) * iterations) / 1_000_000).toFixed(1)} ms` + `[NitroBenchmark] ${metric.id}: ${median(samplesNsPerOp).toFixed(2)} ns/op; ${plan.iterations} ops/sample, chunks of ${plan.chunkIterations}` ) } - return metrics } diff --git a/apps/benchmark/src/benchmarks/types.ts b/apps/benchmark/src/benchmarks/types.ts index 89214e7f3..cd2987539 100644 --- a/apps/benchmark/src/benchmarks/types.ts +++ b/apps/benchmark/src/benchmarks/types.ts @@ -54,19 +54,25 @@ export interface BenchmarkRunnerOptions { reverse: boolean } -export interface BenchmarkMetric { +export interface BenchmarkWork { id: string - version: number - family: BenchmarkFamily - implementation: BenchmarkImplementation iterations: number /** Maximum operations between untimed garbage collections. */ chunkIterations: number +} + +export interface BenchmarkMetric extends BenchmarkWork { + version: number + family: BenchmarkFamily + implementation: BenchmarkImplementation samplesNsPerOp: number[] checksum: number } export interface BenchmarkRunConfiguration { + /** Calibration is discarded; measurement always uses a fresh process. */ + calibration?: true + work?: BenchmarkWork /** Select one case in suite order for a fresh-process measurement. */ benchmarkIndex?: number runId: string diff --git a/scripts/performance/comparison.test.ts b/scripts/performance/comparison.test.ts index 69b3ea861..6f4acc501 100644 --- a/scripts/performance/comparison.test.ts +++ b/scripts/performance/comparison.test.ts @@ -100,4 +100,19 @@ describe('performance comparison', () => { ).suiteComparable ).toBe(false) }) + test('rejects unequal work and calibration data even if timings look identical', () => { + const base = run(BASE_SHA, [100, 100]) + const head = run(HEAD_SHA, [100, 100]) + head.metrics[0]!.iterations = 9_000 + expect(() => compareRuns([base], [head])).toThrow('unequal work') + head.metrics[0]!.iterations = base.metrics[0]!.iterations + head.configuration.calibration = true + expect(() => compareRuns([base], [head])).toThrow('Calibration runs') + expect(() => validateBenchmarkRun(head)).toThrow( + 'Calibration must not contain' + ) + head.configuration.calibration = undefined + head.metrics[0]!.samplesNsPerOp = [] + expect(() => validateBenchmarkRun(head)).toThrow('Sample count') + }) }) diff --git a/scripts/performance/comparison.ts b/scripts/performance/comparison.ts index a01b8de99..f39f55cbb 100644 --- a/scripts/performance/comparison.ts +++ b/scripts/performance/comparison.ts @@ -51,6 +51,9 @@ export function compareRuns( if (baseRuns.length === 0 || baseRuns.length !== headRuns.length) { throw new Error('A matching base run is required for every head run.') } + if ([...baseRuns, ...headRuns].some((run) => run.configuration.calibration)) { + throw new Error('Calibration runs cannot be compared as measurements.') + } const { platform, commitSha: baseSha, suiteHash } = baseRuns[0]!.configuration const headSha = headRuns[0]!.configuration.commitSha for (const [runs, sha] of [ @@ -115,6 +118,17 @@ export function compareRuns( ) { throw new Error(`Benchmark ${id} is missing or has a different version.`) } + if ( + [...base, ...head].some( + (metric) => + metric.iterations !== base[0]!.iterations || + metric.chunkIterations !== base[0]!.chunkIterations + ) + ) { + throw new Error( + `Benchmark ${id} executed unequal work between process runs.` + ) + } const baseSamples = base.flatMap((metric) => metric.samplesNsPerOp) const headSamples = head.flatMap((metric) => metric.samplesNsPerOp) const baseMedian = median(baseSamples) @@ -148,13 +162,22 @@ export function toBencherMetricFormat( return Object.fromEntries( [...indexMetrics(runs)] .sort(([left], [right]) => left.localeCompare(right)) - .map(([id, metrics]) => [ - id, - { - latency: { - value: median(metrics.flatMap((metric) => metric.samplesNsPerOp)), + .map(([id, metrics]) => { + if ( + metrics.length !== runs.length || + metrics.some((metric) => metric.version !== metrics[0]!.version) + ) + throw new Error( + `Benchmark ${id} is missing or changed between head processes.` + ) + return [ + id, + { + latency: { + value: median(metrics.flatMap((metric) => metric.samplesNsPerOp)), + }, }, - }, - ]) + ] + }) ) } diff --git a/scripts/performance/isolated-cases.ts b/scripts/performance/isolated-cases.ts index b45f6675c..4ae257603 100644 --- a/scripts/performance/isolated-cases.ts +++ b/scripts/performance/isolated-cases.ts @@ -11,7 +11,8 @@ export async function runIsolatedCases( const ids = new Set() for (let index = 0; index < count; index++) { const run = index === 0 ? first : validateBenchmarkRun(await runCase(index)) - validateExpectedRun(run, { ...first.configuration, benchmarkIndex: index }) + const { work: _firstWork, ...sharedConfiguration } = first.configuration + validateExpectedRun(run, { ...sharedConfiguration, benchmarkIndex: index }) if ( run.benchmarkCount !== count || run.metrics.length !== 1 || @@ -34,6 +35,7 @@ export async function runIsolatedCases( } const configuration = { ...first.configuration } delete configuration.benchmarkIndex + delete configuration.work return { ...first, configuration, diff --git a/scripts/performance/publish.test.ts b/scripts/performance/publish.test.ts index fa096a0c7..bb57fec14 100644 --- a/scripts/performance/publish.test.ts +++ b/scripts/performance/publish.test.ts @@ -8,6 +8,8 @@ const metadata: ReportMetadata = { pullRequestNumber: 123, baseSha: 'a'.repeat(40), headSha: 'b'.repeat(40), + baseSuiteHash: 'c'.repeat(64), + headSuiteHash: 'c'.repeat(64), platforms: ['android', 'ios'], } @@ -81,4 +83,15 @@ describe('Bencher publications', () => { bencherArguments(main, 'ios', 'base', '/validated', 'nitro') ).toThrow() }) + test('a changed suite records head without an invented paired baseline', () => { + const changed = { ...metadata, headSuiteHash: 'd'.repeat(64) } + const publications = bencherPublications(changed, '/validated', 'nitro') + expect(publications.map((entry) => entry.revision)).toEqual([ + 'head', + 'head', + ]) + expect(publications.flatMap((entry) => entry.command)).not.toContain( + '--start-point' + ) + }) }) diff --git a/scripts/performance/publish.ts b/scripts/performance/publish.ts index 69262c335..636e85357 100644 --- a/scripts/performance/publish.ts +++ b/scripts/performance/publish.ts @@ -17,7 +17,8 @@ export function bencherArguments( ): string[] { const baselineBranch = `baseline-${metadata.baseSha}` const isBase = revision === 'base' - if (isBase && metadata.pullRequestNumber == null) { + const comparable = metadata.baseSuiteHash === metadata.headSuiteHash + if (isBase && (metadata.pullRequestNumber == null || !comparable)) { throw new Error('Only PR reports need a paired baseline upload.') } const command = [ @@ -40,7 +41,7 @@ export function bencherArguments( '--file', path.join(directory, `bencher-${isBase ? 'base-' : ''}${platform}.json`), ] - if (!isBase && metadata.pullRequestNumber != null) { + if (!isBase && metadata.pullRequestNumber != null && comparable) { command.push( '--start-point', baselineBranch, @@ -57,7 +58,8 @@ export function bencherPublications( project: string ) { const revisions = - metadata.pullRequestNumber == null + metadata.pullRequestNumber == null || + metadata.baseSuiteHash !== metadata.headSuiteHash ? (['head'] as const) : (['base', 'head'] as const) // Seed every testbed before creating the PR branch. Never reset that branch diff --git a/scripts/performance/receive.ts b/scripts/performance/receive.ts index 7afda84b1..e6a530a38 100644 --- a/scripts/performance/receive.ts +++ b/scripts/performance/receive.ts @@ -14,6 +14,18 @@ if (platform !== 'android' && platform !== 'ios') { } const configuration: BenchmarkRunConfiguration = { + ...(argumentsMap.has('calibration') ? { calibration: true as const } : {}), + ...(argumentsMap.has('work-id') + ? { + work: { + id: requiredArgument(argumentsMap, 'work-id'), + iterations: Number(requiredArgument(argumentsMap, 'iterations')), + chunkIterations: Number( + requiredArgument(argumentsMap, 'chunk-iterations') + ), + }, + } + : {}), ...(argumentsMap.has('benchmark-index') ? { benchmarkIndex: Number( @@ -90,5 +102,6 @@ try { process.exitCode = 1 } finally { clearTimeout(timeout) - await server.stop(true) + // Finish the accepted HTTP response before closing the receiver. + await server.stop() } diff --git a/scripts/performance/report-validation.test.ts b/scripts/performance/report-validation.test.ts index 4b948699f..1d516da1c 100644 --- a/scripts/performance/report-validation.test.ts +++ b/scripts/performance/report-validation.test.ts @@ -95,6 +95,8 @@ async function createFixture(root: string): Promise<{ pullRequestNumber: 123, baseSha: BASE_SHA, headSha: HEAD_SHA, + baseSuiteHash: SUITE_HASH, + headSuiteHash: SUITE_HASH, workflowRunId: 123456789, runAttempt: 1, }) @@ -248,4 +250,40 @@ describe('trusted performance report validation', () => { await rm(root, { recursive: true, force: true }) } }) + test('changed suites accept a head-only baseline; comparable suites require base', async () => { + const root = await mkdtemp(path.join(os.tmpdir(), 'nitro-performance-')) + try { + const fixture = await createFixture(root) + for (const platform of ['android', 'ios']) + for (const sequence of [1, 2]) { + await rm( + path.join( + fixture.artifact, + 'raw', + platform, + `base-${sequence}.json` + ) + ) + } + expect((await validate(fixture)).exitCode).not.toBe(0) + const file = path.join(fixture.artifact, 'performance-report.json') + const manifest = JSON.parse(await readFile(file, 'utf8')) + manifest.baseSuiteHash = 'd'.repeat(64) + await writeJson(file, manifest) + expect((await validate(fixture)).exitCode).toBe(0) + expect( + await readFile( + path.join(fixture.output, 'performance-summary.md'), + 'utf8' + ) + ).toContain('require a new baseline') + expect( + await Bun.file( + path.join(fixture.output, 'bencher-base-ios.json') + ).exists() + ).toBe(false) + } finally { + await rm(root, { recursive: true, force: true }) + } + }) }) diff --git a/scripts/performance/report.ts b/scripts/performance/report.ts index 6d0cc840c..44153275e 100644 --- a/scripts/performance/report.ts +++ b/scripts/performance/report.ts @@ -1,3 +1,4 @@ +import { readFile } from 'node:fs/promises' import { parseArguments, requiredArgument } from './args' /** The artifact contains raw runs plus provenance. Only trusted code derives a report. */ @@ -8,6 +9,8 @@ export interface PerformanceReport { pullRequestNumber: number | null baseSha: string headSha: string + baseSuiteHash: string + headSuiteHash: string workflowRunId: number runAttempt: number } @@ -30,7 +33,12 @@ if (import.meta.main) { ) { throw new Error(`Unsupported event: ${eventName}`) } + const { baseSuiteHash, headSuiteHash } = JSON.parse( + await readFile(requiredArgument(args, 'suite'), 'utf8') + ) as Pick const report: PerformanceReport = { + baseSuiteHash, + headSuiteHash, schemaVersion: 2, eventName, repository: requiredArgument(args, 'repository'), diff --git a/scripts/performance/run-device.ts b/scripts/performance/run-device.ts index eaae3f7e5..2cb454dfd 100644 --- a/scripts/performance/run-device.ts +++ b/scripts/performance/run-device.ts @@ -1,4 +1,4 @@ -import { median } from '../../apps/benchmark/src/benchmarks/statistics' +import type { BenchmarkWork } from '../../apps/benchmark/src/benchmarks/types' import path from 'node:path' import { mkdir, readFile } from 'node:fs/promises' import { parseArguments, requiredArgument } from './args' @@ -76,12 +76,30 @@ const receiverArguments = [ requiredArgument(argumentsMap, 'toolchain'), ] -async function runCase(index: number) { - const caseOutput = path.join(casesDirectory, `case-${index}.json`) +async function runCase( + index: number, + calibration: boolean, + work?: BenchmarkWork +) { + const caseOutput = path.join( + casesDirectory, + `${calibration ? 'calibration' : 'case'}-${index}.json` + ) const receiver = Bun.spawn( [ 'bun', ...receiverArguments, + ...(calibration ? ['--calibration', 'true'] : []), + ...(work == null + ? [] + : [ + '--work-id', + work.id, + '--iterations', + String(work.iterations), + '--chunk-iterations', + String(work.chunkIterations), + ]), '--output', caseOutput, '--benchmark-index', @@ -164,7 +182,7 @@ async function runCase(index: number) { ) const metric = result.metrics[0]! console.info( - `[NitroBenchmark] case ${index + 1}/${result.benchmarkCount}: ${metric.id}, ${metric.iterations} ops/sample, median timed batch ${((median(metric.samplesNsPerOp) * metric.iterations) / 1e6).toFixed(1)} ms` + `[NitroBenchmark] ${calibration ? 'calibration' : 'measurement'} case ${index + 1}/${result.benchmarkCount}: ${metric.id}, ${metric.iterations} ops/sample` ) return result } catch (error) { @@ -244,5 +262,39 @@ if (platform === 'android') { ) await command('xcrun', ['simctl', 'install', deviceId, app]) } -const result = await runIsolatedCases(runCase) +const workFile = argumentsMap.get('work-plan')?.[0] +const plan = + workFile == null + ? undefined + : validateBenchmarkRun(JSON.parse(await readFile(workFile, 'utf8'))) +const work = plan?.metrics.map(({ id, iterations, chunkIterations }) => ({ + id, + iterations, + chunkIterations, +})) +if ( + plan != null && + (plan.configuration.calibration !== true || + plan.configuration.suiteHash !== + requiredArgument(argumentsMap, 'suite-hash')) +) { + throw new Error('Work plan must be calibration for the measured suite.') +} +if ( + plan != null && + plan.configuration.reverse !== (argumentsMap.get('reverse')?.[0] === 'true') +) + work?.reverse() +const result = await runIsolatedCases(async (index) => { + if (argumentsMap.has('calibration')) return runCase(index, true) + let counts = work?.[index] + if (counts == null) { + if (work != null) throw new Error('Work plan is missing a benchmark case.') + const calibration = await runCase(index, true) + const { id, iterations, chunkIterations } = calibration.metrics[0]! + counts = { id, iterations, chunkIterations } + } + // runCase terminates its process before returning, including calibration. + return runCase(index, false, counts) +}) await Bun.write(output, `${JSON.stringify(result, null, 2)}\n`) diff --git a/scripts/performance/run-sequence.test.ts b/scripts/performance/run-sequence.test.ts new file mode 100644 index 000000000..7d617675c --- /dev/null +++ b/scripts/performance/run-sequence.test.ts @@ -0,0 +1,166 @@ +import { expect, test } from 'bun:test' +import { chmod, mkdir, mkdtemp, readFile, rm } from 'node:fs/promises' +import os from 'node:os' +import path from 'node:path' + +// Exercise the real controller/receiver with a tiny process standing in for +// simctl's app. Runner tests separately exercise timed work and slowdown bounds. +test.each([false, true])( + 'fresh processes share calibrated work; changed suite = %s', + async (changedSuite) => { + const directory = await mkdtemp(path.join(os.tmpdir(), 'nitro-sequence-')) + try { + const simulator = path.join(directory, 'simulator.ts') + await Bun.write( + simulator, + ` + import { appendFile } from 'node:fs/promises' + const configuration = await (await fetch('http://127.0.0.1:8173/config')).json() + if (process.env.CHANGED_SUITE === 'true' && configuration.commitSha.startsWith('a')) throw new Error('Old base must not receive the new protocol.') + const index = configuration.reverse ? 1 - configuration.benchmarkIndex : configuration.benchmarkIndex + const id = ['javascript/control/first', 'javascript/control/second'][index] + const work = configuration.work ?? { id, iterations: [1000, 500][index], chunkIterations: [250, 100][index] } + if (work.id !== id) throw new Error('Work was assigned to the wrong reversed case.') + await appendFile(process.env.SIMULATOR_LOG, JSON.stringify({ pid: process.pid, configuration, work }) + '\\n') + const count = configuration.calibration ? 0 : 20 + const response = await fetch('http://127.0.0.1:8173/result', { + method: 'POST', body: JSON.stringify({ + schemaVersion: 1, suiteVersion: 1, benchmarkCount: 2, configuration, + environment: { reactNativeVersion: '0.85.3', hermes: true, dev: false, nitroBuildType: 'release' }, + runner: { targetBatchDurationMs: 150, warmupCount: count === 0 ? 0 : 5, sampleCount: count }, + startedAt: new Date().toISOString(), durationMs: 100, + metrics: [{ ...work, version: 1, family: 'control', implementation: 'javascript', samplesNsPerOp: Array(count).fill(100), checksum: 0 }], + }), + }) + if (!response.ok) throw new Error(await response.text()) + ` + ) + const xcrun = path.join(directory, 'xcrun') + await Bun.write( + xcrun, + `#!/bin/sh\nif [ "$2" = launch ]; then exec '${process.execPath}' '${simulator}'; fi\n` + ) + await chmod(xcrun, 0o755) + const output = path.join(directory, 'results') + const log = path.join(directory, 'processes.jsonl') + const root = path.resolve(import.meta.dir, '../..') + const baseRoot = path.join(directory, 'base') + if (changedSuite) { + await mkdir(path.join(baseRoot, 'apps/benchmark/src/benchmarks'), { + recursive: true, + }) + await Bun.write( + path.join(baseRoot, 'apps/benchmark/index.js'), + '// old benchmark' + ) + } + const child = Bun.spawn( + [ + 'bun', + path.join(import.meta.dir, 'run-sequence.ts'), + '--platform', + 'ios', + '--base-app', + directory, + '--head-app', + directory, + '--base-root', + changedSuite ? baseRoot : root, + '--head-root', + root, + '--base-sha', + 'a'.repeat(40), + '--head-sha', + 'b'.repeat(40), + '--output-directory', + output, + '--device-id', + 'fixture', + '--device', + 'fixture', + '--os-version', + 'fixture', + '--architecture', + 'arm64', + '--toolchain', + 'fixture', + ], + { + env: { + ...process.env, + PATH: `${directory}:${process.env.PATH}`, + SIMULATOR_LOG: log, + CHANGED_SUITE: String(changedSuite), + }, + stdout: 'pipe', + stderr: 'pipe', + } + ) + const [exitCode, stdout, stderr] = await Promise.all([ + child.exited, + new Response(child.stdout).text(), + new Response(child.stderr).text(), + ]) + expect({ + exitCode, + error: exitCode === 0 ? '' : stdout + stderr, + }).toEqual({ + exitCode: 0, + error: '', + }) + const processes = (await readFile(log, 'utf8')) + .trim() + .split('\n') + .map((line) => JSON.parse(line)) + expect(new Set(processes.map((entry) => entry.pid)).size).toBe( + changedSuite ? 6 : 10 + ) + expect( + processes + .slice(0, 2) + .every((entry) => entry.configuration.calibration === true) + ).toBe(true) + expect( + processes.slice(2).map((entry) => entry.configuration.runId) + ).toEqual( + changedSuite + ? ['ios-head-1', 'ios-head-1', 'ios-head-2', 'ios-head-2'] + : [ + 'ios-base-1', + 'ios-base-1', + 'ios-head-1', + 'ios-head-1', + 'ios-head-2', + 'ios-head-2', + 'ios-base-2', + 'ios-base-2', + ] + ) + for (const file of changedSuite + ? ['head-1', 'head-2'] + : ['base-1', 'head-1', 'head-2', 'base-2']) { + const run = JSON.parse( + await readFile(path.join(output, `${file}.json`), 'utf8') + ) + expect(run.configuration.calibration).toBeUndefined() + const metrics = run.metrics.sort( + (a: { id: string }, b: { id: string }) => a.id.localeCompare(b.id) + ) + expect( + metrics.map( + (metric: { iterations: number; chunkIterations: number }) => [ + metric.iterations, + metric.chunkIterations, + ] + ) + ).toEqual([ + [1000, 250], + [500, 100], + ]) + } + } finally { + await rm(directory, { recursive: true, force: true }) + } + }, + 20_000 +) diff --git a/scripts/performance/run-sequence.ts b/scripts/performance/run-sequence.ts index c4ca5c7c3..dabdc8bde 100644 --- a/scripts/performance/run-sequence.ts +++ b/scripts/performance/run-sequence.ts @@ -30,13 +30,24 @@ const [baseSuiteHash, headSuiteHash] = await Promise.all([ calculateSuiteHash(headRoot), ]) +await Bun.write( + path.join(outputDirectory, 'suite.json'), + `${JSON.stringify({ baseSuiteHash, headSuiteHash }, null, 2)}\n` +) + async function runOne( revision: 'base' | 'head', sequence: number, - reverse: boolean + reverse: boolean, + calibration = false ): Promise { const isBase = revision === 'base' - const output = path.join(outputDirectory, `${revision}-${sequence}.json`) + const output = path.join( + outputDirectory, + calibration + ? `calibration-${revision}.json` + : `${revision}-${sequence}.json` + ) const runId = `${platform}-${revision}-${sequence}` const startedAt = performance.now() console.info( @@ -45,6 +56,15 @@ async function runOne( const command = [ 'bun', path.join(import.meta.dir, 'run-device.ts'), + ...(calibration + ? ['--calibration', 'true'] + : [ + '--work-plan', + path.join( + outputDirectory, + `calibration-${isBase || baseSuiteHash === headSuiteHash ? 'base' : 'head'}.json` + ), + ]), '--platform', platform, '--app', @@ -81,7 +101,19 @@ async function runOne( ) } -await runOne('base', 1, false) -await runOne('head', 1, false) -await runOne('head', 2, true) -await runOne('base', 2, true) +// Derive one plan from base, then discard every calibration process. Changed +// suites need separate plans and will be reported without a comparison. +if (baseSuiteHash === headSuiteHash) { + await runOne('base', 0, false, true) + await runOne('base', 1, false) + await runOne('head', 1, false) + await runOne('head', 2, true) + await runOne('base', 2, true) +} else { + console.info( + '[NitroBenchmark] Benchmark definitions changed; measuring a new head baseline only.' + ) + await runOne('head', 0, false, true) + await runOne('head', 1, false) + await runOne('head', 2, true) +} diff --git a/scripts/performance/runner.test.ts b/scripts/performance/runner.test.ts index 3015eb77c..c00f242d9 100644 --- a/scripts/performance/runner.test.ts +++ b/scripts/performance/runner.test.ts @@ -1,5 +1,8 @@ import { afterEach, describe, expect, spyOn, test } from 'bun:test' -import { runBenchmarkDefinitions } from '../../apps/benchmark/src/benchmarks/runner' +import { + calibrateBenchmarkDefinitions, + runBenchmarkDefinitions, +} from '../../apps/benchmark/src/benchmarks/runner' import { benchmarkRuntime, executeBatch, @@ -11,6 +14,9 @@ import { import type { BenchmarkDefinition } from '../../apps/benchmark/src/benchmarks/types' const runtime = { collectGarbage() {}, async yieldToRuntime() {} } +const work = [ + { id: 'javascript/control/fake-clock', iterations: 1, chunkIterations: 1 }, +] afterEach(() => { spyOn(performance, 'now').mockRestore() @@ -53,6 +59,7 @@ describe('benchmark runner', () => { sampleCount: 2, reverse: false, }, + work, runtime ) @@ -74,6 +81,7 @@ describe('benchmark runner', () => { sampleCount: 1, reverse: false, }, + work, runtime ) ).rejects.toThrow('returned checksum 2, expected 99') @@ -216,36 +224,65 @@ describe('benchmark runner', () => { expect(result).toEqual({ durationMs: 150, checksum: 20_000 }) }) - test('recalibrates after warmup and freezes iterations for all 20 measured samples', async () => { + test('calibrates once and executes identical work on slower fresh runtimes', async () => { let now = 0 - let calls = 0 - const counts: number[] = [] spyOn(performance, 'now').mockImplementation(() => now) - const [metric] = await runBenchmarkDefinitions( - [ - { - ...definition((n) => n * 2), - initialIterations: 1_000, - maxIterations: 1_000_000, - run(n) { - counts.push(n) - now += n * (++calls > 2 ? 0.075 : 0.15) - return n * 2 - }, - }, - ], - { - targetBatchDurationMs: 150, - warmupCount: 5, - sampleCount: 20, - reverse: false, + const makeDefinition = (speed: number, counts: number[]) => ({ + ...definition((n) => n * 2), + initialIterations: 100, + maxIterations: 100_000, + maxChunkIterations: 250, + run(n: number) { + counts.push(n) + now += n * speed + return n * 2 }, + }) + const calibrationCalls: number[] = [] + const plan = await calibrateBenchmarkDefinitions( + [makeDefinition(0.15, calibrationCalls)], + 150, runtime ) - expect(metric?.iterations).toBe(2_000) - expect(metric?.chunkIterations).toBe(2_000) - expect(counts.slice(-20)).toEqual(Array(20).fill(2_000)) - expect(metric?.samplesNsPerOp).toEqual(Array(20).fill(75_000)) + expect(plan[0]?.iterations).toBe(1_000) + for (const speed of [0.15, 0.3, 1.5]) { + const measuredCalls: number[] = [] + const result = await runBenchmarkDefinitions( + [makeDefinition(speed, measuredCalls)], + { + targetBatchDurationMs: 150, + warmupCount: 5, + sampleCount: 20, + reverse: false, + }, + plan, + runtime + ) + expect(measuredCalls).toEqual(Array(25 * 4).fill(250)) + expect(result[0]?.samplesNsPerOp).toEqual(Array(20).fill(speed * 1e6)) + } + }) + + test('rejects incompatible work counts instead of silently reducing head work', async () => { + for (const plan of [ + [], + [{ ...work[0]!, iterations: 2 }], + [{ ...work[0]!, chunkIterations: 2 }], + ]) { + await expect( + runBenchmarkDefinitions( + [definition((n) => n * 2)], + { + targetBatchDurationMs: 1, + warmupCount: 1, + sampleCount: 1, + reverse: false, + }, + plan, + runtime + ) + ).rejects.toThrow('incompatible work counts') + } }) test('preserves slow measured samples instead of filtering scheduler stalls', async () => { @@ -257,7 +294,7 @@ describe('benchmark runner', () => { { ...definition((n) => n * 2), run(n) { - now += ++calls === 4 ? 10 : 1 + now += ++calls === 2 ? 10 : 1 return n * 2 }, }, @@ -268,6 +305,7 @@ describe('benchmark runner', () => { sampleCount: 2, reverse: false, }, + work, runtime ) expect(metric?.samplesNsPerOp).toEqual([10_000_000, 1_000_000]) diff --git a/scripts/performance/schema.ts b/scripts/performance/schema.ts index ce8f0c77e..41f7a4f16 100644 --- a/scripts/performance/schema.ts +++ b/scripts/performance/schema.ts @@ -67,7 +67,28 @@ function validateConfiguration(value: unknown): BenchmarkRunConfiguration { if (!SUITE_HASH_PATTERN.test(suiteHash)) { throw new Error('configuration.suiteHash must be a SHA-256 digest.') } + const work = value.work + if (work !== undefined && !isObject(work)) + throw new Error('configuration.work must be an object.') + if (value.calibration !== undefined && value.calibration !== true) + throw new Error('Invalid calibration flag.') return { + ...(value.calibration === true ? { calibration: true as const } : {}), + ...(work === undefined + ? {} + : { + work: { + id: stringValue(work.id, 'configuration.work.id'), + iterations: positiveInteger( + work.iterations, + 'configuration.work.iterations' + ), + chunkIterations: positiveInteger( + work.chunkIterations, + 'configuration.work.chunkIterations' + ), + }, + }), ...(value.benchmarkIndex === undefined ? {} : { @@ -121,11 +142,7 @@ function validateEnvironment(value: unknown): BenchmarkRunEnvironment { function validateMetric(value: unknown, index: number): BenchmarkMetric { if (!isObject(value)) throw new Error(`metrics[${index}] must be an object.`) const samples = value.samplesNsPerOp - if ( - !Array.isArray(samples) || - samples.length === 0 || - samples.length > MAX_SAMPLES - ) { + if (!Array.isArray(samples) || samples.length > MAX_SAMPLES) { throw new Error(`metrics[${index}].samplesNsPerOp has invalid length.`) } const samplesNsPerOp = samples.map((sample, sampleIndex) => @@ -206,6 +223,13 @@ export function validateBenchmarkRun(value: unknown): BenchmarkRunResult { ) { throw new Error('metrics must be a non-empty bounded array.') } + const configuration = validateConfiguration(value.configuration) + if ( + configuration.calibration && + (value.runner.warmupCount !== 0 || value.runner.sampleCount !== 0) + ) { + throw new Error('Calibration must not contain warmup or measured samples.') + } const validatedMetrics = metrics.map(validateMetric) const benchmarkCount = positiveInteger(value.benchmarkCount, 'benchmarkCount') if (benchmarkCount > MAX_METRICS || benchmarkCount < metrics.length) { @@ -232,7 +256,7 @@ export function validateBenchmarkRun(value: unknown): BenchmarkRunResult { schemaVersion: 1, suiteVersion: 1, benchmarkCount, - configuration: validateConfiguration(value.configuration), + configuration, environment: validateEnvironment(value.environment), runner: { targetBatchDurationMs: finiteNumber( @@ -241,14 +265,12 @@ export function validateBenchmarkRun(value: unknown): BenchmarkRunResult { 1, 10_000 ), - warmupCount: positiveInteger( - value.runner.warmupCount, - 'runner.warmupCount' - ), - sampleCount: positiveInteger( - value.runner.sampleCount, - 'runner.sampleCount' - ), + warmupCount: configuration.calibration + ? finiteNumber(value.runner.warmupCount, 'runner.warmupCount', 0, 100) + : positiveInteger(value.runner.warmupCount, 'runner.warmupCount'), + sampleCount: configuration.calibration + ? finiteNumber(value.runner.sampleCount, 'runner.sampleCount', 0, 100) + : positiveInteger(value.runner.sampleCount, 'runner.sampleCount'), }, startedAt, durationMs: finiteNumber(value.durationMs, 'durationMs'), @@ -264,7 +286,7 @@ export function validateExpectedRun( for (const key of Object.keys( expected ) as (keyof BenchmarkRunConfiguration)[]) { - if (actual[key] !== expected[key]) { + if (JSON.stringify(actual[key]) !== JSON.stringify(expected[key])) { throw new Error(`Result configuration mismatch for ${key}.`) } } diff --git a/scripts/performance/validate-report.ts b/scripts/performance/validate-report.ts index 120849c9c..0f6e96e11 100644 --- a/scripts/performance/validate-report.ts +++ b/scripts/performance/validate-report.ts @@ -181,7 +181,22 @@ function validateReport(value: unknown): PerformanceReport { if (!isSafeSha(baseSha) || !isSafeSha(headSha)) { throw new Error('Report SHAs are invalid.') } + const baseSuiteHash = boundedString( + report.baseSuiteHash, + 'report.baseSuiteHash' + ) + const headSuiteHash = boundedString( + report.headSuiteHash, + 'report.headSuiteHash' + ) + if ( + !/^[0-9a-f]{64}$/.test(baseSuiteHash) || + !/^[0-9a-f]{64}$/.test(headSuiteHash) + ) + throw new Error('Invalid report suite hashes.') return { + baseSuiteHash, + headSuiteHash, schemaVersion: 2, eventName, repository: boundedString(report.repository, 'report.repository'), @@ -285,7 +300,12 @@ async function loadRawRuns( run.configuration.runId !== `${platform}-${revision}-${sequence}` || run.configuration.reverse !== (sequence === 2) || run.configuration.platform !== platform || + run.configuration.benchmarkIndex !== undefined || + run.metrics.length !== run.benchmarkCount || run.configuration.commitSha !== expectedSha || + run.configuration.suiteHash !== + (revision === 'base' ? report.baseSuiteHash : report.headSuiteHash) || + run.configuration.calibration !== undefined || run.runner.targetBatchDurationMs !== 150 || run.runner.warmupCount !== 5 || run.runner.sampleCount !== 20 || @@ -304,14 +324,27 @@ async function loadRawRuns( const rebuiltComparisons = await Promise.all( (['android', 'ios'] as const).map(async (platform) => { - const [baseRuns, headRuns] = await Promise.all([ - loadRawRuns(platform, 'base', report.baseSha), - loadRawRuns(platform, 'head', report.headSha), - ]) - if (baseRuns.length !== headRuns.length) { - throw new Error('Base and head run counts must match.') - } - const comparison = compareRuns(baseRuns, headRuns) + const headRuns = await loadRawRuns(platform, 'head', report.headSha) + const comparable = report.baseSuiteHash === report.headSuiteHash + const baseFiles = ( + await readdir(path.join(artifactDirectory, 'raw', platform)) + ).filter((file) => /^base-/.test(file)) + if (!comparable && baseFiles.length !== 0) + throw new Error( + 'Changed suites must not upload incomparable base measurements.' + ) + const baseRuns = comparable + ? await loadRawRuns(platform, 'base', report.baseSha) + : [] + const comparison = comparable + ? compareRuns(baseRuns, headRuns) + : { + platform, + baseSha: report.baseSha, + headSha: report.headSha, + suiteComparable: false, + comparisons: [], + } return { comparison, baseRuns, headRuns } }) ) @@ -338,6 +371,8 @@ await Bun.write( pullRequestNumber: report.pullRequestNumber, baseSha: report.baseSha, headSha: report.headSha, + baseSuiteHash: report.baseSuiteHash, + headSuiteHash: report.headSuiteHash, workflowRunUrl, platforms: rebuiltComparisons.map( ({ comparison }) => comparison.platform @@ -353,6 +388,7 @@ for (const { comparison, baseRuns, headRuns } of rebuiltComparisons) { [`base-${comparison.platform}`, baseRuns], [comparison.platform, headRuns], ] as const) { + if (runs.length === 0) continue await Bun.write( path.join(outputDirectory, `bencher-${suffix}.json`), `${JSON.stringify(toBencherMetricFormat(runs), null, 2)}\n`