Measure bench baselines on the same runner - #144
Merged
Conversation
Changepacksvespera@0.3.1 → 0.4.0 - Cargo.tomlMinor
vespera@0.3.1 → 0.4.0 - crates/vespera/Cargo.tomlMinor
vespera_core@0.3.1 → 0.4.0 - crates/vespera_core/Cargo.tomlMinor
vespera_inprocess@0.3.1 → 0.4.0 - crates/vespera_inprocess/Cargo.tomlMinor
vespera_jni@0.3.1 → 0.4.0 - crates/vespera_jni/Cargo.tomlMinor
vespera_macro@0.3.1 → 0.4.0 - crates/vespera_macro/Cargo.tomlMinor
Patch
vespera-bridge@0.3.1 → 0.4.0 - libs/vespera-bridge/build.gradle.ktsMinor
vespera-bridge-gradle-plugin@0.3.1 → 0.4.0 - libs/vespera-bridge-gradle-plugin/build.gradle.ktsMinor
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The Criterion regression gate compared measurements taken on two different machines.
The baseline was produced only on
pushtomain, persisted through the actions cache, then restored in a later PR job and compared against numbers measured on whatever runner that PR happened to land on. GitHub'subuntu-latestfleet is heterogeneous, so fleet variance registered as a code regression.Run
33302880992failed 15 benches at +25% to +148% on #141 — a changepacks release PR whose entire diff isversion = "0.3.1"→"0.4.0"strings plus three deleted changepack logs. Zero bytes ofcrates/**changed. The same benchmark code had passed onmaineight minutes earlier (run33302590831).The documented noise filter (
mean > 10%ANDCI lower > 5%) cannot catch this. Criterion's confidence interval measures within-run variance, not between-machine variance — when the two sides run on different hardware criterion becomes more confident, not less. Hence the observed signature of a huge mean shift inside a very tight band (+148.04%mean,+145.58%CI lower).Secondarily,
Cargo.tomlis a path trigger, so every changepacks "Update Versions" PR drags the perf gate along despite touching no source.Changes
--save-baseline main, then the PR head with--baseline main, back to back in one job. The cross-run baseline cache is removed entirely.rm -rf target/criterionbefore the baseline run, so nochange/estimates.jsonsurvives from an unrelated run.Cargo.tomlare parsed withtomlliband compared after removing only changepacks-managed release versions (workspace.package.version, plusversionon path dependencies pointing atcrates/<name>). Anything else — external dependency bumps, profile edits, new dependencies, an unparseable file — fails safe and runs the gate.+10%/+5%→+30%/+20%.Why the thresholds moved
Same-runner comparison cut the noise by an order of magnitude, but did not remove it. Baseline and head are measured minutes apart, so runner drift and co-tenancy still shift the mean.
This PR is its own control: it touches only
bench.yml, so its merge base and head hold byte-identical Rust. Every reported delta below is pure noise.+10%/+5%borrowed_pre_P2/1borrowed_pre_P2/1024,owned_post_P2/1024borrowed_pre_P2/64All three identical-code runs would have failed the old gate, each on a different benchmark. A gate with a ~100% false-positive rate on unchanged code is not a gate.
+30%/+20%sits at roughly 2x the highest observed noise, so the timing gate now catches only big regressions — which is what the header comment always claimed it was for. Precise, deterministic regressions remain covered noise-free by thealloc_budgetintegration test in the normalcargo testjob.Verification
Local, against the real revisions (merge base
31f7e652, #141 head):should_runfalsefalsegarde)truetrueprofile.bench.codegen-units)truetruetruetrueDetermine PR benchmark scopestep was extracted from this YAML and executed underbash -eo pipefail(matching GitHub'sshell: bash) against the real revisions — it emitsmerge_base=31f7e652…andshould_run=false, confirming the heredoc-inside-block-scalar indentation is correct.cargo bench -p vespera_inprocess --bench dispatch -- --liststill resolves all 15 filtered benches.jni-bench.ymlandCI.ymlare untouched.Cost
PR runs measure benches twice, so bench wall-clock roughly doubles (3m50s → ~6m40s). Cargo build outputs are reused across the two measurements within the job, so compile time is not doubled. Release PRs like #141 now skip the gate entirely and get faster.
Note
This must land before #141 can go green — #141 re-run under the current
mainworkflow keeps comparing against the cached cross-machine baseline.