Skip to content

perf: zero-alloc Jid dedup and session lock helper - #500

Merged
jlucaso1 merged 1 commit into
mainfrom
perf/zero-alloc-dedup-and-session-lock-helper
Apr 7, 2026
Merged

perf: zero-alloc Jid dedup and session lock helper#500
jlucaso1 merged 1 commit into
mainfrom
perf/zero-alloc-dedup-and-session-lock-helper

Conversation

@jlucaso1

@jlucaso1 jlucaso1 commented Apr 7, 2026

Copy link
Copy Markdown
Collaborator

Summary

Zero-alloc Jid dedup

  • Replace HashSet + retain + .clone() with in-place sort_unstable_by + dedup_by
  • Eliminates N String allocations per group send (user + server cloned into HashSet)
  • New helpers in wacore::types::jid: sort_dedup_by_user() and sort_dedup_by_device()
  • Order change is safe: downstream resolve_devices is a batch query, order-independent

Session lock helper

  • Extract session_mutexes_for() on Client, replacing duplicated 8-line lock acquisition pattern in send.rs and signal.rs

Misc

  • Remove unnecessary to_jid.clone() in group stanza builder (last use of owned value)

Test plan

  • cargo fmt --all
  • cargo clippy --all --tests — zero warnings
  • cargo test --workspace --exclude e2e-tests — 1119 tests pass

Summary by CodeRabbit

  • Refactor
    • Streamlined session lock acquisition mechanism for device encryption handling, reducing code complexity.
    • Consolidated deduplication utilities for improved maintainability and consistency across session management operations.

- Add sort_dedup_by_user() and sort_dedup_by_device() to wacore JidExt,
  replacing HashSet+retain+clone with in-place sort+dedup (0 allocations
  vs N String clones per group send)
- Add session_mutexes_for() helper on Client, deduplicating the 8-line
  multi-lock acquisition pattern from send.rs and signal.rs
- Remove unnecessary to_jid.clone() in group stanza builder (last use)
@coderabbitai

coderabbitai Bot commented Apr 7, 2026

Copy link
Copy Markdown

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: 3412eaed-474e-44f1-a875-0c051f2065df

📥 Commits

Reviewing files that changed from the base of the PR and between fe12a54 and 3861a55.

📒 Files selected for processing (4)
  • src/features/signal.rs
  • src/send.rs
  • wacore/src/send.rs
  • wacore/src/types/jid.rs

📝 Walkthrough

Walkthrough

The PR refactors session lock and deduplication logic across multiple modules by introducing helper methods: session_mutexes_for centralizes per-device session mutex collection, while sort_dedup_by_user and sort_dedup_by_device replace ad-hoc deduplication patterns. This simplifies repetitive inline implementations previously scattered across the codebase.

Changes

Cohort / File(s) Summary
Session Lock Acquisition Refactoring
src/features/signal.rs, src/send.rs
Replaced manual per-key session lock loops with delegated calls to new session_mutexes_for helper method; added pub(crate) async fn session_mutexes_for to Client that collects and returns mutexes for a slice of device-encryption JIDs in order.
JID Utility Functions
wacore/src/types/jid.rs
Added two new exported sorting-and-deduplication utilities: sort_dedup_by_user (dedup by user/server pair) and sort_dedup_by_device (dedup by user/server/agent/device tuple); updated doc comments for write_protocol_address_to and cmp_for_lock_order.
Deduplication Pattern Consolidation
wacore/src/send.rs
Replaced ad-hoc HashSet-based deduplication of JID lists with calls to new utility functions sort_dedup_by_user and sort_dedup_by_device; removed unnecessary clone in stanza node construction.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Poem

🐰 A helper hops in, gathering locks with care,
No more string-building scattered here and there,
Sort and dedup now dance as one refrain,
Lock-key collection flows through a cleaner lane! 🔐✨

🚥 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 title 'perf: zero-alloc Jid dedup and session lock helper' directly summarizes the main changes: eliminating allocations in Jid deduplication and introducing a session lock helper method.
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/zero-alloc-dedup-and-session-lock-helper

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

jlucaso1 commented Apr 7, 2026

Copy link
Copy Markdown
Collaborator Author

@claude review

@claude

claude Bot commented Apr 7, 2026

Copy link
Copy Markdown

Claude finished @jlucaso1's task in 52s —— View job


Code Review: perf: zero-alloc Jid dedup and session lock helper

  • Read changed files and PR diff
  • Review zero-alloc Jid dedup helpers
  • Review session lock helper extraction
  • Review misc changes
  • Post review feedback

Clean PR — good dedup optimization and nice extraction of the duplicated lock pattern. A few observations:

sort_dedup_by_user / sort_dedup_by_device — looks correct

The sort_unstable_by + dedup_by approach is a solid zero-alloc replacement for the HashSet + retain + .clone() pattern. The behavioral change from unordered-retain to sorted-dedup is safe since resolve_devices is a batch query (order-independent), as noted in the PR description.

One nit: sort_dedup_by_user (wacore/src/types/jid.rs:35) sorts by user then server, but cmp_for_lock_order (line 27) sorts by server then user. This is fine since they serve different purposes (dedup vs lock ordering), but worth being aware of if anyone later tries to unify them.

dedup_by argument order

dedup_by passes (a, b) where a is the later element and b is the earlier one (consecutive duplicates, a is removed). The equality predicates here are symmetric so this is correct, but it's a common footgun worth noting — if these ever became asymmetric (e.g., merging fields from a into b), the argument order matters.

session_mutexes_for — good extraction

The helper at src/send.rs:1496-1507 cleanly deduplicates the 8-line pattern from both send.rs and signal.rs. The buf reuse across iterations avoids per-iteration allocation, which is nice.

Minor: The method assumes jids are already sorted (from build_session_lock_keys). This implicit precondition could silently break if someone calls session_mutexes_for with unsorted jids. Consider either:

  • A debug assertion: debug_assert!(jids.windows(2).all(|w| cmp_for_lock_order(&w[0], &w[1]).is_le()))
  • Or documenting the precondition in the doc comment (e.g., /// Jids must be pre-sorted via build_session_lock_keys``)

This is low-risk since both call sites already go through build_session_lock_keys, but a debug_assert would be a nice safety net.

to_jid.clone() removal

wacore/src/send.rs:1210 — removing the unnecessary .clone() on the last use of to_jid is correct. Good catch.

Unused HashSet import?

wacore/src/send.rs:15 still imports HashSet, which is still used elsewhere in the file (lines 1245, 1246, 1323), so the import is correctly retained.

Overall

LGTM — the changes are well-scoped, the allocation savings are real (N clones per group send eliminated), and the session lock helper reduces duplication nicely. The only suggestion is the optional debug_assert for the sort precondition on session_mutexes_for.


@github-actions

github-actions Bot commented Apr 7, 2026

Copy link
Copy Markdown

🐰 Bencher Report

Branchperf/zero-alloc-dedup-and-session-lock-helper
Testbedubuntu-latest

🚨 1 Alert

BenchmarkMeasure
Units
ViewBenchmark Result
(Result Δ%)
Upper Boundary
(Limit %)
libsignal_benchmark::session_optimization_group::bench_decrypt_with_previous_session previous_session:setup_with_archived_sessions()Instructions
instructions x 1e3
📈 plot
🚷 threshold
🚨 alert (🔔)
47.03 x 1e3
(+7.45%)Baseline: 43.77 x 1e3
45.96 x 1e3
(102.33%)

Click to view all benchmark results
BenchmarkInstructionsBenchmark Result
instructions
(Result Δ%)
Upper Boundary
instructions
(Limit %)
binary_benchmark::attr_parser_group::bench_attr_parser attr_lookup:setup_attr_marshaled()📈 view plot
🚷 view threshold
6,199.00
(-2.56%)Baseline: 6,361.59
6,679.67
(92.80%)
binary_benchmark::child_iteration_group::bench_get_children_by_tag📈 view plot
🚷 view threshold
523,708.00
(-23.75%)Baseline: 686,844.88
721,187.13
(72.62%)
binary_benchmark::jid_optimization_group::bench_jid_to_owned_access jid_access:setup_jid_heavy_marshaled()📈 view plot
🚷 view threshold
20,910.00
(-4.59%)Baseline: 21,915.47
23,011.24
(90.87%)
binary_benchmark::marshal_group::bench_marshal_allocating📈 view plot
🚷 view threshold
98,703.00
(-10.91%)Baseline: 110,787.77
116,327.15
(84.85%)
binary_benchmark::marshal_group::bench_marshal_auto_allocating📈 view plot
🚷 view threshold
98,731.00
(-7.91%)Baseline: 107,212.34
112,572.96
(87.70%)
binary_benchmark::marshal_group::bench_marshal_auto_huge_bytes_allocating📈 view plot
🚷 view threshold
533,017.00
(-0.08%)Baseline: 533,418.00
560,088.90
(95.17%)
binary_benchmark::marshal_group::bench_marshal_auto_long_string📈 view plot
🚷 view threshold
15,955.00
(-3.36%)Baseline: 16,509.80
17,335.29
(92.04%)
binary_benchmark::marshal_group::bench_marshal_auto_many_children_allocating📈 view plot
🚷 view threshold
14,813,622.00
(-5.97%)Baseline: 15,754,914.66
16,542,660.39
(89.55%)
binary_benchmark::marshal_group::bench_marshal_exact_allocating📈 view plot
🚷 view threshold
118,631.00
(-17.17%)Baseline: 143,230.65
150,392.18
(78.88%)
binary_benchmark::marshal_group::bench_marshal_exact_huge_bytes_allocating📈 view plot
🚷 view threshold
534,447.00
(-0.07%)Baseline: 534,840.60
561,582.63
(95.17%)
binary_benchmark::marshal_group::bench_marshal_exact_long_string📈 view plot
🚷 view threshold
18,004.00
(-3.00%)Baseline: 18,560.67
19,488.70
(92.38%)
binary_benchmark::marshal_group::bench_marshal_exact_many_children_allocating📈 view plot
🚷 view threshold
28,200,726.00
(-17.88%)Baseline: 34,342,321.52
36,059,437.60
(78.21%)
binary_benchmark::marshal_group::bench_marshal_huge_bytes_allocating📈 view plot
🚷 view threshold
533,456.00
(-0.08%)Baseline: 533,857.00
560,549.85
(95.17%)
binary_benchmark::marshal_group::bench_marshal_long_string📈 view plot
🚷 view threshold
15,928.00
(-5.76%)Baseline: 16,901.34
17,746.41
(89.75%)
binary_benchmark::marshal_group::bench_marshal_many_children_allocating📈 view plot
🚷 view threshold
14,815,122.00
(-5.97%)Baseline: 15,756,097.29
16,543,902.16
(89.55%)
binary_benchmark::marshal_group::bench_marshal_reusing_buffer📈 view plot
🚷 view threshold
108,446.00
(-8.71%)Baseline: 118,788.14
124,727.55
(86.95%)
binary_benchmark::marshal_group::bench_marshal_reusing_buffer_vec_writer📈 view plot
🚷 view threshold
98,803.00
(-7.91%)Baseline: 107,284.34
112,648.56
(87.71%)
binary_benchmark::roundtrip_group::bench_roundtrip large:setup_large_marshaled()📈 view plot
🚷 view threshold
91,558.00
(-3.58%)Baseline: 94,955.65
99,703.43
(91.83%)
binary_benchmark::roundtrip_group::bench_roundtrip small:setup_small_marshaled()📈 view plot
🚷 view threshold
7,431.00
(-1.84%)Baseline: 7,570.00
7,948.50
(93.49%)
binary_benchmark::roundtrip_group::bench_roundtrip_auto large:setup_large_marshaled()📈 view plot
🚷 view threshold
91,589.00
(-0.85%)Baseline: 92,377.83
96,996.72
(94.42%)
binary_benchmark::roundtrip_group::bench_roundtrip_auto small:setup_small_marshaled()📈 view plot
🚷 view threshold
7,454.00
(+0.98%)Baseline: 7,381.93
7,751.03
(96.17%)
binary_benchmark::roundtrip_group::bench_roundtrip_exact large:setup_large_marshaled()📈 view plot
🚷 view threshold
107,134.00
(-0.94%)Baseline: 108,151.35
113,558.92
(94.34%)
binary_benchmark::roundtrip_group::bench_roundtrip_exact small:setup_small_marshaled()📈 view plot
🚷 view threshold
8,966.00
(+0.81%)Baseline: 8,893.93
9,338.63
(96.01%)
binary_benchmark::unmarshal_group::bench_unmarshal large:setup_large_marshaled()📈 view plot
🚷 view threshold
41,989.00
(-5.57%)Baseline: 44,466.08
46,689.38
(89.93%)
binary_benchmark::unmarshal_group::bench_unmarshal small:setup_small_marshaled()📈 view plot
🚷 view threshold
2,716.00
(-1.47%)Baseline: 2,756.54
2,894.37
(93.84%)
binary_benchmark::unpack_group::bench_unpack_compressed📈 view plot
🚷 view threshold
556,090.00
(-0.01%)Baseline: 556,137.16
583,944.01
(95.23%)
binary_benchmark::unpack_group::bench_unpack_uncompressed📈 view plot
🚷 view threshold
773.00
(+0.10%)Baseline: 772.20
810.81
(95.34%)
libsignal_benchmark::conversation_group::bench_full_dm_conversation full:setup_conversation_data()📈 view plot
🚷 view threshold
27,434,615.00
(-0.93%)Baseline: 27,693,099.09
29,077,754.04
(94.35%)
libsignal_benchmark::dm_group::bench_dm_decrypt_first_message decrypt_prekey:setup_dm_with_first_message()📈 view plot
🚷 view threshold
5,512,366.00
(-0.61%)Baseline: 5,546,120.80
5,823,426.84
(94.66%)
libsignal_benchmark::dm_group::bench_dm_encrypt_first_message first_msg:setup_dm_session()📈 view plot
🚷 view threshold
162,318.00
(-8.28%)Baseline: 176,970.04
185,818.54
(87.35%)
libsignal_benchmark::dm_group::bench_dm_encrypt_subsequent_message subsequent:setup_established_dm_session()📈 view plot
🚷 view threshold
163,047.00
(-8.26%)Baseline: 177,727.87
186,614.26
(87.37%)
libsignal_benchmark::dm_group::bench_dm_session_establishment setup:setup_dm_users()📈 view plot
🚷 view threshold
17,232,286.00
(-0.27%)Baseline: 17,279,475.04
18,143,448.79
(94.98%)
libsignal_benchmark::group_messaging_group::bench_group_create_distribution_message create:setup_group_sender()📈 view plot
🚷 view threshold
298,493.00
(+0.48%)Baseline: 297,055.10
311,907.85
(95.70%)
libsignal_benchmark::group_messaging_group::bench_group_decrypt_message decrypt:setup_group_with_encrypted_message()📈 view plot
🚷 view threshold
12,622,794.00
(+0.22%)Baseline: 12,595,526.63
13,225,302.96
(95.44%)
libsignal_benchmark::group_messaging_group::bench_group_encrypt_message encrypt:setup_group_with_distribution()📈 view plot
🚷 view threshold
713,228.00
(-0.56%)Baseline: 717,228.94
753,090.38
(94.71%)
libsignal_benchmark::session_optimization_group::bench_decrypt_with_previous_session previous_session:setup_with_archived_sessions()📈 view plot
🚷 view threshold
🚨 view alert (🔔)
47,033.00
(+7.45%)Baseline: 43,773.92
45,962.61
(102.33%)

libsignal_benchmark::session_optimization_group::bench_message_key_eviction eviction:setup_message_key_eviction()📈 view plot
🚷 view threshold
14,247,117.00
(-8.38%)Baseline: 15,550,867.85
16,328,411.24
(87.25%)
libsignal_benchmark::session_optimization_group::bench_out_of_order_decryption out_of_order:setup_out_of_order_messages()📈 view plot
🚷 view threshold
5,116,228.00
(-6.34%)Baseline: 5,462,485.05
5,735,609.30
(89.20%)
libsignal_benchmark::session_optimization_group::bench_promote_matching_session promote:setup_promote_matching_session()📈 view plot
🚷 view threshold
298,844.00
(-59.55%)Baseline: 738,709.26
775,644.72
(38.53%)
libsignal_benchmark::signature_group::bench_key_generation keygen📈 view plot
🚷 view threshold
2,830,495.00
(+0.15%)Baseline: 2,826,196.61
2,967,506.44
(95.38%)
libsignal_benchmark::signature_group::bench_signature_creation sign:setup_keypair_with_message()📈 view plot
🚷 view threshold
3,467,011.00
(-0.05%)Baseline: 3,468,670.99
3,642,104.54
(95.19%)
libsignal_benchmark::signature_group::bench_signature_verification verify:setup_keypair_with_message()📈 view plot
🚷 view threshold
126,598,813.00
(+0.98%)Baseline: 125,370,956.33
131,639,504.15
(96.17%)
reporting_token_benchmark::content_extraction_group::bench_content_extraction extended:setup_extended_message()📈 view plot
🚷 view threshold
11,851.00
(+0.07%)Baseline: 11,842.39
12,434.51
(95.31%)
reporting_token_benchmark::content_extraction_group::bench_content_extraction simple:setup_simple_message()📈 view plot
🚷 view threshold
3,879.00
(+0.84%)Baseline: 3,846.88
4,039.22
(96.03%)
reporting_token_benchmark::full_generation_group::bench_full_token_generation extended:setup_full_gen_extended()📈 view plot
🚷 view threshold
77,193.00
(-11.84%)Baseline: 87,556.11
91,933.91
(83.97%)
reporting_token_benchmark::full_generation_group::bench_full_token_generation simple:setup_full_gen_simple()📈 view plot
🚷 view threshold
69,103.00
(-13.17%)Baseline: 79,581.49
83,560.57
(82.70%)
reporting_token_benchmark::key_derivation_group::bench_key_derivation📈 view plot
🚷 view threshold
43,398.00
(-14.53%)Baseline: 50,773.33
53,312.00
(81.40%)
reporting_token_benchmark::message_encoding_group::bench_message_encoding extended:setup_extended_message()📈 view plot
🚷 view threshold
5,939.00
(+2.33%)Baseline: 5,803.66
6,093.84
(97.46%)
reporting_token_benchmark::message_encoding_group::bench_message_encoding simple:setup_simple_message()📈 view plot
🚷 view threshold
2,214.00
(+3.17%)Baseline: 2,145.90
2,253.19
(98.26%)
reporting_token_benchmark::token_calculation_group::bench_token_calculation📈 view plot
🚷 view threshold
19,365.00
(-11.58%)Baseline: 21,900.26
22,995.27
(84.21%)
🐰 View full continuous benchmarking report in Bencher

@jlucaso1
jlucaso1 merged commit b56be2e into main Apr 7, 2026
8 checks passed
@jlucaso1
jlucaso1 deleted the perf/zero-alloc-dedup-and-session-lock-helper branch April 7, 2026 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant