perf(device-registry): build lookup keys with CompactString (inline, no heap) - #682
Merged
Conversation
…no heap) resolve_lookup_keys built UserLookupKeys from std::String, so the user-derived key (user.to_string()) heap-allocated per member on every group send even though LID/PN user parts are short (<=~18 chars). Switch UserLookupKeys to wacore_binary::CompactString (already the type of Jid.user): the user-derived key is now stored inline (no heap), and the cache-derived key moves in via the same type. dhat A/B (group-send, 800 members, 12 msgs), on top of #681: get_user_devices allocs 22996 -> 12538 (-45%; -75% vs pre-#681's 50915) total run 117245 -> 105762 blocks (-10%), 16.08 -> 15.91 MB all_keys() still yields &str (CompactString derefs), so consumers are unchanged. Behavior identical; allocation/CPU-churn only on the per-member-per-send path.
|
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 (1)
📝 WalkthroughSummary by CodeRabbit
Note: This release contains internal improvements with no visible changes to end-user functionality. WalkthroughThis PR optimizes user identifier storage in the device registry by migrating ChangesUser lookup key storage optimization
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Benchmark Results67 unchanged benchmark(s)
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Continuing the large-group send profiling (after #677/#680/#681). With the redundant key re-clone gone (#681), the next dhat hotspot in
get_user_devices(per-member device resolution, run on every group send) wasresolve_lookup_keysbuildingUserLookupKeysfromstd::String.Finding
resolve_lookup_keysdiduser.to_string()for the user-derived lookup key — a heap allocation per member per send, even though LID/PN user parts are short (≤~18 chars).UserLookupKeysusedStringfields throughout.Change
Switch
UserLookupKeystowacore_binary::CompactString(already the type ofJid.user). Short strings live inline, so the user-derived key no longer heap-allocates; the cache-derived key moves in via the same type.all_keys()still returnsVec<&str>(CompactString derefs tostr), so every consumer is unchanged.Measurement (dhat A/B, group-send, 800 members, 12 msgs)
get_user_devicesallocsget_user_devicesbytesCombined with #681, per-member device resolution went from 50915 → 12538 allocs (−75%).
Behavior is identical (same keys, same lookup order, same results) — allocation/CPU-churn only, on the per-member-per-send path that scales with group size × message rate.
Note on diminishing returns
This path is now close to its floor: the remaining churn is largely inherent (the actual
Vec<Jid>device data returned per member, and moka cache machinery for the device-registry and LID-PN caches). The bigger structural lever — caching a group's fully-resolved device set to make warm sends O(1) instead of O(members) — is not pursued: per-send client CPU is already low (~0.15 s for 40 sends to 800 members), the latency is server/network fanout-bound, and it matches WA Web's per-send O(N) iteration.Tests: clippy
--all-targets -D warningsclean; whatsapp-rust lib suite (660) + device_registry (36) green.