docs(quest): plan CI benchmark regressions and new benches - #4149
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Warning Review limit reachedNext included review available in 16 minutes. View limit detailsLimit details: You’ve used all 4 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (5)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (9)
Included review availability: Your plan provides up to 4 included reviews per hour; 0 remain after this review. WalkthroughThe changes update benchmark guidance and add plans for benchmark CI, Criterion coverage, and in-memory session and relay benchmarks. They also update references to benchmark scripts in existing discovery, comparison, and profiling plans. The changes document proposed work; they do not add benchmark implementations. Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to This PR documents proposed benchmark work and updates references; it does not change benchmark execution. No actionable merge-blocking risk is identified, so it is ready for normal checks. Architecture SummaryArchitecture risk: 🔵 Low · up to The change affects 2 systems. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb52c7593e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 56b6f29935
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Verdict: MERGESolid planning-only PR. The four m1 quests (CI regressions, sans-IO session bench, relay session bench, coverage gaps) fill a real hole: none of the Criterion benches run in CI today. Same-runner base/head PR comments (non-blocking) plus a nightly paired comparison with github-action-benchmark is a better fit than CodSpeed ( Quests are sized and ordered sensibly: Thin justfiles as a required dependency for the impact map, session bench feeding the relay bench, coverage named so CI comments stay readable. Path fixes for existing quests ( No code impact, no complexity debt. Merge and execute the quests in the stated order. This is an automated review, not the maintainer's decision |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Auto-merge enabled. Adds four m1 quests (CI benchmark regressions, sans-IO session bench, relay session bench, bench coverage), widens the CLAUDE.md benchmark guideline, and fixes stale bench script paths in three quests. Review rounds addressed: the auth bench is scoped to today's JWT verification, the nightly check is a same-runner paired comparison, and the session benches sweep publishers too. (Written by Claude Opus 5.5) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 862b8c3fc2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| stays informational: record it with | ||
| [github-action-benchmark](https://github.com/benchmark-action/github-action-benchmark) | ||
| on a data branch, with its own alert disabled. The `moq-uring` socket | ||
| benches run only here, and skip loudly when the runner's kernel is too old. Add the workflow's name to |
There was a problem hiding this comment.
When the nightly runner's kernel is below the moq-uring floor, “skip loudly” leaves the workflow green with no socket measurements, so regressions in those targets are neither recorded nor gated despite the goal promising history for every benchmark. The existing session_lite benchmark implements this condition by printing and returning successfully; select a compatible runner or fail the nightly job when the required benchmark cannot run.
AGENTS.md reference: AGENTS.md:L16-L17
Useful? React with 👍 / 👎.
|
|
||
| ## Related | ||
|
|
||
| - [Benchmark comparisons](/quest/m1/performance-comparisons.md) - extends the same `bench/run.sh` with repeated paired rounds |
There was a problem hiding this comment.
When host performance drifts during a run, the current bench/run.sh::compare_criterion always measures each base case before its head case, so a single fixed-order pair can turn thermal or frequency drift into a threshold crossing. performance-comparisons.md:10-20 already identifies the missing repeated, alternating rounds and dispersion estimate, but leaving that quest merely Related allows this quest to make the noisy result fatal first; require that work or incorporate equivalent pairing before failing Nightly.
Useful? React with 👍 / 👎.
|
|
||
| ## Related | ||
|
|
||
| - [Benchmark regressions in CI](/quest/m1/bench-ci.md) - tracks these targets on PRs and nightly |
There was a problem hiding this comment.
If this ready quest lands before bench-ci.md, its stated outcome that CI tracks the new targets is false because no existing workflow runs the Rust Criterion suite. A Related entry does not block execution, so move this dependency to Required; bench-session.md has the same ordering problem.
AGENTS.md reference: AGENTS.md:L51-L51
Useful? React with 👍 / 👎.
1 participant
Problem
None of the 21 Criterion benches run in CI, so performance regressions surface only when someone runs
just bench BASEby hand.Approach
Four m1 quests:
[M]: PRs get a sticky, non-blocking comment comparing the benches they affect (changed crates plus dependents), with base and head built on the same runner. Nightly keeps a trend with github-action-benchmark and fails past a loose threshold, which routes to Discord throughalert.yml. Themoq-uringsocket benches run nightly only. Requires Thin justfiles for the impact map.[M]: publisher to relay to N subscribers inmoq-netover the in-memory mock transport, swept over subscribers and frame size, lite and IETF.[L]: the same scenario throughmoq-relay's connection handler.[M]: targets for moq-mux containers, the hang catalog, moq-auth verification, and moq-pattern matching.Also:
CLAUDE.mdbenchmark guideline now asks for a benchmark on any performance-sensitive change.rs/scripts/bench.sh; they now point atbench/run.shandbench/relay.sh.Impact
Alternatives
iter_custombench (6 of 22 files) and can't see syscalls.@moq/net): deferred; they stay with Browser benchmarks.Follow-ups
(Written by Claude Opus 5.5)
🤖 Generated with Claude Code