Skip to content

runner: base the benchmark verdict on min, not mean - #21

Merged
adriangb merged 1 commit into
mainfrom
runner/min-based-comparison
Jul 31, 2026
Merged

runner: base the benchmark verdict on min, not mean#21
adriangb merged 1 commit into
mainfrom
runner/min-based-comparison

Conversation

@adriangb

Copy link
Copy Markdown
Owner

What

The per-query faster/slower/no-change verdict now comes from bench.sh compare (which reduces each query's iterations to the min) instead of compare_detail (which reduces to the mean). The detailed min / mean ±stddev / max table still follows it, so nothing is lost from the comment.

Why

Both sides run sequentially in the same pod on a spot node, so a slow iteration is usually the node interfering rather than the query being slower. The mean folds that interference into the verdict; the min discards it.

This came out of investigating run-to-run variance. Three A/A runs — baseline.ref and changed.ref both main, so the two sides are the same commit (1ae6b87 vs 1ae6b87) and every reported difference is noise by construction:

statistic median per-query error worst query aggregate
mean (compare_detail) 1.29% 4.31% −0.26%
min (compare) 0.55% 3.52% +0.00%

Source run: adriangb/datafusion#15 (comment). On min, the 22-query aggregate came out at 727.66 ms vs 727.66 ms — exact.

Concretely, the failure mode this removes (run through upstream compare.py, same input both ways — identical 16.00 ms floor, two slow iterations on one side):

### min  → no change
│ Q1 │ 16.00 ms │ 16.00 ms │    no change │

### mean → false regression
│ Q1 │ 16.00 / 16.08 ±0.07 / 16.20 ms │ 16.00 / 17.94 ±2.30 / 21.00 ms │ 1.12x slower │

Why keep compare_detail too

The spread is what makes an unstable query legible as unstable rather than as a regression — it's what made the table above diagnosable in the first place. It costs one extra invocation of a Python script over JSON that's already on disk.

What this is not

The same investigation tested whether mimalloc tuning (MIMALLOC_PURGE_DELAY=-1, MIMALLOC_ALLOW_THP=0, and a pre-reserved arena) could tighten the noise floor. It reduced within-process iteration jitter about 20% (median relative σ 2.41% → 1.87%) but did not improve between-process reproducibility, which is what the comparison actually depends on. Those knobs are deliberately not adopted here.

Testing

  • cargo test — 154 passed.
  • Both compare and compare_detail verified directly against upstream compare.py on constructed results JSON (output above).

The shell-invocation path itself has no unit coverage — it isn't reachable without a benchmark harness — so the first real run on a PR is the actual check on comment formatting.

🤖 Generated with Claude Code

`bench.sh compare_detail` reduces each query's iterations to the mean
before deciding faster/slower/no-change. Both sides run sequentially in
one pod on a spot node, so a slow iteration is usually the node
interfering rather than the query — the mean folds that in, the min
discards it.

Measured on an A/A run (identical SHAs on both sides, so every reported
difference is noise):

               per-query err    worst    aggregate
  mean              1.29%       4.31%      -0.26%
  min               0.55%       3.52%      +0.00%

adriangb/datafusion#15 (issuecomment-5139405485).

The `compare` subcommand already reduces on min, so the verdict table is
now that one. `compare_detail` still follows it: the min/mean±stddev/max
spread is what makes an unstable query legible as unstable rather than
as a regression, and dropping it would cost more than it saves.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@adriangb
adriangb merged commit 309486b into main Jul 31, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant