Repository navigation
ci(pr-benchmarks): require an absolute delta as well as a percentage to fail - #372
Merged
Merged
Conversation
…to fail The gate was ratio-only, so scheduling noise on the ~2.5 us benchmark could breach 20% by itself. A benchmark now fails only when it is over the percentage AND moved by >= 1000 ns (time) / >= 16 B (allocations). Floors are derived from 8 historical PR runs and documented beside the thresholds. Also gates a new allocation on a benchmark whose base allocated 0 B, which the percentage path skipped (division by zero -> "new"). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
No unresolved blocking issues were identified.
Review effort: Lite
Findings: None
What changed in this PR
Updates the PR benchmark gate to require both percentage and absolute regression thresholds, including zero-allocation regressions.
Changes:
- Adds documented time and allocation floors.
- Detects regressions from zero-allocation baselines.
- Improves regression reporting.
| File | Description |
|---|---|
.github/workflows/pr-benchmarks.yaml |
Implements percentage-plus-absolute benchmark gating. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
PR benchmarks vs base (f513c6d)Thresholds: time > +20% and ≥ +1000 ns or allocations > +50% and ≥ +16 B (or any allocation ≥ 16 B where the base allocated none) trigger a gate failure (label
|
8 tasks
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.
Summary
The PR benchmark gate was ratio-only (time > +20%, alloc > +50%). A benchmark now fails only when it breaches the percentage and moves by an absolute floor:
TIME_REGRESSION_FLOOR_NSAction_Success) to ~13 ms. Across 8 PR runs (2026-07-17 to 2026-09-17), the largest noise swing was +16.1% onAction_Success, about 400 ns. 1 µs is ~2.5× that. It only affects sub-5 µs benchmarks, because 20% of anything larger already exceeds it.ALLOC_REGRESSION_FLOOR_BYTESIt also closes a gap: a benchmark whose base allocated 0 B was never gated, because the percentage divided by zero and the row was reported as "new". A new allocation of at least 16 B on a zero-alloc path now fails. That matters for a library with
ZeroAllocationTests.The floor alone should be enough; retry-before-fail is left out. These are pure in-memory benchmarks, and the noise observed stayed under the 20% threshold.
Verification
I extracted the gate's Python and ran it locally (Python 3.12) against synthetic base/head reports:
actionlint and zizmor aren't installed locally, so the workflow lint runs in CI only.
Merge note
This touches
.github/workflows/, so the protected-file guard will need an admin-bypass merge.Closes #326
🤖 Generated with Claude Code