perf: avoid unnecessary clone and pre-allocate Vecs in hot paths by jlucaso1 · Pull Request #545 · oxidezap/whatsapp-rust

jlucaso1 · 2026-04-14T21:55:02Z

Summary

send.rs: borrow own_sending_jid.user instead of cloning the CompactString into the retain closure
usync.rs: use size_hint() upper bound to pre-allocate device Vec instead of growing from empty
message.rs: pre-allocate enc_nodes Vec with capacity 4 in the message decrypt path

Test plan

CI passes
Benchmark CI confirms no regressions

Summary by CodeRabbit

Performance
- Internal optimizations to message handling and device synchronization processes for improved efficiency.

- send.rs: borrow own_sending_jid.user instead of cloning CompactString into the retain closure - usync.rs: collect device nodes to get count, then pre-allocate Vec instead of growing from empty - message.rs: pre-allocate enc_nodes Vec with capacity 4 in the message decrypt path (typical messages have 1-2 enc nodes)

coderabbitai · 2026-04-14T21:55:17Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1b99ed9d-090d-404d-bd57-37daf544bc19

📥 Commits

Reviewing files that changed from the base of the PR and between b40a478 and abcd0de.

📒 Files selected for processing (1)

wacore/src/usync.rs

📝 Walkthrough

Walkthrough

Three micro-optimizations: preallocate vectors in message parsing and device parsing, and switch a local recipient user to borrowing in SKDM recipient filtering. No control-flow or public API changes.

Changes

Cohort / File(s)	Summary
Message parsing preallocation `src/message.rs`	`all_enc_nodes` is now created with `Vec::with_capacity(4)` instead of `Vec::new()`. No logic or control-flow changes.
Recipient filtering borrowing `wacore/src/send.rs`	In `prepare_group_stanza`, `own_sending_jid.user` is borrowed (`&own_sending_jid.user`) and compared via `device_jid.user == *own_user` instead of cloning into an owned `own_user`. Filtering predicates unchanged.
Device list preallocation `wacore/src/usync.rs`	`devices` is preallocated using `Vec::with_capacity(device_list_node.children().map_or(0,

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

perf: eliminate allocations in app state sync and pre-allocate collections #377 — Makes the same Vec::with_capacity(...) change in wacore/src/usync.rs (device preallocation).
feat: improve send group message #259 — Modifies SKDM recipient filtering in wacore/src/send.rs; overlaps with the borrowing/comparison adjustment here.
perf!: reduce hot-path allocations in receipt, send, and signal paths #519 — Targets allocation and clone-avoidance in send/encoding hot paths, touching similar areas as these micro-optimizations.

Poem

🐰 I nibble spare allocations light,

I borrow where a clone took flight.
Small hops, soft thumps — the heap breathes mild,
A thrift of bytes makes Rust code smile.
Carrots crunch: optimizations compiled. 🥕

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The pull request title accurately summarizes the main objectives: avoiding unnecessary clones and pre-allocating vectors in hot paths, which directly matches the three code changes (borrow instead of clone in send.rs, pre-allocate in usync.rs and message.rs).
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch perf/clone-cleanup-and-prealloc

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

jlucaso1 · 2026-04-14T21:59:34Z

@claude review

claude · 2026-04-14T21:59:49Z

Claude finished @jlucaso1's task in 1m 50s —— View job

Code Review: `perf: avoid unnecessary clone and pre-allocate Vecs in hot paths`

Read the diff and changed files
Analyze each change for correctness and improvements
Post review feedback

Verdict: LGTM — All three changes are correct and well-motivated. A couple of minor notes below.

`wacore/src/send.rs` — Borrow instead of clone ✅

-let own_user = own_sending_jid.user.clone();
+let own_user = &own_sending_jid.user;

Good catch. own_sending_jid lives across the retain call, so borrowing into the closure is strictly better than cloning the CompactString. The *own_user dereference in the comparison is correct since device_jid.user is also a CompactString and PartialEq<CompactString> for CompactString covers it.

`src/message.rs` — Pre-allocate `all_enc_nodes` with capacity 4 ✅

-let mut all_enc_nodes: Vec<&NodeRef<'_>> = Vec::new();
+let mut all_enc_nodes: Vec<&NodeRef<'_>> = Vec::with_capacity(4);

Reasonable heuristic. Direct enc children plus a possible participants > to > enc batch fits within 4 in the common case. Avoids 1–2 reallocations on the typical path with no downside — Vec::with_capacity(4) for a vec of references is only 32 bytes on 64-bit.

`wacore/src/usync.rs` — Collect iterator for exact capacity ✅

+let device_nodes: Vec<_> = device_list_node.get_children_by_tag("device").collect();
+let mut devices = Vec::with_capacity(device_nodes.len());

This materializes the iterator to get an exact count, then pre-allocates devices. The trade-off is an extra temporary Vec<&Node> allocation, but it pays for itself by eliminating potential reallocations in the devices vec and gives better cache locality during the subsequent loop. The benchmark confirms the net effect is positive (the -2.67% on group_recv is the most signal here since usync parsing is on the receive path).

One minor thought: get_children_by_tag returns a filter iterator over children(). An alternative would be device_list_node.get_children_by_tag("device").size_hint() — but since filter iterators don't provide useful upper bounds, the .collect() approach here is the right call.

Clean, low-risk PR. The benchmark methodology (iai-callgrind) gives stable instruction-count measurements which is the right tool for validating micro-optimizations like these. Ship it.

github-actions · 2026-04-14T22:01:07Z

Benchmark Results

1 regression(s) detected (>2% threshold):

Benchmark	Current	Baseline	Change
`send_receive_benchmark::group_recv::bench_group_recv text:setup_group_recv()`	12,612,065	12,351,223	+2.1%

58 unchanged benchmark(s)

Benchmark	Current	Baseline	Change
`reporting_token_benchmark::content_extraction_group::bench_content_extraction simple:setup_simple_message()`	3,882	3,882	+0.0%
`reporting_token_benchmark::content_extraction_group::bench_content_extraction extended:setup_extended_message()`	11,863	11,863	+0.0%
`reporting_token_benchmark::key_derivation_group::bench_key_derivation`	43,401	43,401	+0.0%
`reporting_token_benchmark::token_calculation_group::bench_token_calculation`	19,365	19,365	+0.0%
`reporting_token_benchmark::full_generation_group::bench_full_token_generation simple:setup_full_gen_simple()`	68,406	68,406	+0.0%
`reporting_token_benchmark::full_generation_group::bench_full_token_generation extended:setup_full_gen_extended()`	76,382	76,382	+0.0%
`reporting_token_benchmark::message_encoding_group::bench_message_encoding simple:setup_simple_message()`	2,217	2,217	+0.0%
`reporting_token_benchmark::message_encoding_group::bench_message_encoding extended:setup_extended_message()`	5,947	5,947	+0.0%
`send_receive_benchmark::dm_send::bench_dm_send text:setup_dm_send()`	169,925	170,348	-0.2%
`send_receive_benchmark::dm_recv::bench_dm_recv text:setup_dm_recv()`	191,785	191,645	+0.1%
`send_receive_benchmark::group_send::bench_group_send group_10:setup_group_send_10()`	875,423	874,753	+0.1%
`send_receive_benchmark::group_send::bench_group_send group_50:setup_group_send_50()`	966,115	966,096	+0.0%
`send_receive_benchmark::group_send::bench_group_send group_256:setup_group_send_256()`	1,451,298	1,451,306	-0.0%
`send_receive_benchmark::group_send_skdm::bench_group_send_skdm skdm_10:setup_group_skdm_10()`	2,576,888	2,583,228	-0.2%
`send_receive_benchmark::group_send_skdm::bench_group_send_skdm skdm_50:setup_group_skdm_50()`	9,378,766	9,411,859	-0.4%
`send_receive_benchmark::group_send_skdm::bench_group_send_skdm skdm_256:setup_group_skdm_256()`	44,641,399	44,642,651	-0.0%
`binary_benchmark::marshal_group::bench_marshal_allocating`	70,913	70,913	+0.0%
`binary_benchmark::marshal_group::bench_marshal_auto_allocating`	70,946	70,946	+0.0%
`binary_benchmark::marshal_group::bench_marshal_exact_allocating`	97,966	97,966	+0.0%
`binary_benchmark::marshal_group::bench_marshal_reusing_buffer`	77,676	77,676	+0.0%
`binary_benchmark::marshal_group::bench_marshal_reusing_buffer_vec_writer`	71,013	71,013	+0.0%
`binary_benchmark::marshal_group::bench_marshal_long_string`	7,499	7,499	+0.0%
`binary_benchmark::marshal_group::bench_marshal_auto_long_string`	7,543	7,543	+0.0%
`binary_benchmark::marshal_group::bench_marshal_exact_long_string`	9,251	9,251	+0.0%
`binary_benchmark::marshal_group::bench_marshal_huge_bytes_allocating`	530,504	530,504	+0.0%
`binary_benchmark::marshal_group::bench_marshal_auto_huge_bytes_allocating`	530,070	530,070	+0.0%
`binary_benchmark::marshal_group::bench_marshal_exact_huge_bytes_allocating`	531,431	531,431	+0.0%
`binary_benchmark::marshal_group::bench_marshal_many_children_allocating`	8,506,994	8,506,994	+0.0%
`binary_benchmark::marshal_group::bench_marshal_auto_many_children_allocating`	8,451,227	8,451,227	+0.0%
`binary_benchmark::marshal_group::bench_marshal_exact_many_children_allocating`	19,678,704	19,678,704	+0.0%
`binary_benchmark::unmarshal_group::bench_unmarshal small:setup_small_marshaled()`	2,498	2,498	+0.0%
`binary_benchmark::unmarshal_group::bench_unmarshal large:setup_large_marshaled()`	38,500	38,500	+0.0%
`binary_benchmark::unpack_group::bench_unpack_uncompressed`	785	785	+0.0%
`binary_benchmark::unpack_group::bench_unpack_compressed`	556,214	556,214	+0.0%
`binary_benchmark::attr_parser_group::bench_attr_parser attr_lookup:setup_attr_marshaled()`	5,031	5,031	+0.0%
`binary_benchmark::roundtrip_group::bench_roundtrip small:setup_small_marshaled()`	5,330	5,330	+0.0%
`binary_benchmark::roundtrip_group::bench_roundtrip large:setup_large_marshaled()`	66,219	66,219	+0.0%
`binary_benchmark::roundtrip_group::bench_roundtrip_auto small:setup_small_marshaled()`	5,357	5,357	+0.0%
`binary_benchmark::roundtrip_group::bench_roundtrip_auto large:setup_large_marshaled()`	66,255	66,255	+0.0%
`binary_benchmark::roundtrip_group::bench_roundtrip_exact small:setup_small_marshaled()`	6,722	6,722	+0.0%
`binary_benchmark::roundtrip_group::bench_roundtrip_exact large:setup_large_marshaled()`	89,618	89,618	+0.0%
`binary_benchmark::child_iteration_group::bench_get_children_by_tag`	479,370	479,370	+0.0%
`binary_benchmark::jid_optimization_group::bench_jid_to_owned_access jid_access:setup_jid_heavy_marshaled()`	11,612	11,612	+0.0%
`libsignal_benchmark::dm_group::bench_dm_session_establishment setup:setup_dm_users()`	17,461,873	17,282,898	+1.0%
`libsignal_benchmark::dm_group::bench_dm_encrypt_first_message first_msg:setup_dm_session()`	157,773	157,773	+0.0%
`libsignal_benchmark::dm_group::bench_dm_decrypt_first_message decrypt_prekey:setup_dm_with_first_message()`	5,511,012	5,511,012	+0.0%
`libsignal_benchmark::dm_group::bench_dm_encrypt_subsequent_message subsequent:setup_established_dm_session()`	158,602	158,602	+0.0%
`libsignal_benchmark::group_messaging_group::bench_group_create_distribution_message create:setup_group_sender()`	296,675	296,675	+0.0%
`libsignal_benchmark::group_messaging_group::bench_group_encrypt_message encrypt:setup_group_with_distribution()`	706,993	706,993	+0.0%
`libsignal_benchmark::group_messaging_group::bench_group_decrypt_message decrypt:setup_group_with_encrypted_message()`	12,541,271	12,573,585	-0.3%
`libsignal_benchmark::conversation_group::bench_full_dm_conversation full:setup_conversation_data()`	27,600,863	27,401,606	+0.7%
`libsignal_benchmark::signature_group::bench_signature_creation sign:setup_keypair_with_message()`	3,467,011	3,467,011	+0.0%
`libsignal_benchmark::signature_group::bench_signature_verification verify:setup_keypair_with_message()`	125,271,923	125,434,263	-0.1%
`libsignal_benchmark::signature_group::bench_key_generation keygen`	2,830,452	2,830,452	+0.0%
`libsignal_benchmark::session_optimization_group::bench_decrypt_with_previous_session previous_session:setup_with_archived_sessions()`	46,003	46,003	+0.0%
`libsignal_benchmark::session_optimization_group::bench_out_of_order_decryption out_of_order:setup_out_of_order_messages()`	5,089,072	5,089,072	+0.0%
`libsignal_benchmark::session_optimization_group::bench_promote_matching_session promote:setup_promote_matching_session()`	316,826	316,826	+0.0%
`libsignal_benchmark::session_optimization_group::bench_message_key_eviction eviction:setup_message_key_eviction()`	14,247,117	14,247,117	+0.0%

Avoids materializing the iterator into a temporary Vec just to get a count. Filter iterators propagate the parent's upper bound through size_hint(), which here is the total children count.

jlucaso1 · 2026-04-14T22:20:51Z

@claude do you have any idea why benchmark regreted? send_receive_benchmark::group_recv::bench_group_recv text:setup_group_recv()

claude · 2026-04-14T22:21:10Z

Claude finished @jlucaso1's task in 3m 55s —— View job

Investigating benchmark regression

Read the changed files and benchmark setup
Analyze the group_recv benchmark path
Explain the regression

Why `group_recv` regressed +2.1%

Short answer: The regression is compiler codegen noise, not an actual algorithmic regression. None of the code changed in this PR is on the group_recv benchmark's execution path.

The benchmark path (bench_group_recv at wacore/benches/send_receive_benchmark.rs:667) calls decrypt_group() which does:

make_sender_key_name() — builds a sender key name
group_decrypt() — libsignal sender-key decryption
MessageUtils::unpad_message_ref() — removes padding
wa::Message::decode() — protobuf decode

None of these touch send.rs, usync.rs, or message.rs.

Why it still shows a change: The benchmark lives in the wacore crate, and two of three changes (send.rs, usync.rs) are also in wacore. When the crate is recompiled, the compiler produces a different binary layout — function ordering, alignment padding, inlining decisions all shift. This changes instruction counts for unrelated functions due to instruction cache alignment effects. This is a well-known phenomenon with iai-callgrind.

Evidence it's noise:

decrypt_group() doesn't call any code in the changed files
The similar libsignal_benchmark::group_decrypt_message benchmark (which exercises a nearly identical libsignal decrypt path) shows only +0.4%
Other wacore benchmarks show 0.0% changes (smaller instruction counts are more stable)
+2.1% (~256K instructions on a 12.5M base) is right at the noise floor for codegen-sensitive benchmarks

Bonus finding: `size_hint()` in usync.rs is a no-op

One thing I noticed while investigating — the size_hint() approach in the second commit (b40a478) doesn't actually pre-allocate anything.

get_children_by_tag returns Option<&[Node]>.into_iter().flatten().filter(...). For Rust's std Flatten iterator, when the outer Option::IntoIter hasn't been consumed yet, size_hint() returns (0, None) — because Flatten can't know the inner slice's length without advancing the outer iterator. Then Filter passes through that None upper bound.

So device_iter.size_hint().1.unwrap_or(0) always evaluates to 0, making Vec::with_capacity(0) equivalent to Vec::new().

The original .collect() approach in the first commit actually did pre-allocate correctly (at the cost of a temporary Vec). If you want pre-allocation here, you could either:

Go back to the .collect() approach (allocates a temp vec but gives exact capacity)
Use device_list_node.children().map_or(0, |c| c.len()) as a cheap upper-bound estimate

Neither is critical for correctness — just flagging that the current code doesn't achieve its stated intent.

size_hint() on Flatten+Filter returns (0, None) — the previous approach was a no-op equivalent to Vec::new(). Use the total children count as an upper-bound estimate instead.

fix: use size_hint instead of collect for device Vec capacity

b40a478

Avoids materializing the iterator into a temporary Vec just to get a count. Filter iterators propagate the parent's upper bound through size_hint(), which here is the total children count.

fix: use children().len() for device Vec capacity

abcd0de

size_hint() on Flatten+Filter returns (0, None) — the previous approach was a no-op equivalent to Vec::new(). Use the total children count as an upper-bound estimate instead.

jlucaso1 merged commit 0797fe9 into main Apr 14, 2026
8 checks passed

jlucaso1 deleted the perf/clone-cleanup-and-prealloc branch April 14, 2026 22:55

This was referenced Jun 4, 2026

perf(groups): cache Arc<GroupInfo> to avoid deep-cloning group metadata on warm sends #710

Merged

perf(send): memoize the group phash on the device-list memo entry #840

Merged

Navigation Menu

Uh oh!

Uh oh!

perf: avoid unnecessary clone and pre-allocate Vecs in hot paths - #545

perf: avoid unnecessary clone and pre-allocate Vecs in hot paths#545
jlucaso1 merged 3 commits into
mainfrom
perf/clone-cleanup-and-prealloc

jlucaso1 commented Apr 14, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Apr 14, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

Uh oh!

jlucaso1 commented Apr 14, 2026

Uh oh!

claude Bot commented Apr 14, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Apr 14, 2026 •

edited

Loading

Uh oh!

jlucaso1 commented Apr 14, 2026

Uh oh!

claude Bot commented Apr 14, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

jlucaso1 commented Apr 14, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Summary by CodeRabbit

Uh oh!

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

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

Uh oh!

jlucaso1 commented Apr 14, 2026

Uh oh!

claude Bot commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review: perf: avoid unnecessary clone and pre-allocate Vecs in hot paths

wacore/src/send.rs — Borrow instead of clone ✅

src/message.rs — Pre-allocate all_enc_nodes with capacity 4 ✅

wacore/src/usync.rs — Collect iterator for exact capacity ✅

Uh oh!

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

Uh oh!

Benchmark Results

Uh oh!

jlucaso1 commented Apr 14, 2026

Uh oh!

claude Bot commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Investigating benchmark regression

Why group_recv regressed +2.1%

Bonus finding: size_hint() in usync.rs is a no-op

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

jlucaso1 commented Apr 14, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Apr 14, 2026 •

edited

Loading

claude Bot commented Apr 14, 2026 •

edited

Loading

Code Review: `perf: avoid unnecessary clone and pre-allocate Vecs in hot paths`

`wacore/src/send.rs` — Borrow instead of clone ✅

`src/message.rs` — Pre-allocate `all_enc_nodes` with capacity 4 ✅

`wacore/src/usync.rs` — Collect iterator for exact capacity ✅

github-actions Bot commented Apr 14, 2026 •

edited

Loading

claude Bot commented Apr 14, 2026 •

edited

Loading

Why `group_recv` regressed +2.1%

Bonus finding: `size_hint()` in usync.rs is a no-op