From af9ae0d0ff574b73a66dffbfd708cb1c69f8ee66 Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Sat, 3 Oct 2026 00:58:27 +0200 Subject: [PATCH] perf(table): reserve packed repeated enum capacity --- .../unreleased/changed-20261003-003250.yaml | 3 + buffa/src/table/decode.rs | 23 ++++- buffa/src/table/shape.rs | 58 ++++++++++++- buffa/src/table/tests.rs | 86 +++++++++++++++++++ 4 files changed, 168 insertions(+), 2 deletions(-) create mode 100644 .changes/unreleased/changed-20261003-003250.yaml diff --git a/.changes/unreleased/changed-20261003-003250.yaml b/.changes/unreleased/changed-20261003-003250.yaml new file mode 100644 index 00000000..6143a8ea --- /dev/null +++ b/.changes/unreleased/changed-20261003-003250.yaml @@ -0,0 +1,3 @@ +kind: Changed +body: The table codec now reserves packed repeated enum capacity from the payload length. +time: 2026-10-03T00:32:50.399909+02:00 diff --git a/buffa/src/table/decode.rs b/buffa/src/table/decode.rs index cce1bc19..b4688e0e 100644 --- a/buffa/src/table/decode.rs +++ b/buffa/src/table/decode.rs @@ -388,6 +388,14 @@ unsafe fn merge_enum( let wire = tag.wire_type(); if wire == WireType::LengthDelimited { let mut payload = take_len_delimited(buf)?; + while !payload.is_empty() { + let raw = types::decode_int32_packed(&mut payload)?; + let stored = (vt.set_packed)(slot, raw, payload); + enum_store_result(table, e, base, raw, stored, ctx)?; + if stored { + break; + } + } while !payload.is_empty() { let raw = types::decode_int32_packed(&mut payload)?; enum_store(table, e, base, vt, slot, raw, ctx)?; @@ -427,7 +435,20 @@ unsafe fn enum_store( ctx: DecodeContext<'_>, ) -> Result<(), DecodeError> { // SAFETY: `slot` matches the shape `vt` was built for. - if unsafe { (vt.set)(slot, raw) } || table.unknown == NO_UNKNOWN { + let stored = unsafe { (vt.set)(slot, raw) }; + enum_store_result(table, e, base, raw, stored, ctx) +} + +#[inline] +fn enum_store_result( + table: &MessageTable, + e: &Entry, + base: *mut u8, + raw: i32, + stored: bool, + ctx: DecodeContext<'_>, +) -> Result<(), DecodeError> { + if stored || table.unknown == NO_UNKNOWN { return Ok(()); } ctx.register_unknown_field()?; diff --git a/buffa/src/table/shape.rs b/buffa/src/table/shape.rs index 214d47ba..de0857f7 100644 --- a/buffa/src/table/shape.rs +++ b/buffa/src/table/shape.rs @@ -174,6 +174,7 @@ pub struct EnumVt { /// The argument points to a live slot of the shape the descriptor was /// built for. pub(super) set: unsafe fn(*mut u8, i32) -> bool, + pub(super) set_packed: unsafe fn(*mut u8, i32, &[u8]) -> bool, /// The value at `idx` (ignored for singular shapes), or `None` if unset. /// /// # Safety @@ -212,6 +213,17 @@ pub unsafe trait EnumShape { /// `slot` points to a live [`Slot`](Self::Slot). unsafe fn set(slot: *mut u8, raw: i32) -> bool; + /// Store a packed value the same way as [`set`](Self::set), using the + /// remaining payload as a capacity hint. + /// + /// # Safety + /// + /// `slot` points to a live [`Slot`](Self::Slot). + unsafe fn set_packed(slot: *mut u8, raw: i32, _remaining_payload: &[u8]) -> bool { + // SAFETY: this method has the same slot contract as `set`. + unsafe { Self::set(slot, raw) } + } + /// The value at `idx` (ignored for singular shapes), or `None` if unset. /// /// # Safety @@ -236,14 +248,18 @@ impl EnumVt { Self { card: S::CARD, set: S::set, + set_packed: S::set_packed, get: S::get, len: S::len, } } } +const MAX_OPEN_ENUM_RESERVE_BYTES: usize = 64 * 1024; +const MAX_CLOSED_ENUM_RESERVE_VALUES: usize = 16; + macro_rules! enum_shape { - ($(#[$m:meta])* $name:ident, $card:ident, $slot:ty, $set:expr, $get:expr, $len:expr) => { + ($(#[$m:meta])* $name:ident, $card:ident, $slot:ty, $set:expr, $set_packed:expr, $get:expr, $len:expr) => { $(#[$m])* pub struct $name(PhantomData); @@ -260,6 +276,13 @@ macro_rules! enum_shape { ($set)(s, raw) } + #[inline] + unsafe fn set_packed(slot: *mut u8, raw: i32, remaining_payload: &[u8]) -> bool { + // SAFETY: as above. + let s = unsafe { &mut *slot.cast::<$slot>() }; + ($set_packed)(s, raw, remaining_payload) + } + #[inline] unsafe fn get(slot: *const u8, idx: usize) -> Option { // SAFETY: as above. @@ -273,6 +296,7 @@ macro_rules! enum_shape { let s = unsafe { &*slot.cast::<$slot>() }; ($len)(s) } + } }; } @@ -281,6 +305,7 @@ enum_shape!( /// An open enum with implicit presence: `EnumValue`. ImplicitOpen, IMPLICIT, EnumValue, |s: &mut EnumValue, raw| { *s = EnumValue::from(raw); true }, + |s: &mut EnumValue, raw, _| { *s = EnumValue::from(raw); true }, |s: &EnumValue, _| Some(s.to_i32()), |_: &EnumValue| 0 ); @@ -288,6 +313,7 @@ enum_shape!( /// A closed enum with implicit presence: `E`. ImplicitClosed, IMPLICIT, E, |s: &mut E, raw| match E::from_i32(raw) { Some(v) => { *s = v; true } None => false }, + |s: &mut E, raw, _| match E::from_i32(raw) { Some(v) => { *s = v; true } None => false }, |s: &E, _| Some(s.to_i32()), |_: &E| 0 ); @@ -295,6 +321,7 @@ enum_shape!( /// An open enum with explicit presence: `Option>`. OptionalOpen, OPTIONAL, Option>, |s: &mut Option>, raw| { *s = Some(EnumValue::from(raw)); true }, + |s: &mut Option>, raw, _| { *s = Some(EnumValue::from(raw)); true }, |s: &Option>, _| s.as_ref().map(EnumValue::to_i32), |_: &Option>| 0 ); @@ -302,6 +329,7 @@ enum_shape!( /// A closed enum with explicit presence: `Option`. OptionalClosed, OPTIONAL, Option, |s: &mut Option, raw| match E::from_i32(raw) { Some(v) => { *s = Some(v); true } None => false }, + |s: &mut Option, raw, _| match E::from_i32(raw) { Some(v) => { *s = Some(v); true } None => false }, |s: &Option, _| s.as_ref().map(Enumeration::to_i32), |_: &Option| 0 ); @@ -309,6 +337,18 @@ enum_shape!( /// A repeated open enum: `Vec>`. RepeatedOpen, REPEATED, Vec>, |s: &mut Vec>, raw| { s.push(EnumValue::from(raw)); true }, + |s: &mut Vec>, raw, remaining: &[u8]| { + let requested = remaining + .len() + .saturating_add(1) + .min(MAX_OPEN_ENUM_RESERVE_BYTES / core::mem::size_of::>().max(1)); + let available = s.capacity().saturating_sub(s.len()); + if available == 0 && requested > 0 { + s.reserve_exact(requested); + } + s.push(EnumValue::from(raw)); + true + }, |s: &Vec>, i| s.get(i).map(EnumValue::to_i32), |s: &Vec>| s.len() ); @@ -316,6 +356,22 @@ enum_shape!( /// A repeated closed enum: `Vec`. RepeatedClosed, REPEATED, Vec, |s: &mut Vec, raw| match E::from_i32(raw) { Some(v) => { s.push(v); true } None => false }, + |s: &mut Vec, raw, remaining: &[u8]| match E::from_i32(raw) { + Some(v) => { + let requested = remaining + .len() + .saturating_add(1) + .min(MAX_CLOSED_ENUM_RESERVE_VALUES) + .min(MAX_OPEN_ENUM_RESERVE_BYTES / core::mem::size_of::().max(1)); + let available = s.capacity().saturating_sub(s.len()); + if available == 0 && requested > 0 { + s.reserve_exact(requested); + } + s.push(v); + true + } + None => false, + }, |s: &Vec, i| s.get(i).map(Enumeration::to_i32), |s: &Vec| s.len() ); diff --git a/buffa/src/table/tests.rs b/buffa/src/table/tests.rs index 05346705..6d4c39b1 100644 --- a/buffa/src/table/tests.rs +++ b/buffa/src/table/tests.rs @@ -518,6 +518,92 @@ fn a_closed_enum_value_it_does_not_know_becomes_an_unknown_field() { ); } +#[test] +fn a_packed_repeated_open_enum_appends_values_to_existing_storage() { + let mut wire = vec![0x42, 0x40]; + for _ in 0..32 { + wire.extend_from_slice(&[0x80, 0x01]); + } + + let mut msg = Wide { + rep_enum_open: vec![EnumValue::from(42)], + ..Wide::default() + }; + msg.merge_from_slice(&wire).unwrap(); + + assert_eq!(msg.rep_enum_open.len(), 33); + assert_eq!(msg.rep_enum_open[0].to_i32(), 42); + assert!(msg.rep_enum_open[1..] + .iter() + .all(|value| value.to_i32() == 128)); +} + +#[test] +fn an_empty_packed_repeated_enum_payload_does_not_reserve() { + let msg = Wide::decode_from_slice(&[0x42, 0x00]).unwrap(); + + assert!(msg.rep_enum_open.is_empty()); + assert_eq!(msg.rep_enum_open.capacity(), 0); +} + +#[test] +fn a_packed_repeated_closed_enum_preserves_unknown_values() { + let mut wire = vec![0x5a, 0x41]; + wire.push(0x00); + for _ in 0..32 { + wire.extend_from_slice(&[0x80, 0x01]); + } + + let msg = Outer::decode_from_slice(&wire).unwrap(); + + assert_eq!(msg.packed_closed, [Color::Red]); + assert_eq!(msg.unknown.len(), 32); +} + +#[test] +fn an_all_unknown_packed_closed_enum_does_not_reserve() { + let mut wire = vec![0x5a, 0x40]; + for _ in 0..32 { + wire.extend_from_slice(&[0x80, 0x01]); + } + + let msg = Outer::decode_from_slice(&wire).unwrap(); + + assert!(msg.packed_closed.is_empty()); + assert_eq!(msg.unknown.len(), 32); + assert_eq!(msg.packed_closed.capacity(), 0); +} + +#[test] +fn a_malformed_packed_open_enum_does_not_reserve_from_later_bytes() { + let mut wire = vec![0x42, 0x4b]; + wire.extend_from_slice(&[0x80; 11]); + wire.extend_from_slice(&[0x00; 64]); + + let mut msg = Wide::default(); + assert_eq!(msg.merge_from_slice(&wire), Err(DecodeError::VarintTooLong)); + assert!(msg.rep_enum_open.is_empty()); + assert_eq!(msg.rep_enum_open.capacity(), 0); +} + +#[test] +fn a_malformed_packed_open_enum_tail_keeps_existing_spare_capacity() { + let mut wire = vec![0x42, 0x70]; + wire.push(0x00); + wire.extend_from_slice(&[0x80; 11]); + wire.extend_from_slice(&[0x00; 100]); + + let mut msg = Wide { + rep_enum_open: Vec::with_capacity(2), + ..Wide::default() + }; + msg.rep_enum_open.push(EnumValue::from(42)); + let capacity = msg.rep_enum_open.capacity(); + assert_eq!(msg.merge_from_slice(&wire), Err(DecodeError::VarintTooLong)); + assert_eq!(msg.rep_enum_open.len(), 2); + assert_eq!(msg.rep_enum_open.capacity(), capacity); +} + #[test] fn a_closed_enum_value_is_dropped_by_a_message_that_drops_unknown_fields() { // rep_enum_closed (20) = [1, 7, 2] unpacked; optional closed (9) = 7.