Iterate presence bits in clear(), writeTo() and getSerializedSize() for union-like messages - #13
Merged
Merged
Conversation
Union-like messages declare many singular message fields but only ever set one or two: Pulsar's BaseCommand has ~60, and its clear() walked a sequential `if (hasX()) x.clear()` guard for every one of them on every parse and every fill cycle. Flame graphs showed the guard chain as ~5-7% of self time, and removing it measured far larger — sixty correlated branches per clear() cost more in branch-prediction pressure than their profile share suggested. For messages with at least 4 singular message fields, clear() now iterates the set bits of (_bitFieldN & _MSG_FIELDS_MASKN) with a numberOfTrailingZeros loop and a dense switch on the field index, clearing only the children that are actually present: O(set message fields) instead of O(declared fields). Messages below the threshold keep the plain per-field guards and generate byte-identical code. Throughput (JMH, Apple M-series, ops/us, 2-3 forks): BaseCommand deserialize 36.1 -> 48.1 (+33%) BaseCommand serialize 25.4 -> 29.7 (+17%) MessageMetadata ser/deser unchanged (no singular message fields)
There was a problem hiding this comment.
Pull request overview
This PR optimizes generated clear() implementations for “union-like” messages that declare many singular (non-oneof) message fields but usually set only a few, by emitting a bit-driven loop that clears only the message fields whose presence bits are set.
Changes:
- Add a generator heuristic (
BIT_DRIVEN_CLEAR_THRESHOLD = 4) to choose between per-fieldif (hasX()) x.clear()and a set-bit iteration strategy. - Emit bit-driven
clear()code that iterates(_bitFieldN & _MSG_FIELDS_MASKN)and clears only present singular message fields via aswitchon the bit index. - Generate per-bitfield-word
_MSG_FIELDS_MASKNconstants to restrict the set-bit loop to singular message fields only.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Same principle as the clear() change: union-like messages set only one or two of their many fields, yet serialization and size computation walked every field's has() guard. For messages where all fields are singular and non-oneof (so every field carries a presence bit), _writeTo() and getSerializedSize() now iterate the set bits per word. Set bits ascend, so fields are emitted in declaration order and the output bytes are identical to the guard walk; proto3 implicit-presence fields keep their non-default check inside the case, and required fields always have their bit set once checkRequiredFields() has passed. Messages with repeated, map or oneof fields keep the guard walk (ordering across unbitted fields could not be preserved otherwise), as do messages below an 8-field threshold. Throughput (JMH, Apple M-series, ops/us, 2 forks, same-session baseline on top of the bit-driven clear() commit): BaseCommand serialize 28.8 -> 43.4 (+50%) BaseCommand deserialize unchanged (52.3 +- 1.9) MessageMetadata ser/deser unchanged (controls)
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
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.
Motivation
Union-like messages declare many singular message fields but only ever set one or two. Pulsar's
BaseCommandhas 52 singular fields, and three hot methods walked a guard per declared field on every cycle:clear()(runs at the start of everyparseFrom()and every fill): 60 sequentialif (hasX()) x.clear()guards_writeTo(): ahas()guard per fieldgetSerializedSize(): ahas()guard per fieldFresh flame graphs (post-#11/#12) surfaced the
clear()guard chain at ~5–7% of self time; removing all three walks measured far larger than the profile share — dozens of correlated branches per call cost more in branch-prediction pressure than their sample counts suggest.Changes
clear()(all message shapes with ≥ 4 singular message fields): iterate the set bits of_bitFieldN & _MSG_FIELDS_MASKNwith anInteger.numberOfTrailingZerosloop and a denseswitch, clearing only the children actually present — O(set fields) instead of O(declared fields). The presence bit implies the child was allocated, the same invariant the guarded form relied on._writeTo()andgetSerializedSize()(messages where every field is singular and non-oneof, ≥ 8 fields): the same set-bit traversal. Set bits ascend, so fields are emitted in declaration order and the output bytes are identical to the guard walk — verified by the round-trip tests against protobuf-java. proto3 implicit-presence fields keep their non-default check inside the case; required fields always have their bit set oncecheckRequiredFields()has passed. The two methods share the traversal emitter, so they agree exactly on which fields contribute (writeTo fills an array sized by getSerializedSize).Messages with repeated, map or oneof fields keep the guard walk for serialization (those fields carry no presence bits, so ordering across them could not be preserved), and small messages generate byte-identical code to before —
MessageMetadata,Frameetc. are unaffected and were verified flat as benchmark controls.Results
JMH, Apple M-series, ops/µs, same-session baselines, 2–3 forks:
(Per-commit split: the clear() change contributes +17% ser / +33% deser; the writeTo/size change adds a further +50% ser.)
BaseCommand vs protobuf-java: serialize 1.5× → 2.6×, deserialize 2.8× → 4.1×.
Verification
Full test suite green — including byte-identical round-trips against protobuf-java (which pins the serialization order) and the instance-reuse suite from #11 exercising clear/reparse cycles on pooled instances.