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

[carlos-alm]

What happened

The pre-publish benchmark gate failed PR #2586 (job log) with three native-engine regressions:

build benchmarks       > native — No-op rebuild:     30 → 103  (+243%, threshold 50%)
incremental benchmarks > native — No-op rebuild:     32 → 103  (+222%, threshold 50%)
query benchmarks       > native — diffImpact latency: 10.1 → 26.5 (+162%, threshold 25%)

PR #2586 adds one match/map arm for enum_declaration to the JS export-kind tables in both engines. A no-op rebuild parses zero files, so that code is never reached — the diff cannot produce this.

Evidence it was runner noise, not a regression

Comparing the uploaded benchmark-results-json artifacts across the four most recent PR runs (all at 847 files, all within a few commits of each other):

run	native noop	native 1-file	native full build	wasm noop	native diffImpact
#2582 (32212933277)	23ms	217	5223	23	12.4
#2584 (32215138390)	32ms	267	6709	30	16.5
#2585 (32220394153)	32ms	264	6592	29	16.9
#2586 (32225365117)	103ms	273	6332	32	26.5

Everything else in the failing run matched baseline closely — native full build 6332 vs 6345 at 3.17.0, wasm full build 19154 vs 19082, wasm noop 32 vs 31 — so the runner was not uniformly slow, and the per-phase breakdown for the 1-file rebuild shows no new or inflated phase. Historical native noopRebuildMs has never exceeded 32ms across all 36 recorded releases (15 → 32, tracking file-count growth).

The same CI run also had build / Build x86_64-unknown-linux-musl hang for 6 hours inside apt-get update (#2589), which suggests the Azure host pool was degraded during that window.

Re-running the failed jobs was the resolution; no code change was needed.

The interesting part

No-op rebuild already gets the strongest noise treatment in the suite — NOISY_METRICS (50% threshold), 2 warmup runs, and timeMedian(..., RUNS=5). For a median of 5 to land at 103ms, at least 3 of the 5 samples had to be ≥103ms. This was a sustained ~3x slowdown across most of the sampling window, not a single GC blip, so more samples would not obviously have rescued it.

Note also that native diffImpact latency has been running 16.5–16.9ms against a 10.1ms baseline (+63–67%, well past its 25% threshold) on recent passing runs — those only pass because the ~10ms MIN_ABSOLUTE_DELTA floor filters a 6.8ms delta. At 26.5ms the delta cleared the floor and the metric flagged. The baseline may simply be stale relative to where this metric now sits, making it a thin-margin tripwire.

Suggested work

• Decide whether No-op rebuild's remaining exposure is worth further hardening (e.g. trimmed mean, discard-max, or a re-measure-once-on-failure retry) or whether an occasional infra-driven red PR is acceptable given a re-run fixes it.
• Separately, look at whether the native diffImpact latency baseline should be refreshed — it currently sits ~65% under where the metric actually runs, so the check is riding entirely on the absolute-delta floor.

[carlos-alm]

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.