buffa-types: back Any.value with bytes::Bytes for refcount-bump clone#51
buffa-types: back Any.value with bytes::Bytes for refcount-bump clone#51iainmcgin merged 3 commits intoanthropics:mainfrom
Conversation
`Any` is commonly cached and cloned into `repeated google.protobuf.Any` response fields. With `value: Vec<u8>` each `Any.clone()` is a payload memcpy; with `bytes::Bytes` it is one atomic refcount bump. Wire encoding is unchanged (`Bytes` derefs to `&[u8]`). Change is driven from the regenerator config so `task gen-wkt-types` and the check-generated-code CI job stay green: - gen_wkt_types.rs: set `bytes_fields = [".google.protobuf.Any.value"]`. - buffa-types/Cargo.toml: add direct `bytes` dep (codegen emits `::bytes::Bytes` paths). - generated/google.protobuf.any.rs: regenerated. - any_ext.rs: `pack()` uses `encode_to_bytes()`; `unpack_*()` use `.as_ref()`; JSON-fallback and textproto-merge sites convert via `.into()`. - tests/wkt_roundtrip.rs + any_ext.rs test fixtures: `vec![..].into()`. Adds `buffa-types/benches/any_clone.rs` (criterion): | payload | Vec<u8> | Bytes | speedup | | --- | --- | --- | --- | | 64 B | 32 ns | 34 ns | 1.0x | | 1 KiB | 43 ns | 34 ns | 1.3x | | 16 KiB | 189 ns | 35 ns | 5.4x | | 256 KiB | 5.99 us | 35 ns | 171x | | 1000x1KiB into Vec<Any> | 143 us | 54 us | 2.6x | Clone is now constant-time in payload size (type_url String clone is the floor at ~34 ns). This is a source-breaking change to the public field type. Callers that construct or move `Any.value` directly need a one-line `.into()` / `Default::default()`; reads via `&[u8]` deref are unchanged.
|
All contributors have signed the CLA ✍️ ✅ |
- benches/any_clone.rs: use .to_vec() instead of .iter().cloned().collect() - tests/wkt_roundtrip.rs: drop useless .into() on BytesValue.value (still Vec<u8>)
|
I have read the CLA Document and I hereby sign the CLA |
|
recheck |
- CHANGELOG: document the Vec<u8> -> Bytes breaking change. - Test: assert that cloning Any shares the payload buffer via pointer equality, pinning down the headline invariant of this change so a future refactor that reintroduces a copy can't silently regress it.
|
[claude code] Reviewed and pushed two follow-ups directly to your branch (maintainerCanModify):
Two perf follow-ups I spotted but didn't fix in this PR (they're out of scope / larger):
Minor nits not worth blocking on (feel free to address in this PR or skip):
Otherwise LGTM — clean use of the existing |
|
recheck |
1 similar comment
|
recheck |
iainmcgin
left a comment
There was a problem hiding this comment.
[claude code] LGTM — clean use of the existing bytes_fields knob, correctness verified, no_std clean on host + bare-metal ARM, all CI green.
google.protobuf.Anyis commonly cached and cloned intorepeated google.protobuf.Anyresponse fields (RPC servers, fan-out, message buses). Withvalue: Vec<u8>eachAny.clone()deep-copies the encoded payload; withbytes::Bytesit is one atomic refcount increment, regardless of payload size.The change is driven from
gen_wkt_types.rs(bytes_fields = [".google.protobuf.Any.value"]) sotask gen-wkt-typesand thecheck-generated-codeCI job stay green.any_ext.rshelpers (pack,unpack_*, JSON/textproto paths) are updated; signatures are unchanged.Benchmark
New
buffa-types/benches/any_clone.rs:Any.value.len()Vec<u8>BytesVec<Any>Any.clone()is now constant-time in payload size; the ~34 ns floor is thetype_url: Stringclone.Compatibility
Wire format: fully compatible.
Bytesderefs to&[u8]; encode (encode_bytes), decode (Bytes::from(decode_bytes(buf)?)), JSON (base64 of&[u8]), and textproto all produce/accept identical bytes.Rust API: source-breaking (public field type changed). Common caller patterns:
Any { value: vec![..], .. }vec![..].into()Any { value: Vec::new(), .. }Default::default()let v: Vec<u8> = any.valueany.value.into()any.value.as_slice()&any.value[..]or.as_ref()&any.value(deref to&[u8])any.value.len()/ indexing /is_empty()Any::pack/unpack_if/unpack_uncheckedany.clone()The
.into()/Default::default()shims compile against both the current and the changed type (Vec<u8>.into()is identity forVec<u8>,From<Vec<u8>>forBytes), so callers can pre-adopt before this lands.