perf(send): pin the warm group stanza as flat in group size by jlucaso1 · Pull Request #1279 · oxidezap/whatsapp-rust

jlucaso1 · 2026-08-11T15:25:02Z

Summary

Target: encoding proportional to the participant count on the group send path. An external profile of a 128-member group-send reported ~736 instructions of growth per additional member per message, with classify_string_hint / write_node / node_encoded_size_with_cache named as +16.5K Ir / +155 calls of it, and suggested the encoded participant node was a candidate for caching between sends.

Refused: there is nothing per-member to cache. Measured in-process, prepare_group_stanza costs 333,963 instructions at 8 members and 334,099 at 512 — +136 instructions over +504 members. <participants> carries sender-key distributions only, and a steady-state send distributes none to members; the phash covering the whole device set is a fixed-width digest memoized on the resolved set. The encoded warm stanza is the same size at 8 and at 512 members. node_encoded_size_with_cache's recursion mark in the external profile is not a missing cache — StringHintCache is a single-marshal plan→encode replay tape, by construction, not a memo across sends.

The one thing a warm send does fan out is a fresh SKDM to our own companion devices, every send, because own devices are never memoized warm (WA Web !isMeDevice, see update_sender_key_devices). So the stanza's per-recipient content tracks our own device count and never the group's. Both steady states are pinned below.

What actually scaled in here was the benchmark. run_group_send built and dropped an N-participant GroupInfo inside the measured body. Production resolves the group once and holds it behind an Arc across sends — ensure_self_in_group hands the same Arc straight back whenever we are already a member, which is the steady state — so no send pays that. At 512 members the fixture charged 26.8K instructions per send, 7.2% of the measurement, and it was the whole of the apparent group-size growth.

Changes

wacore/benches/send_receive_benchmark.rs — hoist GroupInfo construction (and the one-time self-append) out of run_group_send into setup_group_send, matching how production holds the resolved group across sends. bench_group_send_* now measures the send.
wacore/src/send/tests.rs — warm_group_stanza_size_tracks_own_devices_not_group_size: prepares a warm group stanza at 8 and at 512 members, for both a single-device account and a two-companion one, and asserts the <to jid> values are exactly our companions (not merely how many), that each carries enc type="msg", that the phash is a 2:-prefixed fixed-width digest, that the whole node hierarchy matches recursively, and that the encoded sizes match. Pinned as a test, not a bench, because the claim is about the shape of the stanza — a change that folded member state into it would still benchmark fine on a small group. Sizes are compared with byte payloads normalised, since every plaintext is padded by a uniform random 1..=16 bytes and the ciphertext length therefore differs run to run.
wacore/binary/benches/group_fanout_benchmark.rs (new target) — bench_marshal_exact_group_fanout over widths [8, 32, 128, 512], sweeping the one group stanza whose encode really is proportional to its recipients (the SKDM fan-out). Keeping both facts measurable is what distinguishes "the warm stanza grew a per-member node" from "this account has companions" or "this group is redistributing". Recipients are a typed Jid per device spread over distinct users, as build_participant_node produces them, and the sweep marshals through marshal_exact because that is what Client::marshal_node_for_send picks for every outbound stanza. Its ciphertexts are type="msg", so it characterizes redistribution and explicitly not a first-contact fan-out. It is its own crate root rather than a section of binary_benchmark because the fixture perturbed an unrelated benchmark there — see below. binary_benchmark.rs is byte-identical to main.

No production code changed.

Cost

Method: callgrind (valgrind 3.22) over a driver built from the same bench fixture, running K warm sends after one setup. Per-send instructions are (Ir at K=51 − Ir at K=1) / 50, so fixture setup cancels exactly. Allocations from divan::AllocProfiler. Times from cargo bench (profile.bench).

Per warm group send, instructions:

group_size	before	after	Δ
8	341,312	340,670	−0.2%
32	342,958	339,932	−0.9%
128	349,612	340,914	−2.5%
512	374,812	340,890	−9.0%

Before, 8 → 512 grew +9.8%; after it is flat within ±0.3% with no trend. prepare_group_stanza itself is unchanged by this PR and was already flat (333,963 → 334,099).

The encoder, per warm group send — identical at 8 and at 512 members:

	Ir/send	calls/send
`marshal`	5,087	1
`write_node`	5,009	4
`Node::encode_attrs`	3,386	4
`classify_string_hint`	2,334	21
`write_string_with_hint`	1,279	21
`parse_jid_meta`	715	5

Marshalling is 1.5% of a warm group send and does not move with the group.

Allocation, per warm group send: 22 allocations / 3.58 KB, identical at 10, 50 and 256 members. malloc+free+realloc ≈ 6.0K Ir = 1.8% of the send.

Hashing, per warm group send, inside wacore: hash_one 722 Ir / 2 calls + sip::Hasher::write 518 Ir / 6 calls = 1,240 Ir = 0.37%, identical at 8 and at 512.

Where a warm group send actually goes (inclusive, 8 members, 334K total):

	Ir/send	share
Ed25519 signature over the `SenderKeyMessage`	227,635	68%
SHA-256 (chain step + reporting token)	90,936	27%
reporting token	39,391	12%
marshal	5,087	1.5%
malloc/free	~6,000	1.8%

One signature per message is the protocol's cost, not ours. (The external profile's "Ed25519 verify 40.8%" is the same operation seen from the receive side; on the send side it is calculate_signature.)

Fan-out marshal sweep, wall time: 1.79 µs (8) / 5.44 µs (32) / 20.2 µs (128) / 104.2 µs (512) — ~0.20 µs per recipient, linear, for the msg redistribution shape. A first-contact fan-out carries the larger pkmsg payload once per recipient, which raises that per-recipient term; this sweep is a lower bound there and is documented as not characterizing it.

CodSpeed against the local numbers

Latest run against main (5c9e4dc): 0 regressions, 4 improved, 238 untouched, 8 new.

benchmark	mode	`BASE`	`HEAD`
`bench_group_send_256`	Simulation	225.8 µs	205.2 µs	+10.07%
`bench_group_send_256`	Memory	19.2 KB	3.2 KB	×6.1
`bench_group_send_50`	Memory	6.3 KB	3.2 KB	+99.19%
`bench_group_send_10`	Memory	3.8 KB	3.2 KB	+19.34%

Divergences from what I measured locally, all expected:

Only 256 moves on Simulation. The fixture overhead is proportional to the group, so it is 2.8K Ir at 8 members and 26.8K at 512 — immaterial at the small sizes, which is exactly what the local sweep shows (−0.2% at 8, −9.0% at 512). CodSpeed's largest group bench is 256.
+10.07% there against my −9.0% at 512. Different metric and different size: CodSpeed's Simulation is a cycle estimate over a cache model, mine is raw instruction count, and its 256 sits between my 128 (−2.5%) and 512 (−9.0%) data points. Same direction, same order.
Memory 19.2 KB → 3.2 KB at 256. That is the participant Vec — 256 Jids, ~16 KB — that the fixture allocated and dropped every iteration and production never does. The post-fix figure is flat at 3.2 KB across 10/50/256, matching the 3.58 KB flat my AllocProfiler run measured (the two instruments account differently; the flatness is the claim).

The fan-out sweep is on record in CI as linear in both instruments — 23.2 / 56.7 / 187.9 / 711.1 µs and 1.9 / 7.4 / 29.4 / 117.4 KB at widths 8 / 32 / 128 / 512. Binary size is unchanged (every metric 0).

bench_attr_parser, +300 Ir — found and removed. CodSpeed flagged it, and my first reading (runner variance) was wrong; it reproduced locally at 8,605.4 → 8,906.3 Ir/iteration on one machine. My second reading was also wrong on the where: I put it in the owned-builder path in setup, but the flamegraph shows the single malloc in Decoder::read_attributes taking glibc's unlink_chunk slow path. No library code changed; the cause was the 512 transient format!-built JID strings the first fan-out fixture allocated per iteration. Two commits close it independently — the typed-Jid switch (which removed those allocations, and landed for the u8 device-id finding) and the move to a separate crate root, which returns it to exactly 8,605.4 and makes binary_benchmark.rs byte-identical to main so no future fixture there can re-trigger it. It is back in CodSpeed's untouched bucket.

Checked and not changed

Target 1 — SipHash on the hot path. Inside wacore's group send, hashing is 1,240 Ir = 0.37% and does not grow with the group; there is nothing here to win. Independently of that, per-map key control, which is the binding question:

map	key	who controls the key	verdict
`SenderKeyDeviceMap.devices` (`src/sender_key_device_cache.rs`)	participant user string	network — built from the participant list the server sends	keep `RandomState`
`SignalStoreCache` sessions / identities / sender_keys / sender_key_locks (`wacore/src/store/signal_cache.rs`)	protocol address `user.device`, `group::address`	network — peer JIDs and group ids	keep `RandomState`
`GroupDevicesMemo.members`, the set behind `resolve_skdm_targets_memoized` (`src/client/device_registry.rs`)	participant users plus their LID/PN aliases	network	keep `RandomState`
`LidPnCache`'s `TypedCache<Arc<str>, …>` (`src/lid_pn_cache.rs`)	LID / PN user strings	network	keep `RandomState`
`SignalStoreCache::removed_prekeys`	our own prekey id (`u32`)	local	locally keyed, but zero calls on the group send path — no measurable gain, left alone

Every map the profile named is fed by the group participant list or by peer JIDs. That is exactly the case the DoS-resistant default exists for, and a group's membership is the one input an adversary can influence directly. A blanket default-hasher swap is not on the table either.

Target 3 — allocation. 22 allocations / 3.58 KB per warm group send in wacore, flat in group size, ~1.8% of the send in instructions — the same order as the 0.43% the DM batch measured and declined. The external 387 allocs / 87.7 KiB is a whole-client number; wacore's stanza path is ~6% of it, and the named growth site (ensure_sessions_for_devices) is on the SKDM path, which a steady-state send takes only for own companions. Not attacked here, and it would be a different PR regardless.

A pkmsg fan-out sweep. The cold path's per-recipient cost is genuinely higher, so it cannot be inferred from the msg numbers. Rather than assert a slope I have not measured, the benchmark documents that it does not cover that case. Adding the sweep is worth its own change.

Divergence from the external profile. ~66 of the reported 736 Ir/member (9%) is reachable from wacore, and this PR shows all of it was the fixture. The remaining ~670 Ir/member lives in the whatsapp-rust client crate, which has no benchmarks and is not in a CodSpeed shard — filter_skdm_targets is one hash lookup per device, memoized by skdm_warm_memo only while the (devices, sender-key-map) Arc pair, generation and sending identity all hold. Measuring that needs client-level benchmarks that do not exist yet; nothing in this PR claims to have measured it.

Which group sizes this is about. The fixture overhead removed here is 2.8K Ir at 8 members and 26.8K at 512, so the measurement only changes meaningfully at 128 and above. The flatness claim holds at every size.

Validation

cargo fmt --all
cargo clippy --workspace --all-targets -- -D warnings — clean
cargo test -p wacore -p wacore-binary -p wacore-libsignal -p wacore-appstate -p wacore-noise -p whatsapp-rust — all green (wacore 1418 passed, whatsapp-rust 1608 passed); voip-cli skipped locally, its alsa-sys build dependency is not installed in this container
The new test run 20–40× in a row after each change, to confirm the padding-normalised size comparison is stable
The recursive shape assertion checked by mutation: injecting an attribute into a nested <enc> on the 512-member stanza alone fails it, where the previous direct-children comparison passed
bench_attr_parser measured under callgrind at main, at this branch, and after the bench split, to confirm the CodSpeed regression is gone rather than merely re-rolled
cargo bench -p wacore --bench send_receive_benchmark -- group_send and cargo bench -p wacore-binary --bench group_fanout_benchmark both run

"Semver Checks (informational)" is red. It is continue-on-error: true by design ("advisory only … this job exists to make the break visible in the summary, not to block the PR") and compares the API against the last published release, not this PR's base; this diff adds no public surface at all — bench files and one #[cfg(test)] module.

Caveats carried over from the profile this batch came from, since the numbers above are compared against it: its cycle sweep is not monotonic (361K, 446K, 354K, 441K) — only instructions grow cleanly, so the wall-time effect is smaller and noisier than the instruction effect, which is what I see here too (instructions −9.0% at 512, wall time ~−1% and inside the noise). Its per-item attribution in the time profile is ~3 samples per row and comes from callgrind, not perf; the 39.9% aggregate is the solid part. Its allocation numbers already exclude the benchmark's own build_bot_async handler.

An external profile of a 128-member group send reported ~736 instructions of growth per additional member per message, attributing part of it to the encoder. Measured in-process, `prepare_group_stanza` is flat: 333,963 instructions at 8 members and 334,099 at 512 (+136 over +504 members). A steady-state group send carries no per-participant data at all, so there is no encoded participant node to cache between sends. The reason the group benchmarks looked like they scaled is the fixture: `run_group_send` built and dropped an N-participant `GroupInfo` inside the measured body. Production resolves the group once and holds it behind an `Arc` across sends (`ensure_self_in_group` returns the same `Arc` whenever we are already a member, the steady state), so no send pays that. At 512 members it charged 26.8K instructions per send, 7.2% of the measurement, and was the whole of the apparent group-size growth. Hoist it into setup, pin the shape claim as a test (the encoded warm stanza is the same size for a group of 8 and a group of 512), and sweep the one group stanza whose encode really is proportional to participants — the sender-key distribution fan-out — so a regression that folds per-participant state into the warm stanza is distinguishable from a group that is merely redistributing. Per warm group send, by callgrind, before -> after: 8 members 341,312 -> 340,670 32 members 342,958 -> 339,932 128 members 349,612 -> 340,914 512 members 374,812 -> 340,890 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TeyUovZnNhUUR2bNhL3QhY

coderabbitai · 2026-08-11T15:25:35Z

📝 Walkthrough

Summary by CodeRabbit

Bug Fixes
- Improved consistency and sizing of group message stanzas across different group sizes and companion-device configurations.
- Ensured sender-key distributions are delivered only to the appropriate companion devices.
Tests
- Added regression coverage for group stanza structure, serialization size, recipient handling, and message metadata.
Performance
- Added benchmarks for group fan-out processing and optimized repeated group-send measurements.

Walkthrough

The PR caches GroupInfo in send benchmarks, adds a sender-key fan-out marshalling benchmark for multiple recipient widths, and adds regression coverage for warm group stanza structure and serialized size.

Changes

Warm Group Fanout

Layer / File(s)	Summary
Cache group metadata for send benchmarks `wacore/benches/send_receive_benchmark.rs`	The group-send fixture builds `GroupInfo` during setup and reuses it for each send iteration.
Measure fanout serialization widths `wacore/binary/Cargo.toml`, `wacore/binary/benches/group_fanout_benchmark.rs`	A disabled-harness Divan benchmark builds typed-JID sender-key fan-out nodes and measures `marshal_exact` for 8, 32, 128, and 512 recipients.
Validate warm-group stanza shape `wacore/src/send/tests.rs`	The regression test checks companion distribution, fixed-width `phash` values, child structure, and equal normalized serialized sizes for 8- and 512-member groups.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

oxidezap/whatsapp-rust#501: Modifies the same send benchmark fixture for prepared group data.
oxidezap/whatsapp-rust#610: Covers related group sender-key fan-out performance and send-path changes.
oxidezap/whatsapp-rust#824: Covers related group-send device resolution and phash handling.

Suggested labels: performance

Suggested reviewers: greptile-apps

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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 identifies the performance change to keep warm group stanza size flat across group sizes.
Description check	✅ Passed	The description directly explains the benchmark, regression test, fan-out benchmark, measurements, validation, and absence of production-code changes.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch claude/group-send-perf-analysis-if3q1x

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.}

greptile-apps · 2026-08-11T15:28:57Z

Greptile Summary

The PR moves reusable group metadata out of the measured send loop, adds coverage for warm sends with companion devices, and introduces a dedicated binary fanout benchmark.

Reuses GroupInfo across benchmark iterations.
Pins warm stanza shape and normalized encoded size across group sizes.
Registers a standalone group-fanout benchmark target.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
wacore/benches/send_receive_benchmark.rs	Hoists group participant construction and self-inclusion into benchmark setup.
wacore/src/send/tests.rs	Extends warm-group coverage to single-device and companion-device account shapes.
wacore/binary/benches/group_fanout_benchmark.rs	Adds a standalone typed-JID fanout marshalling benchmark.
wacore/binary/Cargo.toml	Registers the new standalone benchmark target.

_{Reviews (6): Last reviewed commit: "test(send): make the warm fixture actual..." | Re-trigger Greptile}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2fef0272eb

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

chatgpt-codex-connector · 2026-08-11T15:31:59Z

+    let devices: Vec<Node> = (0..width)
        .map(|i| {
            NodeBuilder::new("to")
                .attr("jid", format!("5511999990000:{i}@s.whatsapp.net"))


Keep fanout device IDs within the AD_JID range

At the 512-recipient benchmark width, i reaches 256, but parse_jid_meta accepts device components only as u8; recipients 256–511 therefore stop using the intended AD_JID encoding and are encoded as JID_PAIR values with the colon embedded in the user. This makes the largest measurement mix two encoding paths and no longer represent a real sender-key fanout, undermining the scaling result the new benchmark is meant to track. Generate additional users while keeping every device ID at or below 255.

Useful? React with 👍 / 👎.

…size Three fixes to the new fixtures, none touching production code. The size assertion was flaky and CI caught it: every plaintext is padded by a uniform random 1..=16 bytes, so the skmsg ciphertext length differs run to run (CI hit 274 vs 291). Normalise every byte payload to a fixed size before comparing — the claim under test is the stanza's structure and attributes, not the ciphertext. 40/40 stable locally after the change. The warm fixture also over-claimed. A steady-state send distributes no sender key to group *members*, but it does re-distribute to our own companions on every send, because own devices are never memoized warm (WA Web `!isMeDevice`). A blanket "no <participants>" assertion therefore pinned only the single-device shape. The test now runs both steady states — 0 and 2 own companions — at 8 and at 512 members, and asserts the distributed count equals the companion count in each. That is the real invariant: the stanza tracks our own device count, never the group's. The fan-out marshal sweep modelled recipients as one user numbered 0..width, so above 255 the device component overflowed `parse_jid_meta`'s `u8` and those recipients silently encoded as JID_PAIR — the 512-wide measurement was averaging two wire shapes. Recipients are now a typed `Jid` per device (as `build_participant_node` passes them, skipping a string classifier production never runs) spread over distinct users with 4 devices each, so the whole sweep stays on the AD_JID path. `create_fanout_node` keeps its original width-8 body, leaving `bench_unmarshal_fanout`'s baseline untouched. Sweep after the fix: 1.55 / 4.81 / 17.7 / 88.3 µs at widths 8 / 32 / 128 / 512 — still linear, ~0.17 µs per recipient. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TeyUovZnNhUUR2bNhL3QhY

codspeed-hq · 2026-08-11T15:35:20Z

Merging this PR will improve performance by 99.95%

⚠️

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

⚡ 4 improved benchmarks
✅ 238 untouched benchmarks
🆕 8 new benchmarks
⏩ 2 skipped benchmarks¹

Performance Changes

	Mode	Benchmark	`BASE`	`HEAD`	Efficiency
⚡	Memory	`bench_group_send_256`	19.2 KB	3.2 KB	×6.1
⚡	Memory	`bench_group_send_50`	6.3 KB	3.2 KB	+99.19%
⚡	Memory	`bench_group_send_10`	3.8 KB	3.1 KB	+19.94%
⚡	Simulation	`bench_group_send_256`	225.8 µs	205.2 µs	+10.05%
🆕	Memory	`bench_marshal_exact_group_fanout[128]`	N/A	29.4 KB	N/A
🆕	Memory	`bench_marshal_exact_group_fanout[32]`	N/A	7.4 KB	N/A
🆕	Memory	`bench_marshal_exact_group_fanout[512]`	N/A	117.4 KB	N/A
🆕	Memory	`bench_marshal_exact_group_fanout[8]`	N/A	1.9 KB	N/A
🆕	Simulation	`bench_marshal_exact_group_fanout[128]`	N/A	187.9 µs	N/A
🆕	Simulation	`bench_marshal_exact_group_fanout[32]`	N/A	56.7 µs	N/A
🆕	Simulation	`bench_marshal_exact_group_fanout[512]`	N/A	711.1 µs	N/A
🆕	Simulation	`bench_marshal_exact_group_fanout[8]`	N/A	23.2 µs	N/A

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/group-send-perf-analysis-if3q1x (d14a123) with main (5c9e4dc)}

2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

jlucaso1 · 2026-08-11T15:37:22Z

@codspeedbot why the benchmark bench_attr_parser regressed?

codspeed-hq · 2026-08-11T15:37:55Z

@codspeedbot why the benchmark bench_attr_parser regressed?

The 8.08% on bench_attr_parser is not in the decoder — it is a single malloc call inside it landing on a slower glibc path because of heap state left behind by the other benchmarks in the same bench binary. Nothing in wacore-binary changed on this branch, and the effect is already gone at the branch's current tip.

Where the 0.69 µs is

bench_attr_parser allocates exactly once per iteration: the attrs Vec in Decoder::read_attributes. Comparing the two flamegraphs (base 763aea9 vs head), every user-code frame has identical call counts and near-identical cost, and the whole delta is that one allocation:

frame (total)	`BASE`	`HEAD`
`malloc` subtree under `read_attributes`	1.23 µs (15.8%)	1.92 µs (22.6%)
`_int_malloc` callees	`checked_request2size`, `alloc_perturb`	`checked_request2size`, `unlink_chunk.isra.0` (131 ns)
benchmark total	7.81 µs	8.49 µs

+0.69 µs in malloc, +0.686 µs in the benchmark. On the head run that one allocation missed the fast path and had to unlink a chunk from a bin; on base it did not. That is a property of the heap's free lists at the moment the benchmark ran, not of the code being measured.

Reproduced locally

Built the wacore-binary benches at three revisions and ran them with codspeed run --mode simulation on one machine (so no CPU difference in play), with the same MALLOC_* pins the workflow sets:

bench file from	how it ran	Simulation
`main` (`763aea9`)	full suite	8.036 µs
`2fef027` (measured by CI)	full suite	8.569 µs (+6.6%)
`2fef027` (same binary)	`bench_attr_parser` alone	7.948 µs (no regression)
branch tip `77bf9ab`	full suite	8.007 µs (−0.4%), identical on repeat

Same binary, same inputs: 8.57 µs when the rest of the suite ran first, 7.95 µs on its own. Restricting the run to the new fan-out benches plus bench_attr_parser reproduces part of it (8.14 µs), which is what you'd expect from a perturbation carried in the allocator rather than a single culprit.

Why it is already fixed

The commit on top of the one CodSpeed measured (77bf9ab) replaced the fan-out fixture's 512 format!-built JID strings with typed Jid values — a 13-char user is inline in CompactString, so those 512 transient small allocations per iteration are gone. At that tip bench_attr_parser is back to base cost (8.007 µs vs 8.036 µs, bit-identical across two runs). The next CodSpeed run on this branch should drop the regression.

Two caveats worth having on record:

This is heap-layout luck, not a guarantee. Any future bench added to binary_benchmark.rs can re-trigger it, because bench_attr_parser's one allocation is effectively a tell for the allocator's bin state. The MALLOC_ARENA_MAX/MALLOC_*_THRESHOLD_ pins in codspeed.yml freeze the adaptive thresholds but not free-list layout.
The report also flagged different runtime environments: base ran on an Intel Xeon 8573C and head on an AMD EPYC 9V74, which shifts the simulated cache model. That is real but small here — bench_unmarshal_small, for instance, moved +2.3% on the memory term with a byte-identical instruction term across the same pair of runners. It is not what produced the 8%.

No code changes made; nothing to fix beyond the commit already on the branch.

Summary

Warming up the performance wizard...
Investigating bench_attr_parser regression — The 8% is one glibc malloc call inside read_attributes taking the unlink_chunk path because of heap state left by the new fanout bench; reproduced locally and confirmed the branch's latest commit removes it. No code changes needed.
Handling comment — Comment handling finished successfully

_{Learnings updated. See what CodSpeed remembers about this repository.}

_{Was this response helpful? React with 👍 or 👎 to help us improve CodSpeedBot.}

jlucaso1 · 2026-08-11T15:38:34Z

It didn't — the base and head ran on different CPUs, and CodSpeed's own comparison names bench_attr_parser as one of the benchmarks that crossed the difference.

Base run (main @ 763aea9) ran on INTEL(R) XEON(R) PLATINUM 8573C; head run (2fef027) ran on AMD EPYC 9V74. Querying the comparison directly, binary_benchmark.rs::bench_attr_parser (Simulation) is listed under Environment Differences → Hardware, with CodSpeed's own note:

Simulation mode estimates CPU cycles by running the compiled code on a simulated CPU whose cache model is derived from the physical CPU. A different CPU or toolchain changes the simulated cache behavior, so the estimate can shift on identical source.

57 other benchmarks crossed the same Intel→AMD swap and came back unchanged, so the swap mostly cancels; bench_attr_parser is the one that landed outside the band. Consistent with that, nothing in this PR can reach it: the diff to binary_benchmark.rs adds create_skdm_fanout_node plus one new #[divan::bench], and create_attr_node, bench_attr_parser, unmarshal_ref and the attr parser are untouched. As of the current head (77bf9ab) create_fanout_node is byte-identical to main again, so bench_unmarshal_fanout's fixture is unchanged too.

The run on 77bf9ab will re-measure it; if it lands on a matching runner the delta should disappear. I'll flag it if it doesn't.

Generated by Claude Code

CodSpeed flagged `bench_attr_parser` as +369 Ir/iteration, and it was real — not the Intel→AMD runner swap I first attributed it to. Measured locally with callgrind, base 8,605.4 Ir/iteration against head 8,906.3. None of it is library code. The fan-out fixture instantiates the typed-JID attribute path (`NodeBuilder::attr::<Jid>`), and adding it to that crate root shifted inlining enough that `create_attr_node`'s builder stopped pre-sizing its `SmallVec` and grew through an extra realloc instead: the delta is `Attrs::insert` +261.7, `SmallVec::try_grow` +254.7, `realloc` +255 — the owned-node path, which `bench_attr_parser` reaches only from its setup, never from the `unmarshal_ref` + attr-parse body it measures. Moving the sweep to its own crate root puts `bench_attr_parser` back at 8,605.4 — exactly the base figure — and leaves `binary_benchmark.rs` byte-identical to main, so no baseline in it can move. A new fixture should not be able to shift an unrelated benchmark. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TeyUovZnNhUUR2bNhL3QhY

jlucaso1 · 2026-08-11T15:47:08Z

Correcting myself: I was wrong above. The runner did differ, but that is not what caused this — the regression is real, and it's mine. Fixed in 527d245.

Measured locally with callgrind, same binary, same machine, both built from profile.bench:

	Ir/iteration
`main` (`763aea9`)	8,605.4
this PR (`77bf9ab`)	8,906.3
after `527d245`	8,605.4

+300.9, which lines up with the +369 the wizard measured.

Cause. No library path got slower. The fan-out fixture I added instantiates the typed-JID attribute path (NodeBuilder::attr::<Jid>), and putting it in that crate root shifted inlining enough that create_attr_node's builder stopped pre-sizing its SmallVec and grew through an extra reallocation instead. The per-function diff is entirely that:

+261.7 Ir  <wacore_binary::node::Attrs>::insert::<Cow<str>, NodeValue>
+254.7 Ir  <SmallVec<[(Cow<str>, NodeValue); 3]>>::reserve_one_unchecked -> try_grow
+255.0 Ir  realloc / _int_realloc / _int_malloc
 +56.5 Ir  unlink_chunk

Attrs::insert is the owned node builder. bench_attr_parser reaches it only from create_attr_node() in its setup — never from the unmarshal_ref + attr-parse body it measures, which works on NodeRef/ValueRef. So this was the benchmark's own fixture getting more expensive, not the attribute parser.

Fix. The sweep moved to its own bench target, group_fanout_benchmark.rs. That puts bench_attr_parser back at exactly 8,605.4 and leaves binary_benchmark.rs byte-identical to main (git diff main -- wacore/binary/benches/binary_benchmark.rs is empty), so nothing in it can move. A new fixture shouldn't be able to shift an unrelated benchmark's baseline, and in its own crate root it can't.

The four bench_marshal_auto_group_fanout[N] entries change URI with the move. They were new in this PR, so no baseline is lost.

Generated by Claude Code

jlucaso1 · 2026-08-11T15:49:09Z

@codspeedbot's analysis is right and mine was wrong on the where. Correcting the record, since I posted a specific attribution above.

I said the extra ~300 Ir sat in the owned-builder path (Attrs::insert → SmallVec::try_grow → realloc), reached only from create_attr_node() in setup. The flamegraph shows it is the single malloc in Decoder::read_attributes — the measured body — taking the unlink_chunk path. What my per-function diff actually captured was allocator perturbation surfacing at whichever allocation sites ran; unlink_chunk appears in both readings, and that was the real signal. The conclusion is unchanged and I'll restate it plainly: nothing in wacore-binary changed, no code path got slower, and the cause was the 512 transient format!-built JID strings my first fan-out fixture allocated per iteration.

Both of the fixes for it are on the branch and they are independent:

77bf9ab replaced those strings with typed Jid values (a 13-char user is inline in CompactString, so the transient allocations are gone). That is what removed the regression, and it landed for a different reason — Greptile and Codex both flagged that device ids above 255 overflow parse_jid_meta's u8, so the 512-wide case was half-encoding as JID_PAIR.
527d245 moved the sweep to its own crate root, which is worth keeping precisely because of the caveat in that analysis — "any future bench added to binary_benchmark.rs can re-trigger it". binary_benchmark.rs is now byte-identical to main, so this fixture cannot perturb its heap or its codegen at all, whatever gets added to it later.

Generated by Claude Code

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 527d2454a8

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

…y use `Client::marshal_node_for_send` routes every outbound stanza through the two-pass `marshal_exact`; the sweep was calling one-pass `marshal_auto`. The two differ in exactly what this benchmark measures — one-pass reserves and grows, two-pass plans the size and replays a hint tape — so presenting the result as the encoder cost of a cold group send tracked a path no group send takes. `binary_benchmark` already pins `marshal_exact` as the production strategy for the same reason. Still linear, at the two-pass rate: 1.79 / 5.44 / 20.2 / 104.2 µs at widths 8 / 32 / 128 / 512, ~0.20 µs per recipient against 0.17 one-pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TeyUovZnNhUUR2bNhL3QhY

Dismissed because a newer commit was pushed; Greptile will re-review the current head.

github-actions · 2026-08-11T16:05:56Z

📦 Binary size report

Metric	main	PR
bin size (stripped)	10.04 MiB	10.04 MiB
bin .text	8.05 MiB	8.05 MiB
bin allocated (text+data+bss)	10.04 MiB	10.04 MiB
llvm-lines wacore	533,670	533,670
llvm-lines wacore copies	17,422	17,422
llvm-lines whatsapp-rust lib	762,401	762,401
llvm-lines whatsapp-rust lib copies	23,771	23,771
deps crates (Cargo.lock)	462	462

.text per crate

Crate	main	PR	Δ
.text whatsapp_rust	1.84 MiB	1.84 MiB	0
.text wacore	693.00 KiB	692.69 KiB	-312 B (-0.04%) 🔽
.text wacore_binary	81.61 KiB	81.61 KiB	0
.text wacore_libsignal	178.98 KiB	178.98 KiB	0
.text wacore_appstate	22.35 KiB	22.35 KiB	0
.text wacore_noise	20.94 KiB	20.94 KiB	0
.text waproto	1.74 MiB	1.74 MiB	0
.text whatsapp_rust_sqlite_storage	540.56 KiB	540.56 KiB	0
.text whatsapp_rust_tokio_transport	40.49 KiB	40.49 KiB	0
.text whatsapp_rust_ureq_http_client	12.68 KiB	12.68 KiB	0
.text std	995.62 KiB	995.62 KiB	0
.text other deps	1.90 MiB	1.90 MiB	+312 B (+0.02%) 🔺

Baseline: 5c9e4dc6e (latest main run) · Head: 792a507cb · Graphs

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e9a46cd797

ℹ️ 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 multi-device fixture passed the two own companions as `distribution_targets` while handing `ResolvedGroupDevices` only the group members, so `<participants>` named devices the `phash` did not cover — a shape no send produces. Production filters the SKDM targets out of that very set (`filter_skdm_targets` over `all_devices_for_phash`), and the server validates the phash against every recipient device, so the targets are always a subset of what is hashed. The companions now go into the resolved set the phash is computed over. None of the assertions move: the count still matches the companions, the phash is still fixed-width, and the encoded sizes still match at 8 and 512. What changes is that the fixture now stands for a stanza the server would accept. Re-ran 25× to confirm the padding-normalised comparison is stable. Also states what the fan-out sweep's payload models. `type="msg"` is the redistribution shape, for devices that already hold a pairwise session; first contact emits `type="pkmsg"`, whose PreKeySignalMessage adds an identity key, a base key and the registration id — roughly twice the bytes. Marshalling is linear in payload size, so that case rides the same slope from a higher intercept, and the per-recipient term this sweep exists to pin is unmoved. Building real ciphertexts is not available here in any case: wacore-binary does not depend on libsignal. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TeyUovZnNhUUR2bNhL3QhY

Dismissed because a newer commit was pushed; Greptile will re-review the current head.

The fixture doc was corrected to name the `type="msg"` redistribution shape, but the bench comment above it still opened with "cold group send" — the exact wording that was wrong. Both now agree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TeyUovZnNhUUR2bNhL3QhY

coderabbitai

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 `@wacore/src/send/tests.rs`:
- Around line 4065-4071: The stanza-shape helper and assertions in the relevant
test should compare the full node hierarchy recursively, including each node’s
tag and descendants, so changes inside <to> or <enc> are detected. Strengthen
the phash assertion to require the “2:” prefix and preserve the ten-character
length requirement.

🪄 Autofix

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 (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: ddd36af0-fe34-4ecc-8704-d5e40180ba16

📥 Commits

Reviewing files that changed from the base of the PR and between 2fef027 and 9c2ee8c.

📒 Files selected for processing (3)

wacore/binary/Cargo.toml
wacore/binary/benches/group_fanout_benchmark.rs
wacore/src/send/tests.rs

`child_tags` only looked at the root's direct children, so a `<to>` or `<enc>` subtree that grew with the group could slip past it — and a length-neutral change (a renamed attribute) would slip past the size comparison too. The shape is now walked recursively: tag, sorted attribute keys, then children. Attribute *keys* only, since the values legitimately differ — the phash digests two different device sets. Confirmed it bites by mutating a nested `<enc>` on the 512-member stanza alone: the old assertion passed, the new one fails with the full hierarchy in the message. The phash assertion accepted any ten-character value while the doc claimed "2:" plus 8 base64 chars. It now asserts the prefix as well, which is what makes it the phash the server expects rather than some other attribute that happens to be ten characters wide. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TeyUovZnNhUUR2bNhL3QhY

Dismissed because a newer commit was pushed; Greptile will re-review the current head.

coderabbitai

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

wacore/src/send/tests.rs (1)
4021-4037: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Verify the actual fan-out targets, not only the target count.

The request supplies distribution_targets from own_companions before prepare_group_stanza runs. This bypasses target-selection logic if that logic lives upstream. The assertion then checks only the number of <participants> children. A wrong list containing two group members passes.

If prepare_group_stanza is only the serializer, move this regression to the caller that computes distribution_targets. Otherwise, exercise the production selection path. In both cases, compare the emitted jid values with the expected companion JIDs.

Also applies to: 4084-4093
🤖 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 `@wacore/src/send/tests.rs` around lines 4021 - 4037, Update the regression
tests around prepare_group_stanza to exercise the production distribution-target
selection path instead of prepopulating distribution_targets from
own_companions. Assert the emitted participants’ jid values match the expected
companion JIDs, not merely the participant count; if prepare_group_stanza only
serializes targets, move the test to its caller where distribution_targets is
computed.

🤖 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 `@wacore/src/send/tests.rs`:
- Around line 4021-4037: Update the regression tests around prepare_group_stanza
to exercise the production distribution-target selection path instead of
prepopulating distribution_targets from own_companions. Assert the emitted
participants’ jid values match the expected companion JIDs, not merely the
participant count; if prepare_group_stanza only serializes targets, move the
test to its caller where distribution_targets is computed.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 82b71d91-5f1f-4712-9263-8aaf6cd9ee20

📥 Commits

Reviewing files that changed from the base of the PR and between 10d0b42 and b4b54e9.

📒 Files selected for processing (1)

wacore/src/send/tests.rs

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b4b54e9fe9

ℹ️ 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 fixture claimed the companions' SKDM encrypts to `msg`, but `process_prekey_bundle` alone leaves the session holding a pending pre-key, so it was emitting `pkmsg` — first contact, not the steady state the test is named for. Adding an `enc type` assertion proved it: `left: Some("pkmsg")`. Clearing the pending key is what the companion's reply does in production, so the fixture now does that too and the assertion holds it there. Also compares the `<to jid>` values against the expected companions rather than counting them, so a list of the right length addressing group members would fail — which is the regression this test exists to catch. Fixes a wrong claim in the fan-out sweep's doc as well. It said the larger `pkmsg` payload "rides the same slope from a higher intercept". It does not: `marshal_exact` copies every payload through the writer, so those bytes are paid once per recipient and raise the slope. The doc now says the sweep does not characterize a first-contact fan-out and must not be extrapolated to it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TeyUovZnNhUUR2bNhL3QhY

Dismissed because a newer commit was pushed; Greptile will re-review the current head.

coderabbitai Bot added the performance label Aug 11, 2026

coderabbitai Bot approved these changes Aug 11, 2026

View reviewed changes

greptile-apps Bot reviewed Aug 11, 2026

View reviewed changes

Comment thread wacore/src/send/tests.rs

chatgpt-codex-connector Bot reviewed Aug 11, 2026

View reviewed changes

greptile-apps Bot previously approved these changes Aug 11, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Aug 11, 2026

View reviewed changes

Comment thread wacore/binary/benches/group_fanout_benchmark.rs Outdated

greptile-apps Bot previously approved these changes Aug 11, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Aug 11, 2026

View reviewed changes

Comment thread wacore/binary/benches/group_fanout_benchmark.rs

Comment thread wacore/src/send/tests.rs Outdated

coderabbitai Bot requested changes Aug 11, 2026

View reviewed changes

Comment thread wacore/src/send/tests.rs Outdated

greptile-apps Bot previously approved these changes Aug 11, 2026

View reviewed changes

coderabbitai Bot reviewed Aug 11, 2026

View reviewed changes

coderabbitai Bot approved these changes Aug 11, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Aug 11, 2026

View reviewed changes

Comment thread wacore/src/send/tests.rs

Comment thread wacore/binary/benches/group_fanout_benchmark.rs Outdated

greptile-apps Bot approved these changes Aug 11, 2026

View reviewed changes

jlucaso1 merged commit 8f2beb7 into main Aug 11, 2026
26 of 27 checks passed

jlucaso1 deleted the claude/group-send-perf-analysis-if3q1x branch August 11, 2026 16:47

jlucaso1 mentioned this pull request Aug 11, 2026

bench(client): measure the group send the client crate actually pays #1283

Merged

Uh oh!

Uh oh!

Conversation

jlucaso1 commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Cost

CodSpeed against the local numbers

Checked and not changed

Validation

Uh oh!

coderabbitai Bot commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Walkthrough

Changes

Uh oh!

greptile-apps Bot commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

codspeed-hq Bot commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will improve performance by 99.95%

Performance Changes

Footnotes

Uh oh!

jlucaso1 commented Aug 11, 2026

Uh oh!

codspeed-hq Bot commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Where the 0.69 µs is

Reproduced locally

Why it is already fixed

Uh oh!

jlucaso1 commented Aug 11, 2026

Uh oh!

jlucaso1 commented Aug 11, 2026

Uh oh!

jlucaso1 commented Aug 11, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

github-actions Bot commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📦 Binary size report

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

jlucaso1 commented Aug 11, 2026 •

edited

Loading

coderabbitai Bot commented Aug 11, 2026 •

edited

Loading

greptile-apps Bot commented Aug 11, 2026 •

edited

Loading

codspeed-hq Bot commented Aug 11, 2026 •

edited

Loading

codspeed-hq Bot commented Aug 11, 2026 •

edited

Loading

github-actions Bot commented Aug 11, 2026 •

edited

Loading