bench(integration): port integration benchmarks to CodSpeed (simulation + memory) - #898
Conversation
…on + memory) Replace the custom `[[bin]]` integration benchmark (timing + a hand-rolled counting allocator, emitting `customSmallerIsBetter` JSON for github-action-benchmark) with a divan harness run under `cargo codspeed`, the same model the unit benches already use. The benches keep the real async client and the external bartender mock server; only the measurement layer changes. CodSpeed's memory instrument supersedes the custom counting allocator and adds deterministic memory metrics, simulation instruction counts, and flamegraph profiling on the dashboard. - New harness `tests/bench-integration/benches/integration.rs` with a shared multi-thread tokio runtime and a connected-and-warmed client pair, both built once outside the measured region. Benches: connect_to_ready, send_message, send_and_receive, reconnect. The single + x20 coverage is preserved via `#[divan::bench(args = [1, 20])]` looping the op inside the measured region. - `bench-integration` becomes bench-only: drop the `[[bin]]`, the `dhat` dependency / `dhat-heap` feature / cargo-shear ignore, and the now-unused `anyhow`/`wacore`/`log`/`serde`/`serde_json` deps; add `divan` and the `[[bench]]` target. Delete `src/main.rs` and `src/counting_alloc.rs`. - Add a separate `integration-benchmarks` job to `codspeed.yml` carrying the bartender service container, the mock-server wait, a `-p bench-integration` build, and `MOCK_SERVER_URL`/`RUST_LOG` on the run step. The unit-bench job is left untouched so it stays fast and mock-server-free. Delete the old `bench-integration.yml`. End-to-end runs are CI-only (they need the GHCR bartender image); local validation covers compile, clippy, and `cargo codspeed build`.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughWe're killing the homegrown binary benchmark and custom allocator and replacing them with divan integrated into CodSpeed. The old Changesbench-integration migration to divan + CodSpeed
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
📦 Binary size report
.text per crate
Top movers (cargo-bloat attribution)
Baseline: |
![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: af47f1743c
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/codspeed.yml:
- Line 75: Replace the `:latest` tag in the bartender service image reference on
line 75 with a specific version tag or SHA256 digest to ensure reproducibility
in CI environments. Instead of using
`ghcr.io/whiskeysockets-devtools/bartender:latest`, pin it to a concrete version
like `ghcr.io/whiskeysockets-devtools/bartender:v1.0.0` or use the full image
digest format with `@sha256:`. Check the GHCR package registry for available
versions of the bartender image and select an appropriate stable version to pin
to.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1e7be9e0-ec95-4214-8f59-ae927cca1a1b
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
.github/workflows/bench-integration.yml.github/workflows/codspeed.ymltests/bench-integration/Cargo.tomltests/bench-integration/benches/integration.rstests/bench-integration/src/counting_alloc.rstests/bench-integration/src/main.rs
💤 Files with no reviewable changes (3)
- .github/workflows/bench-integration.yml
- tests/bench-integration/src/counting_alloc.rs
- tests/bench-integration/src/main.rs
Merging this PR will not alter performance
Performance Changes
Comparing 
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 7 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/codspeed.yml">
<violation number="1" location=".github/workflows/codspeed.yml:75">
P2: Pin the mock-server container image to a fixed version tag or digest instead of `:latest` so benchmark runs stay reproducible across CI executions.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…easured region
The integration job's `cargo codspeed run` discovered no benchmarks
("No benchmarks found for the simulation mode") because
tests/bench-integration is not a workspace default-member, so a bare
run ignores it. Select it explicitly with `-p bench-integration`,
matching the build step.
Also convert connect_to_ready, send_message and send_and_receive to the
Bencher setup-outside pattern (like reconnect already does) so runtime
construction and the pair connect + Signal warmup run before the
measured closure rather than being charged to the first iteration.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d881a473a9
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/bench-integration/benches/integration.rs`:
- Around line 120-137: The send_message benchmark function reuses a shared pair
without draining inbound messages received by guard.b, causing accumulated state
to leak across iterations and distort results. After the bencher.bench_local
closure completes, add explicit cleanup to consume any pending inbound events
from guard.b, or alternatively modify the benchmark to create a fresh pair for
each iteration instead of reusing the shared pair() so each sample starts from a
clean state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 023c1d48-76d0-40a6-b0ed-86f1cd8f0b9e
📒 Files selected for processing (2)
.github/workflows/codspeed.ymltests/bench-integration/benches/integration.rs
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
send_message is sender-only and never drains client b, so its delivered messages accumulate in b's unbounded event channel. With a single shared pair, send_and_receive reused it and its measured wait_for_text had to discard that backlog before seeing its own message — leaking send_message's iteration count (and divan's run order) into the round-trip result. Give the sender-only and round-trip benches their own warmed pairs so each bench's measurement is independent of the other.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/bench-integration/benches/integration.rs (1)
132-149:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift
send_messagestill accumulates unbounded receiver backlog across samples.At Line 132-Line 149,
guard.bis never drained/reset, so sender-only runs keep growing queued inbound events. That keeps this benchmark stateful over time and can distort CodSpeed memory/simulation outputs even though cross-benchmark leakage is fixed.Please add a per-sample cleanup/reset strategy for the sender-only pair outside the measured region (or recycle that pair on a controlled cadence) so each sample starts from a bounded baseline.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/bench-integration/benches/integration.rs` around lines 132 - 149, The send_message function has an unbounded receiver backlog that accumulates across benchmark samples because guard.b is never drained or reset. After the measured async block in bencher.bench_local completes (but outside the timed region), add cleanup logic to drain or reset guard.b so that each new sample iteration starts with a bounded baseline state. This ensures the benchmark remains stateless across samples and prevents memory/simulation distortion from carryover inbound events.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@tests/bench-integration/benches/integration.rs`:
- Around line 132-149: The send_message function has an unbounded receiver
backlog that accumulates across benchmark samples because guard.b is never
drained or reset. After the measured async block in bencher.bench_local
completes (but outside the timed region), add cleanup logic to drain or reset
guard.b so that each new sample iteration starts with a bounded baseline state.
This ensures the benchmark remains stateless across samples and prevents
memory/simulation distortion from carryover inbound events.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a5a6019e-914f-445f-97b9-ca8af2c27ebe
📒 Files selected for processing (1)
tests/bench-integration/benches/integration.rs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5cb9e6f22
ℹ️ 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".
…vents send_message never reads its recipient (client b), so b's unbounded event channel retained every delivered message and grew across samples, letting CodSpeed memory/simulation numbers depend on how many sends ran earlier. Spawn a background task that drains the send pair's receiver (left off the round-trip pair, which consumes b via wait_for_text) so b stays bounded without adding work to the measured send.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c4affc805
ℹ️ 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".
…n-to-codspeed # Conflicts: # Cargo.lock # tests/bench-integration/Cargo.toml
The earlier drain only covered client b's incoming messages, but a also accrues an Event::Receipt for every delivered DM in its own unread channel — the same unbounded growth across samples, present on both pairs. Drain a's channel in the background as well (always, since no bench reads it); b stays conditional, as the round-trip pair consumes it via wait_for_text.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a79a75e22f
ℹ️ 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".
The measured send closures generated each unique body with format! and text_msg(), charging two harness String allocations per message to the memory-mode results. Move that generation into divan's with_inputs setup (unmeasured) and send the pre-built message by value, so the reported allocations track the send / round-trip path rather than harness overhead.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e90c82e536
ℹ️ 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".
reconnect_and_wait() drains its event channel via try_recv at the start of the call, inside the measured closure — so a sample paid to clear the late init-IQ responses left buffered by the previous reconnect, making the result depend on prior-sample traffic. Drain the channel in divan's with_inputs setup (unmeasured) so each measured reconnect starts from an empty queue.
The integration benches drive the real client (connect/reconnect handshakes) against the mock server under Valgrind, so the run step is inherently slow and variable. Add a 30-minute job timeout so a genuinely stuck run fails fast with a clear signal instead of hanging CI; a healthy run finishes well under it even on slow runners.
2 participants
What
Port the integration benchmarks from a custom
[[bin]](wall-clock timing + ahand-rolled counting allocator, emitting
customSmallerIsBetterJSON forgithub-action-benchmark) to a divan harness run undercargo codspeed, matchingthe model the unit benches already use.
tests/bench-integration/benches/integration.rswith a sharedmulti-thread tokio runtime and a connected-and-warmed
TestClientpair, bothbuilt once outside the measured region. Benches:
connect_to_ready,send_message,send_and_receive,reconnect. Single + x20 coverage ispreserved via
#[divan::bench(args = [1, 20])]looping the op inside themeasured region.
bench-integrationbecomes bench-only: dropped the[[bin]], thedhatdependency /
dhat-heapfeature / cargo-shear ignore, and the now-unusedanyhow/wacore/log/serde/serde_jsondeps; addeddivan+ the[[bench]]target; deletedsrc/main.rsandsrc/counting_alloc.rs.integration-benchmarksjob tocodspeed.yml(bartenderservice container, mock-server wait,
-p bench-integrationbuild,MOCK_SERVER_URL/RUST_LOGon the run step). The unit-bench job is untouched.Deleted the old
bench-integration.yml.Why
CodSpeed's memory instrument supersedes the custom counting allocator and gives
deterministic memory metrics, simulation instruction counts, and flamegraph
profiling on the dashboard — the same observability the unit benches already get
— while keeping the real async client and external mock server unchanged.
Verification
cargo fmt --all -- --check,cargo clippy -p bench-integration --benches -- -D warnings— cleancargo build -p bench-integration --benches,cargo build --workspace --exclude e2e-tests— passcargo codspeed build -p bench-integrationregisters theintegrationsuite