Skip to content

codegen: inline view field decoding#312

Merged
iainmcgin merged 1 commit into
anthropics:mainfrom
ogarciarevett:fix/inline-view-field-merge
Jul 16, 2026
Merged

codegen: inline view field decoding#312
iainmcgin merged 1 commit into
anthropics:mainfrom
ogarciarevett:fix/inline-view-field-merge

Conversation

@ogarciarevett

Copy link
Copy Markdown
Contributor

What this does

  • emits #[inline] on generated MessageView::merge_view_field implementations
  • regenerates the checked-in WKT and descriptor view sources
  • documents the same hot-path requirement for hand-written views
  • adds a codegen regression assertion and changelog fragment

Closes #298.

Why

The shared decode loop lives in buffa, while each generated field match lives in the downstream crate. Without an inline hint, the split introduced a per-field cross-crate call on zero-copy view decoding. The benchmark reported in #298 measured 4.47 s on 0.8.1 and 4.17 s with this hint, recovering most of the regression against 0.7.1.

RED → GREEN

Before the implementation, the focused test failed with:

merge_view_field must inline into the cross-crate decode loop

After emitting the attribute, the same test passes:

test tests::view_codegen::test_view_repeated_message_field ... ok

Verification

  • cargo fmt --all --check
  • cargo clippy --workspace --all-targets -- -D warnings
  • cargo build
  • cargo test --workspace
  • strict rustdoc: RUSTDOCFLAGS='-D warnings' cargo doc --workspace --all-features --no-deps
  • Markdown lint over the tracked tree
  • regenerated WKT and descriptor outputs twice; the resulting diff was stable
  • required Rust correctness/performance and API ergonomics reviews: no Critical, High, or Medium findings remain

No local performance benchmark was run; the timing above is the issue reporter's controlled measurement.

@github-actions

Copy link
Copy Markdown

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@iainmcgin iainmcgin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[claude code] Thanks for chasing this down and for the clean fix — the diagnosis in #298 was exactly right, and putting the attribute on the one concrete non-generic function in the eager view-decode path is the correct and sufficient boundary. Reviewed the change end to end: all 45 generated merge_view_field impls carry the attribute, #[inline] (hint) rather than inline(always) is the right call given the size of the DescriptorProto-scale match bodies, the codegen assertion catches a future removal, and lazy views correctly needed no change since their field match already lives inside a single generated function.

We also validated the fix independently on a dedicated bare-metal box (c7i.metal-24xl, criterion, main as baseline vs this branch), across the repo's decode_view benchmark suite:

dataset main this PR delta
google_message1_proto3 364.1 ns 246.0 ns −32.5%
mesh 5.446 ms 3.916 ms −28.1%
log_record 22.08 µs 17.84 µs −19.3%
api_response 6.453 µs 5.459 µs −15.7%
media_frame 12.02 µs 10.73 µs −10.6%
analytics_event 681.8 µs 618.5 µs −9.3%
packed_tile 1.306 ms 1.211 ms −7.3%
packed_signed 95.33 µs 95.00 µs −0.4%

The owned decode control rows (which this change does not touch) stayed within the box's ±5% floor, so the gains above are the real effect. packed_signed sitting at ~0% is the expected null — that shape is dominated by packed varint runs, where per-element decode work rather than per-field dispatch is the cost. So the improvement generalises well beyond the MVT workload in #298, and is larger on shapes with many small nested messages, where the per-field call boundary dominated most.

One note for anyone reproducing this: the bench crate's default [profile.bench] (lto = true, codegen-units = 1) hides the effect entirely, because fat LTO grants the cross-CGU inlining that this attribute exists to grant. The numbers above were measured with the downstream-realistic profile (lto = false, codegen-units = 16), which is the regime a normal consumer builds in and the one #298 was measured under.

Approving — thank you for the contribution.

@iainmcgin
iainmcgin added this pull request to the merge queue Jul 16, 2026
Merged via the queue into anthropics:main with commit 2748c9d Jul 16, 2026
9 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 16, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

View decode ~10% slower in 0.8.1: generated merge_view_field lacks #[inline]

2 participants