Repository navigation
Compare pull requests against the right benchmark baseline - #1893
Merged
Merged
Conversation
jprendes
requested review from
danbugs,
dblnz,
devigned,
jsturtevant,
ludfjig and
syntactically
as code owners
October 6, 2026 15:04
jprendes
added
C-bug
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The core baseline-selection behavior needs regression coverage.
Review effort: Balanced
Findings: 1
What changed in this PR
Selects benchmark baselines reliably by filtering workflow conclusions locally and reporting baseline distance.
Changes:
- Adds workflow conclusion data and local success filtering.
- Reports commit distance from the requested baseline.
| File | Description |
|---|---|
src/hyperlight_ci/src/remote.rs |
Improves benchmark run selection and diagnostics. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
A pull request is measured against the daily run at the commit it branched from, found by walking the successful runs of that workflow. Asking GitHub for only the successful ones is what made it answer out of a cold index: that listing came back weeks stale in four of ten tries, stably enough that two replies agreed on it, while the same listing unnarrowed was current every time. The narrowing happens here now. PR hyperlight-dev#1881 branched at `66d5dd3e` on 2 October and was reported against `c3e1d311` from 4 September. Of 114 table cells, 102 held no comparison at all, because most of the reported benchmarks did not exist a month earlier, and two configurations announced a regression of 3.79x for a change that redirects fuzzer output. Reading the same run now finds `66d5dd3e` itself and fills every cell. A baseline that far back reads the same as one next door, so the run says how many commits separate them. Signed-off-by: Jorge Prendes <jorge.prendes@gmail.com>
jprendes
force-pushed
the
ci-fresh-baseline-listing
branch
from
October 6, 2026 15:17
be59a3b to
37a61cc
Compare
Benchmark ResultsMeasured commit: kvm / amd (Linux) (❌ 1.64x)Top regressions
Benchmark Resultsfunction_call_codec
payload_allocation
sandboxes
slot_pool
snapshot_files
virtq_readonly
virtq_readwrite
kvm / intel (Linux) (❌ 1.56x)Top regressions
Benchmark Resultsfunction_call_codec
payload_allocation
sandboxes
slot_pool
snapshot_files
virtq_readonly
virtq_readwrite
mshv3 / amd (Linux) (➖ stable)No benchmark improved or regressed. Benchmark Resultsfunction_call_codec
payload_allocation
sandboxes
slot_pool
snapshot_files
virtq_readonly
virtq_readwrite
mshv3 / intel (Linux) (➖ stable)No benchmark improved or regressed. Benchmark Resultsfunction_call_codec
payload_allocation
sandboxes
slot_pool
snapshot_files
virtq_readonly
virtq_readwrite
hyperv-ws2025 / amd (Windows) (➖ stable)No benchmark improved or regressed. Benchmark Resultsfunction_call_codec
payload_allocation
sandboxes
slot_pool
snapshot_files
virtq_readonly
virtq_readwrite
hyperv-ws2025 / intel (Windows) (❌ 1.60x)Top regressions
Benchmark Resultsfunction_call_codec
payload_allocation
sandboxes
slot_pool
snapshot_files
virtq_readonly
virtq_readwrite
Reported by |
andreiltd
approved these changes
Oct 6, 2026
ludfjig
approved these changes
Oct 6, 2026
jprendes
added a commit
that referenced
this pull request
Oct 8, 2026
…#1894) A pull request is measured against the daily benchmark run at the commit it branched from. That run came from another machine. The runner pools hold more than one processor model per vendor and are shared with whatever else is scheduled there, so the difference between two machines reads as a difference between two commits. PR #1893 touches one file of CI tooling and cannot alter a benchmark. It was reported as a 1.58x regression on `hyperv-ws2025 / intel`, because its baseline landed on a processor the rest of the week did not: | run | `virtq_readwrite/slot_pool_segmented/262144` | processor | |:----|---:|:---| | 2 Oct | 43.2k | Model 106 | | 3 Oct | 43.0k | Model 106 | | 4 Oct | 45.2k | Model 106 | | 5 Oct | 43.1k | Model 106 | | **6 Oct, the baseline** | **37.6k** | **Model 143** | | **the pull request** | **59.5k** | **Model 106** | ## Cause Two machines, not two commits. Of the six configurations, two draw from pools holding more than one processor model, where the models differ by 10% and 11.9% while runs on the same model differ by 2% and 4.4%. Shared tenancy moves the rest. `payload_allocation/slot_pool_segmented/262144` sat at 524, 526, 526, 533 and 593 ns across the week on one processor model, and the same commit measured 835 ns on the pull request's runner. Criterion's confidence interval does not see either. It measures the samples within one run, which vary by 0.90%, while a run measured again the next day moves 7.65%, more than the interval allows in 111 of 114 benchmarks. ## What this does `cargo ci bench --baseline-ref <COMMIT>` measures that commit before the one checked out, saving its results as the baseline criterion then compares against. The two sit in one criterion directory, so a report reads the comparison the run carries and names no baseline of its own. The baseline is built in a git worktree, so the checkout in hand is never moved and a run that fails leaves nothing to restore. Each benchmark binary resolves its guests relative to its own source tree, so the baseline reads the worktree while the current tree keeps its own, with no swapping between passes. The guests the baseline needs are built by the job that already holds the toolchain for them. `HEAD^1` names the branch point because the checkout is the merge its validation builds, and both jobs assert that before relying on it, since on anything else `HEAD^1` is the previous commit and would measure against it without saying so. ## Also here criterion-markdown 0.2.2. A report shows criterion's typical estimate, the regression slope wherever linear sampling fits one, while the change beside it read the mean. The mean weights every sample alike and so carries the warm-up the slope leaves behind: `slot_pool/alloc_dealloc_1500` reported 1.56x slower while its slope held 7% faster, from 32 samples that ran cold before the rest settled. ## Verified The pass and its comparison run end to end, leaving `base`, `new` and `change` in one directory for the report to read, and the worktree removed after it. `cargo ci bench` without `--baseline-ref` is unchanged, which is what the daily runs use. The benchmark report on this pull request is the first one measured this way. --------- Signed-off-by: Jorge Prendes <jorge.prendes@gmail.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
A pull request is measured against the daily benchmark run at the commit it branched from. That run was being chosen out of a listing GitHub served stale, so reports compared against commits weeks older than the branch point.
PR #1881 branched at
66d5dd3eon 2 October and was reported againstc3e1d311from 4 September. 102 of its 114 table cells held no comparison at all, because most of the reported benchmarks did not exist a month earlier, and two configurations announced a 3.79x regression for a change that redirects fuzzer output.Cause
run_atasked GitHub for the successful runs of the workflow. Narrowing a listing server-side is what leaves it answering out of a cold index:The runs it omits are indistinguishable from the ones it returns, both
status=completed conclusion=success. The existing guard asks until two replies agree on the newest run, which does not help: the stale answer is stable, so consecutive replies agree on it.The narrowing happens locally now, on a
conclusionfield the listing already carries.Also here
The run reports how many commits separate the baseline from the branch point, read from the comparison it already performs:
A baseline a month back reads the same as one next door. This is what would have made the bug visible the first time it happened.
Verified
Re-running the same report against the same run:
latest_run_forstill filters by--branch, the same class of server-side narrowing. It measured current in 5 of 5 tries, so this leaves it alone.