From 35ba47f41d74cb1b13263c2a6f34f2cf8fbc6482 Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Fri, 31 Jul 2026 00:17:55 -0500 Subject: [PATCH] runner: base the benchmark verdict on min, not mean MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 --- controller/src/runner/bench_datafusion.rs | 34 +++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/controller/src/runner/bench_datafusion.rs b/controller/src/runner/bench_datafusion.rs index 6166677..34095a5 100644 --- a/controller/src/runner/bench_datafusion.rs +++ b/controller/src/runner/bench_datafusion.rs @@ -291,19 +291,49 @@ pub async fn run(config: &RunnerConfig, poster: &CommentPoster) -> Result<()> { } // Compare and post results, driven by the artifacts that were produced: - // - results/*.json (dfbench suites) → `bench.sh compare_detail` + // - results/*.json (dfbench suites) → `bench.sh compare` + `compare_detail` // - target/criterion baselines (Criterion targets + the bench.sh SQL // harness) → `critcmp` let mut report = String::new(); if any_shell && results_have_json(&bench_benchmarks, &base_results_name).await { + // Two tables over the same results JSON, differing only in the statistic + // `compare.py` reduces each query's iterations to: + // + // `compare` → min (fastest iteration) + // `compare_detail` → mean (and renders min/mean±stddev/max) + // + // The verdict table is the min-based one. An A/A run — identical SHAs on + // both sides, so every reported difference is noise — put the mean-based + // per-query error at 1.29% median / 4.31% worst and the aggregate at + // -0.26%, against 0.55% / 3.52% and +0.00% for min: + // https://github.com/adriangb/datafusion/pull/15#issuecomment-5139405485 + // + // Both sides run sequentially in one pod, so a slow iteration is usually + // the node interfering rather than the query; the mean folds that in and + // the min discards it. `compare_detail` still follows, because the spread + // it shows is what makes an unstable query legible as unstable instead of + // as a regression. + let summary = shell::run_command( + "./bench.sh", + &["compare", &base_results_name, &bench_branch_name], + &bench_benchmarks, + ) + .await + .context("bench.sh compare")?; + report.push_str(&summary); + let detail = shell::run_command( "./bench.sh", &["compare_detail", &base_results_name, &bench_branch_name], &bench_benchmarks, ) .await - .context("bench.sh compare")?; + .context("bench.sh compare_detail")?; + if !report.is_empty() { + report.push('\n'); + } + report.push_str("Distribution per query (min / mean ±stddev / max):\n\n"); report.push_str(&detail); }