bench: cover the receive path — plaintext decode and appstate index-MAC dedup - #856
Conversation
…AC dedup Two gaps on the inbound side, benched before any optimization: - decode_plaintext (unpad + prost decode): runs on every received message and is the inbound mirror of the already-benched encode_and_pad. Shapes match the send-side bench plus an inline-SKDM group first-message. - collect_unique_index_macs: the O(N²) linear-scan dedup feeding the batched previous-value-MAC lookup, extracted from its two inline copies (inbound process_patch_list and outbound build_patch) into a pure function so it can be measured. At the 1000-mutation resume-sync upper bound the scan takes ~1.5ms; a HashSet swap measured 6-120% slower at small N in this codebase, so both ends are pinned (N=10 and N=1000) before deciding. No behavior change: the extraction is byte-identical logic. https://claude.ai/code/session_01EoJjbyorpCARCBTZSmUMRr
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR extracts duplicate index MAC collection logic from two patch-processing sites into a single reusable helper, ChangesAppstate sync extraction and benchmarking
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Merging this PR will not alter performance
Performance Changes
Comparing |
…detection `detect_duplicate_index_in_patch` deduped index_macs with two `HashSet<&[u8]>`. The keys are HMAC outputs (uniformly random), so SipHash buys no distribution benefit over a trivial compare — it only costs the per-key hash setup and a table allocation, which `bench_process_patch_50_validated` showed via `RandomState::hash_one` + `fallible_with_capacity`. Swap both sets for linear-scan `Vec<&[u8]>`, the same trade-off already validated for the sibling `collect_unique_index_macs` (#856), where HashSet measured 6-120% worse at the patch sizes seen in practice. Set and Remove stay deduped independently, so the existing semantics (`process_patch_rejects_duplicate_set_index` and `process_patch_allows_same_index_across_set_and_remove`) are unchanged. Zero new dependencies (no fast-hash crate), matching the recent dependency trimming. CI/CodSpeed measure the delta on bench_process_patch_50_validated. https://claude.ai/code/session_01XHsbPwjaCRDHDL69HbEgR8
Bench-only PR (same flow as #854): map the baseline with CodSpeed before touching any code. Two inbound-side coverage gaps:
1.
decode_plaintext— unpad + prost decode (wacore/src/messages.rs)Runs on every received message and is the inbound mirror of the already-benched
encode_and_pad, closing the send/receive coverage asymmetry. It's also where the #853 submessage boxing lands on the receive side, so the flamegraph doubles as a check on that.Shapes match the send-side bench (
text_reply,media_refs,large_text) plusgroup_skdm_text— the inline-SKDM group first-message.Local numbers (median):
text_reply591 ns,large_text644 ns,media_refs1.11 µs,group_skdm_text500 ns.2. App-state index-MAC dedup — O(N²) scan (
wacore/src/appstate_sync.rs)The linear-scan dedup feeding the batched previous-value-MAC lookup, extracted from its two inline copies (inbound
process_patch_listand outboundbuild_patch) into a purecollect_unique_index_macsso it can be measured. Byte-identical logic, no behavior change.This is the measurement the known HashSet fix is blocked on: HashSet-vs-scan measured 6–120% worse at small N in this project, so both ends are pinned — N=10 (typical incremental patch) and N=1000 (resume-sync upper bound).
Local numbers (median): N=10 → 288 ns, N=1000 → 1.48 ms. The quadratic blowup is real at the upper bound (~500k
Vec<u8>compares per resume patch); whether the fix pays without regressing small N is now CodSpeed's call.Verification
cargo fmt/cargo clippy -p wacore --benches --testsclean (only the pre-existing clippy.toml chrono warning)cargo test -p wacore: 987 + auxiliary suites, all greencargo check --allpasseshttps://claude.ai/code/session_01EoJjbyorpCARCBTZSmUMRr
Generated by Claude Code