Skip to content

bench(integration): cut CodSpeed variance with a fixed 2-worker runtime + deterministic allocator - #902

Merged
jlucaso1 merged 2 commits into
mainfrom
claude/happy-newton-h0nk9d
Jun 18, 2026
Merged

jlucaso1 merged 2 commits into
mainfrom
claude/happy-newton-h0nk9d

Conversation

@jlucaso1

@jlucaso1 jlucaso1 commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

Motivation

The integration benchmarks show run-to-run variance and trip CodSpeed's "Different runtime environments detected" warning (seen on #901, where send_and_receive[20] Memory and send_message[20] Simulation moved with no real code cause). This applies two techniques from CodSpeed's reducing-variance and regression-causes guides, both scoped to the bench binary only — production code is untouched.

Changes

Fixed-size multi-thread runtime. The default multi-thread runtime sizes its worker pool to the runner's CPU count, so thread-stack memory and scheduling varied per runner — a source of the cross-environment noise. The shared runtime is now pinned to a fixed worker_threads(2), making the benches runner-independent.

Iteration note: the first commit used new_current_thread() for maximum determinism, but its CodSpeed run surfaced a real memory regression — with a current-thread runtime the background event drainers in connect_warmed_pair only advance inside block_on, so they freeze between divan samples and each sample's backlog leaks into the next (send_message[20] Memory 38.8 KB → 124.7 KB). A fixed 2-worker pool keeps those drainers running continuously between samples while still removing the per-runner thread-count variance. (Caught by Codex in review.)

Deterministic global allocator (bench-only). Added a #[global_allocator] whose realloc always allocates a fresh block and copies min(old, new) bytes instead of growing in place. Whether the system allocator can grow in place depends on the live heap layout, which varies run-to-run and was charged to the memory instrument as noise. Production keeps the system allocator.

Expected effect

One-time re-baseline: the deterministic allocator may make the Memory number slightly higher but stable (the alloc+copy realloc has a transient old+new peak). After that step, variance should drop.

Validation

  • cargo clippy -p bench-integration --benches and cargo fmt clean.
  • The Run CodSpeed integration benchmarks job passes — the client drives fine on the pinned runtime.
  • Variance is validated by this PR's CodSpeed comparison.

Out of scope (deferred)

Synchronous event draining in unmeasured setup (an alternative to the fixed pool), pinning the bartender mock image to a digest, and evaluating a dedicated CodSpeed runner — the last is the actual root cause of the cross-environment warning but is an infra/cost decision.

…me + deterministic allocator

The integration benches only run CodSpeed's Valgrind simulation+memory
instruments, which serialize threads onto a single core. A multi-thread runtime
therefore bought no measurable signal, only variance: its worker count tracked
the runner's CPU count, so thread-stack memory and scheduling differed per
runner (feeding the "different runtime environments" noise on send_message[20]).
Switch to a current-thread runtime; the client has no block_in_place, so it
drives fine and the scheduling becomes deterministic.

Add a bench-only #[global_allocator] whose realloc always allocates a fresh
block and copies instead of growing in place. In-place growth depends on the
live heap layout, which varies run-to-run and was charged to the memory
instrument as noise (send_and_receive[20]). Both follow CodSpeed's
reducing-variance guide; production keeps the system allocator and runtime.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014VC9pPVwJqgcic3KEn4y98
@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The integration benchmark harness adds a DeterministicAlloc custom global allocator that forces every realloc to allocate a fresh block, copy the overlapping byte range, and free the old block—this prevents in-place reallocation from distorting CodSpeed memory metrics. The shared Tokio runtime is also pinned to exactly 2 worker threads instead of relying on defaults, ensuring consistent runtime behavior across benchmark runs.

Changes

Benchmark Infrastructure: Deterministic Allocator and Fixed Tokio Runtime

Layer / File(s) Summary
DeterministicAlloc global allocator
tests/bench-integration/benches/integration.rs
Installs DeterministicAlloc via #[global_allocator]; alloc/dealloc delegate to System while realloc always allocates a new block, copies min(old_size, new_size) bytes, and deallocates the old block to prevent in-place growth from skewing memory instrumentation.
Tokio runtime worker thread count
tests/bench-integration/benches/integration.rs
Sets the shared Tokio runtime to an explicit worker_threads(2) count, replacing implicit defaults while preserving the multi-thread executor with enable_all().

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • oxidezap/whatsapp-rust#898: The integration benchmark harness that introduced the file being modified here for CodSpeed integration and Tokio runtime initialization.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and specifically describes the two main changes: a fixed 2-worker runtime and a deterministic allocator, directly matching the PR's core objectives.
Description check ✅ Passed The PR description clearly explains the motivation (reducing variance in integration benchmarks), details the two key changes (fixed 2-worker runtime and deterministic allocator), documents technical decisions with trade-off analysis, and validates the approach.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/happy-newton-h0nk9d

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

github-actions Bot commented Jun 18, 2026

Copy link
Copy Markdown

📦 Binary size report

Metric main PR Δ
bin size (stripped) 10.59 MiB 10.59 MiB 0
bin .text 8.70 MiB 8.70 MiB 0
bin allocated (text+data+bss) 10.59 MiB 10.59 MiB 0
llvm-lines wacore 639,594 639,594 0
llvm-lines wacore copies 17,666 17,666 0
llvm-lines whatsapp-rust lib 654,931 654,931 0
llvm-lines whatsapp-rust lib copies 20,159 20,159 0
deps crates (Cargo.lock) 347 347 0
.text per crate
Crate main PR Δ
.text whatsapp_rust 1.44 MiB 1.44 MiB 0
.text wacore 517.29 KiB 517.29 KiB 0
.text wacore_binary 156.73 KiB 156.73 KiB 0
.text wacore_libsignal 166.63 KiB 166.63 KiB 0
.text wacore_appstate 36.98 KiB 36.98 KiB 0
.text wacore_noise 27.71 KiB 27.71 KiB 0
.text waproto 876.21 KiB 876.21 KiB 0
.text whatsapp_rust_sqlite_storage 207.48 KiB 207.48 KiB 0
.text whatsapp_rust_tokio_transport 32.49 KiB 32.49 KiB 0
.text whatsapp_rust_ureq_http_client 5.93 KiB 5.93 KiB 0
.text std 1.13 MiB 1.13 MiB 0
.text other deps 4.07 MiB 4.07 MiB 0
Top movers (cargo-bloat attribution)
Crate main PR Δ
regex_automata 3.06 KiB 1.67 KiB -1.39 KiB (-45.49%)
prost 471.26 KiB 472.66 KiB +1.39 KiB (+0.30%)

Baseline: d252c7caf (latest main run) · Head: 06d35e15c · Graphs

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

Re-trigger cubic

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 216e0caf00

ℹ️ 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".

Comment thread tests/bench-integration/benches/integration.rs Outdated
…rent-thread

current_thread froze the background event drainers (connect_warmed_pair) between
divan samples: spawned tasks only progress inside block_on, so receipts and
delivered messages arriving after a measured send leaked into the next sample,
regressing the cross-sample isolation the drainers provide (flagged by Codex on
this PR). Pin a fixed 2-worker multi-thread runtime instead: its worker threads
keep draining continuously between samples, while the fixed count stays
independent of the runner's CPU count -- the per-runner thread-count variance the
original switch was meant to remove. The deterministic allocator is unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014VC9pPVwJqgcic3KEn4y98
@codspeed-hq

codspeed-hq Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will improve performance by 16.5%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 2 improved benchmarks
✅ 182 untouched benchmarks

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Memory send_message[20] 38.8 KB 32.3 KB +20.06%
Memory send_and_receive[1] 13.6 KB 12 KB +13.05%

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.


Comparing claude/happy-newton-h0nk9d (738bdcb) with main (d252c7c)

Open in CodSpeed

@jlucaso1 jlucaso1 changed the title bench(integration): cut CodSpeed variance with a current-thread runtime + deterministic allocator bench(integration): cut CodSpeed variance with a fixed 2-worker runtime + deterministic allocator Jun 18, 2026
@jlucaso1
jlucaso1 merged commit 21aa1e2 into main Jun 18, 2026
14 checks passed
@jlucaso1
jlucaso1 deleted the claude/happy-newton-h0nk9d branch June 18, 2026 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants