perf(waproto): pin the Message codec to one instantiation via non-generic helpers - #842
Merged
Conversation
…eric helpers prost's Message methods are generic, so rustc instantiates them in every crate that calls them; the per-crate symbols carry distinct instantiating-crate hashes that LTO cannot merge, and the release binary shipped three full copies of Message::encode_raw (~160 KiB each, instantiated by wacore, the lib and the consuming bin crate) plus duplicated slices of the decode tree, including a second buffer-type instantiation for callers that decoded from a by-value &[u8]. waproto::codec adds #[inline(never)] non-generic entry points for the hot roots (Message encode/decode, WebMessageInfo and HistorySync decode, MessageContextInfo encode) and every production call site routes through them, so the whole tree is codegen'd once in the defining crate and decode uses a single buffer shape. Measured on the release bin (fat LTO, cgu=1): .text 13.03 MiB to 11.85 MiB (-1208 KiB, -9.1%); Message::encode_raw from 3 copies to 1; cross-crate duplicate-symbol waste from 2528 KiB to 1480 KiB.
|
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
WalkthroughThis PR adds a new ChangesProtobuf codec abstraction layer migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
(Note: I expect this to land cleanly if encoding/decoding behavior is identical; verify decode error paths and test coverage.) 🚥 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 |
Contributor
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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/messages.rs`:
- Around line 50-53: Replace the hardcoded protobuf field tag constants (e.g.
TAG_MESSAGE_CONTEXT_INFO) used in manual wire framing calls (such as
len_delimited_len(...) calling waproto::codec::message_context_info_encoded_len)
with the generated tag constants from the schema (use the constants in the
waproto::tags module, e.g. waproto::tags::TAG_MESSAGE_CONTEXT_INFO); apply the
same substitution for the other occurrences called out (the usages at the other
ranges) so the manual framing uses the canonical generated tags, or
alternatively add compile-time asserts that the local TAG_* values equal
waproto::tags::* to prevent silent renumbering. Ensure you update all uses
(recipient/DSM helpers) referencing TAG_* in this file to reference
waproto::tags::* or add the asserts.
🪄 Autofix (Beta)
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
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d19df05c-d430-4964-8065-fe1b9da7b28b
📒 Files selected for processing (12)
src/client/sender_keys.rssrc/features/newsletter.rssrc/message/msg_secret.rssrc/message/special.rssrc/pdo.rssrc/send.rswacore/src/comment.rswacore/src/message_edit.rswacore/src/messages.rswacore/src/reporting_token.rswacore/src/types/events.rswaproto/src/lib.rs
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
…a constants The hand-written recipient/DSM framing hardcoded the four field numbers it splices. Deriving them from waproto::tags turns a .proto renumber into a compile error here instead of a silently changed wire payload; the splice_* differential tests keep pinning the framing itself against prost.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This was referenced Jun 11, 2026
This was referenced Jun 14, 2026
7 tasks
This was referenced Jul 19, 2026
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.
Problem
A cargo-bloat audit of the release binary (fat LTO, codegen-units=1) found that 2.5 MiB of the 13.1 MiB
.textsection is duplicated monomorphization: the same generic function compiled in two or three crates. prost'sMessagemethods are generic, rustc instantiates them in whichever crate calls them, and the per-crate symbols carry distinct instantiating-crate hashes, so even fat LTO cannot merge them. The defining crate (waproto) instantiated about 1 KiB of all that codegen; wacore, the lib and the consuming bin crate paid for the rest.The worst case was
whatsapp::Message::encode_raw::<Vec<u8>>: three full copies at ~160 KiB each. The decode side additionally split by buffer type, with most call sites instantiating the&mut &[u8]tree and a few by-value&[u8]callers dragging in a second one (BotMetadata::merge_field::<&[u8]>alone was 65 KiB of pure duplication).This affects every consumer the same way: their binary gets its own copies on top of the lib's.
Change
New
waproto::codecmodule with#[inline(never)]non-generic entry points for the hot proto roots:wa::Message:message_encoded_len,message_encode_into,message_to_vec,message_decodewa::WebMessageInfoandwa::HistorySync: pinned decodewa::MessageContextInfo: pinned encode (it is spliced manually on the DM/group send paths)Every production call site now routes through them (send paths in
wacore::messagesincluding the pre-sized splice buffers, receive/edit/comment/msg-secret/sender-keys/newsletter/PDO decode, reporting token, the lazy history-sync accessor). The whole encode and decode tree is codegen'd exactly once, in the defining crate, and decode uses a single buffer shape (&mut &[u8], the one the workspace already instantiated everywhere).#[inline(never)]keeps MIR inlining from re-expanding the bodies at call sites, which would silently reintroduce the per-crate copies. Test-only call sites were left untouched.Measured
Release bin, same flags as the shipping profile (strip disabled only to read symbols):
.text: 13.03 MiB -> 11.85 MiB (-1208 KiB, -9.1%)Message::encode_raw::<Vec<u8>>: 3 copies -> 1.NNNclones excluded): 2528 KiB -> 1480 KiBRuntime cost is one direct call into a function that encodes or decodes the full message tree (micro vs. the body itself); the CodSpeed run on this PR is the neutrality check.
Tests
No behavior change intended; the full wacore + lib suites pass (1838 tests). The codec helpers are exercised by every existing encode/decode test through the converted call sites.