bench: regression guard failed PR #2586 on runner noise — native No-op rebuild median(5) landed at 103ms vs 23–32ms on adjacent runs #2590

Activity

1. carlos-alm commented on Aug 19, 2026

   

   carlos-alm

   on Aug 19, 2026

   ContributorAuthor

   More actions

   Update: root cause narrowed, and it is a concrete defect — diffImpact is the only query metric measured with no warmup

   I re-ran the gate twice more on the same unchanged PR head. Each attempt failed on a different metric, and each previously-failing metric returned to baseline — the signature of measurement variance, not a code regression:

   attempt	native noop	native diffImpact	wasm 1-file (setupMs)	result
   1	103 (baseline 32)	26.5	173 (7.8)	FAIL ×3
   2	24 ✓	12.3 ✓	349 (221.8)	FAIL ×1
   3	23 ✓	21.8	158 (16.1) ✓	FAIL ×1

   In attempt 2, the entire 176ms excess on wasm 1-file rebuild sat in a single phase — setupMs 221.8 vs a typical 8ms — with every other phase normal.

   The defect

   In scripts/query-benchmark.ts, WARMUP_RUNS = 3 is applied only inside benchDepths (which serves fnDeps and fnImpact). benchDiffImpact calls timeMedian(...) directly with no warmup loop:

   const latencyMs = round1(
     await timeMedian(() => {
       lastResult = diffImpactData(dbPath, { staged: true, depth: 3, noTests: true });
     }, RUNS),
   );

   So diffImpact latency is the one query metric measured cold. diffImpactData is a distinct query path from fnDeps/fnImpact — its own prepared statements, its own DB pages, plus a git subprocess for the staged diff — so the NAPI/static init already paid by the earlier benchDepths calls does not warm it.

   This is exactly the defect class #2584 just fixed for benchmark.ts's Full build metric ("give benchmark.ts's Full build metric a warmup-then-median too"), and which #2436 traced part of the gate's non-determinism to: the one metric that measured without this warmup. diffImpact is now the remaining instance.

   Supporting evidence that this metric is the weak point

   Native diffImpact latency across the last four PR runs — 12.4, 16.5, 16.9, then 12.3/21.8/26.5 on this PR — against a recorded 3.17.0 baseline of 10.1. Recent passing runs at 16.5–16.9 were already +63–67% over the 25% threshold and survived only because the ~10ms MIN_ABSOLUTE_DELTA floor filtered a 6.8ms delta. Anything landing at ≥20.1ms clears the floor and fails. The check is riding on the floor, not on real headroom.

   Suggested fix

   1. Give benchDiffImpact the same warmup-then-median treatment benchDepths already has (mirrors fix(bench): give benchmark.ts's Full build metric a warmup-then-median too #2584).
   2. Re-measure and consider refreshing the stale 10.1 baseline afterward.
   3. Optionally add diffImpact latency to NOISY_METRICS — it is a sub-30ms metric with a subprocess in the measured path, which is precisely that set's stated criterion.

   Note the benchmark also records affectedFunctions: 0, affectedFiles: 0 on every historical entry — the probe appends only a comment (\n// benchmark-probe\n), so the query resolves nothing. The metric is therefore measuring near-pure fixed overhead, which is why it is so variance-dominated.
2. carlos-alm commented on Aug 19, 2026

   

   carlos-alm

   on Aug 19, 2026

   ContributorAuthor

   More actions

   Item 1 of the suggested fix (warmup for benchDiffImpact) is now open as #2591. Items 2 and 3 — refreshing the stale 10.1 baseline, and the fact that this benchmark measures near-pure fixed overhead (affectedFunctions: 0 on every historical entry) — remain open here.
3. carlos-alm

   added a commit that references this issue on Aug 20, 2026

   fix(bench): give query-benchmark.ts's diffImpact metric a warmup too (#…

   ...

   4cbc5c5