From d4635d28f064ce831f3566cf46abef302f3ecff4 Mon Sep 17 00:00:00 2001 From: "glm-5.3-flash" Date: Thu, 3 Sep 2026 17:28:20 +0000 Subject: [PATCH] perf: fixed-size struct fast path, integer union dispatch, zero-alloc read_next_borrowed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Targets the bench gaps from the 0.3.0 port review (commit dea96f0): packet read was ~111-189x hand-rolled, chunk read ~18x. - ReadPlan gains compile-time fixed_size (cached field-size sum). Fixed structs skip the cursor size walk entirely (one bounds check instead); fixed-size union variants skip the plan_walk_variant_size pre-pass, eliminating the double walk of variant bytes for the common SFTP-shaped case. - CompositePlan::Union gains an int_keys dispatch table (pre-parsed u64 mapping keys); byte-discriminator unions dispatch on the raw integer instead of stringifying per read. Returned discriminator String unchanged (public API). String-keyed fallback preserved. - Additive SequentialReader::read_next_borrowed returns the field name borrowed from the plan — zero allocs per field for hot loops. read_next stays the owned-name form (single source of truth). - plan_walk_struct_size / union shared walk: per-field format! moved to the error path only. - materialize: with_capacity for bytes arrays, arrays, and struct objects. Benches (1024 chunks/iter, criterion, pre-review baseline vs now): - read_packet_stream: 600 -> 246 µs (~2.4x; gap to hand 189x -> ~74x) - read_chunk_stream: 104 -> 67 µs (~1.6x; 18x -> ~11x) - write/validate groups unchanged (within noise) - engine_compile +8% (int_keys table + fixed-size precompute), still one-shot Verification: 566 tests pass, clippy -D warnings clean, wasm32 build green. Bench baselines saved as pre-review/post-review. --- benches/wire_vs_bast.rs | 14 +-- src/materialize.rs | 16 ++-- src/read_plan.rs | 136 ++++++++++++++++++++-------- src/sequential_reader.rs | 190 ++++++++++++++++++++++++++++++++++----- 4 files changed, 284 insertions(+), 72 deletions(-) diff --git a/benches/wire_vs_bast.rs b/benches/wire_vs_bast.rs index 4a667e3..08ed40b 100644 --- a/benches/wire_vs_bast.rs +++ b/benches/wire_vs_bast.rs @@ -216,11 +216,11 @@ fn alktype_read_stream(buf: &[u8], n: usize, reader: &mut SequentialReader) -> u let mut total = 0usize; for _ in 0..n { reader.reset(); - let st = match reader.read_next(&buf[pos..]) { + let st = match reader.read_next_borrowed(&buf[pos..]) { Ok(Some((_, FieldValue::U8(v)))) => v, _ => break, }; - let len = match reader.read_next(&buf[pos..]) { + let len = match reader.read_next_borrowed(&buf[pos..]) { Ok(Some((_, FieldValue::U32(v)))) => v, _ => break, }; @@ -372,10 +372,12 @@ fn hand_read_packet_stream(buf: &[u8], n: usize) -> usize { // --------------------------------------------------------------------------- /// Walk one variant's fields to exhaustion; returns bytes consumed. +/// Uses `read_next_borrowed` — the zero-alloc hot-loop pattern for +/// consumers that match or discard the field name. fn alktype_walk_variant(reader: &mut SequentialReader, buf: &[u8]) -> Option { reader.reset(); loop { - match reader.read_next(buf) { + match reader.read_next_borrowed(buf) { Ok(Some((name, value))) => { black_box(name); black_box(&value); @@ -397,7 +399,7 @@ fn alktype_read_packet_stream( let mut total = 0usize; for _ in 0..n { packet.reset(); - let disc = match packet.read_next(&buf[pos..]) { + let disc = match packet.read_next_borrowed(&buf[pos..]) { Ok(Some((_, FieldValue::Union { discriminator, variant_start, @@ -437,7 +439,7 @@ fn assert_packet_reader_parity( let read_pkt = make_packet_bytes(5, &payload); packet.reset(); - match packet.read_next(&read_pkt) { + match packet.read_next_borrowed(&read_pkt) { Ok(Some((_, FieldValue::Union { discriminator, variant_start, @@ -453,7 +455,7 @@ fn assert_packet_reader_parity( let write_pkt = make_packet_bytes(6, &payload); packet.reset(); - match packet.read_next(&write_pkt) { + match packet.read_next_borrowed(&write_pkt) { Ok(Some((_, FieldValue::Union { discriminator, variant_start, diff --git a/src/materialize.rs b/src/materialize.rs index a6aca7c..aadf7fe 100644 --- a/src/materialize.rs +++ b/src/materialize.rs @@ -46,7 +46,7 @@ pub fn materialize_packed( buffer: &[u8], ) -> Result { let mut offset = 0usize; - let mut obj = Map::new(); + let mut obj = Map::with_capacity(plan.fields().len()); for field in plan.fields() { let value = materialize_plan_field(field, field.name(), buffer, &mut offset)?; obj.insert(field.name().to_string(), value); @@ -188,7 +188,8 @@ fn materialize_plan_primitive( reason: "bytes size 4 + len overflows usize".to_string(), })?; *offset = checked_add_at(*offset, len, field_path, "bytes")?; - let arr: Vec = b.iter().map(|&byte| Value::from(u32::from(byte))).collect(); + let mut arr = Vec::with_capacity(b.len()); + arr.extend(b.iter().map(|&byte| Value::from(u32::from(byte)))); Ok(Value::Array(arr)) } other => Err(AlkTypeError::Schema(format!( @@ -215,7 +216,7 @@ fn materialize_plan_struct( buffer: &[u8], offset: &mut usize, ) -> Result { - let mut obj = Map::new(); + let mut obj = Map::with_capacity(plan.fields().len()); for field in plan.fields() { let path = format!("{field_path}.{}", field.name()); let value = materialize_plan_field(field, &path, buffer, offset)?; @@ -243,7 +244,7 @@ fn materialize_plan_array( ))) } }; - let mut arr = Vec::new(); + let mut arr = Vec::with_capacity(count); for i in 0..count { let path = format!("{field_path}[{i}]"); let before = *offset; @@ -309,12 +310,13 @@ fn materialize_plan_union( buffer: &[u8], offset: &mut usize, ) -> Result { - let (disc, shared, variants) = match body { + let (disc, shared, variants, _) = match body { CompositePlan::Union { disc, shared, variants, - } => (disc, shared, variants), + int_keys: _, + } => (disc, shared, variants, ()), _ => { return Err(AlkTypeError::Schema(format!( "internal: union body at {field_path} is not CompositePlan::Union" @@ -645,7 +647,7 @@ fn materialize_array_packed( ) -> Result { let element_ty = array.element(); let count = array.count(); - let mut arr = Vec::new(); + let mut arr = Vec::with_capacity(count); for i in 0..count { let path = format!("{field_path}[{i}]"); let resolved_elem = doc.resolve_typeref(element_ty)?; diff --git a/src/read_plan.rs b/src/read_plan.rs index 1935629..c8cf680 100644 --- a/src/read_plan.rs +++ b/src/read_plan.rs @@ -68,6 +68,7 @@ pub struct ReadPlan { endian: Endian, fields: Vec, by_name: BTreeMap, + fixed_size: Option, schema: Arc, } @@ -111,10 +112,18 @@ pub enum CompositePlan { /// variant starts immediately after the discriminator's bytes. /// A variant may itself be `CompositePlan::Union` (nested unions — /// ordinary recursion, no separate variant type). + /// + /// `int_keys` is the byte-discriminator dispatch table: when every + /// mapping key parses as a `u64`, it carries `(int_key, + /// variant_index)` in `variants` order and the read loop dispatches + /// on the raw discriminator integer without stringifying it. + /// `None` when any key is non-numeric (or overflows `u64`) — the + /// string-keyed fallback applies. Union { disc: DiscriminatorPlan, shared: Option>, variants: Vec<(String, CompositePlan)>, + int_keys: Option>, }, /// A fixed-count array. `element_stride` is the true stride for /// fixed-size elements; `0` signals variable-length elements (the @@ -209,6 +218,19 @@ impl ReadPlan { &self.schema } + /// The struct's compile-time-known byte size, or `None` when any + /// field is variable-length (or a union/record). `Some` means a + /// struct walk consumes exactly this many bytes regardless of + /// buffer contents. + /// + /// Additive in 0.3.0: computed once during [`ReadPlan::compile`], + /// used by the packed read loop to skip size walks for fixed + /// structs (and to skip union variant size pre-passes when the + /// variant is fixed-size). + pub fn fixed_size(&self) -> Option { + self.fixed_size + } + /// A stable-within-version hash of the plan (ADR-012 §1/§4). /// /// Two plans with equal [`fingerprint`](Self::fingerprint)s (equal @@ -297,10 +319,12 @@ fn compile_struct( for (i, field) in fields.iter().enumerate() { by_name.entry(field.name().to_string()).or_insert(i); } + let fixed_size = fixed_plan_size(&fields)?; Ok(ReadPlan { endian: container_endian, fields, by_name, + fixed_size, schema: Arc::clone(schema), }) } @@ -469,10 +493,12 @@ fn compile_union( for (i, field) in plan.iter().enumerate() { by_name.entry(field.name().to_string()).or_insert(i); } + let fixed_size = fixed_plan_size(&plan)?; Some(Box::new(ReadPlan { endian: container_endian, fields: plan, by_name, + fixed_size, schema: Arc::clone(schema), })) } @@ -484,13 +510,37 @@ fn compile_union( compile_variant(doc, variant_ty, container_endian, &variant_path, depth, seen, schema)?; variants.push(((*key).to_string(), body)); } + let int_keys = compile_int_keys(&variants, path)?; Ok(CompositePlan::Union { disc, shared, variants, + int_keys, }) } +/// Build the integer dispatch table for a byte-discriminator union: +/// `(parsed_key, variant_index)` pairs in `variants` order, or `None` +/// when any key is non-numeric (the string fallback applies). Keys are +/// validated at parse time as stringified integers; a `u64` overflow +/// means the key cannot equal a discriminator read from at most 4 +/// bytes, so such a table is simply not built (the string path also +/// handles it correctly — it would never match, erroring at read time +/// exactly as before). +fn compile_int_keys( + variants: &[(String, CompositePlan)], + _path: &str, +) -> Result>, AlkTypeError> { + let mut out = Vec::with_capacity(variants.len()); + for (i, (key, _)) in variants.iter().enumerate() { + match key.parse::() { + Ok(v) => out.push((v, i)), + Err(_) => return Ok(None), + } + } + Ok(Some(out)) +} + /// Compile one union mapping entry. A variant must be a struct or a /// union (mirroring the 0.2.0 read loop's `resolve_and_walk_variant`); /// a `$ref` variant resolves through the cycle set. @@ -640,10 +690,12 @@ fn wrap_leaf( // is `Value::Null` by construction. It never escapes — // `materialize_plan_composite` unwraps it and the // public `schema()` accessor returns the root plan's. + let fixed_size = fixed_plan_size(std::slice::from_ref(&field))?; Ok(CompositePlan::Struct(ReadPlan { endian: container_endian, fields: vec![field], by_name: BTreeMap::new(), + fixed_size, schema: Arc::new(Value::Null), })) } @@ -654,45 +706,15 @@ fn wrap_leaf( } } -/// The compile-time-known byte size of a composite node, or `None` when -/// variable-length. Used for array strides: primitives and enums -/// contribute their fixed size; structs sum their fields; nested arrays -/// with fixed elements contribute `count × stride`; unions, records, -/// and variable-length primitives make the whole node variable. -/// -/// Returns `Err` only on arithmetic overflow while summing struct field -/// sizes or array products — after the array caps ([`MAX_ARRAY_BYTES`], -/// [`MAX_ARRAY_ELEMENTS`]) no honest schema can reach those arms, and -/// treating overflow as "variable-length" (the pre-0.3.1 -/// `unwrap_or_default` behavior) would silently mis-plan the wire. -fn fixed_composite_size(body: &CompositePlan) -> Result, AlkTypeError> { - match body { - CompositePlan::Struct(plan) => fixed_plan_size(plan), - CompositePlan::Union { .. } | CompositePlan::Record { .. } => Ok(None), - CompositePlan::Array { - count, - element_stride, - .. - } => { - if *element_stride == 0 { - Ok(None) - } else { - match count.checked_mul(*element_stride) { - Some(total) => Ok(Some(total)), - None => Err(AlkTypeError::Schema( - "internal: array size product overflowed while sizing a fixed-stride \ - composite (array caps should have rejected this schema earlier)" - .to_string(), - )), - } - } - } - } -} - -fn fixed_plan_size(plan: &ReadPlan) -> Result, AlkTypeError> { +/// The shared compile-time size computation behind +/// [`ReadPlan::fixed_size`] and array-stride sizing: sum the fields' +/// fixed byte sizes, returning `None` when any field is +/// variable-length (or a union/record, which are variable by +/// construction). `Err` only on arithmetic overflow, which the array +/// compile-time caps make unreachable for honest schemas. +fn fixed_plan_size(fields: &[FieldPlan]) -> Result, AlkTypeError> { let mut total = 0usize; - for field in plan.fields() { + for field in fields { let size = match field.kind() { ReadKind::Primitive(k) if k.is_fixed_size() => { k.type_size().ok_or_else(|| { @@ -730,6 +752,42 @@ fn fixed_plan_size(plan: &ReadPlan) -> Result, AlkTypeError> { Ok(Some(total)) } +/// The compile-time-known byte size of a composite node, or `None` when +/// variable-length. Used for array strides: primitives and enums +/// contribute their fixed size; structs sum their fields; nested arrays +/// with fixed elements contribute `count × stride`; unions, records, +/// and variable-length primitives make the whole node variable. +/// +/// Returns `Err` only on arithmetic overflow while summing struct field +/// sizes or array products — after the array caps ([`MAX_ARRAY_BYTES`], +/// [`MAX_ARRAY_ELEMENTS`]) no honest schema can reach those arms, and +/// treating overflow as "variable-length" (the pre-0.3.1 +/// `unwrap_or_default` behavior) would silently mis-plan the wire. +fn fixed_composite_size(body: &CompositePlan) -> Result, AlkTypeError> { + match body { + CompositePlan::Struct(plan) => Ok(plan.fixed_size), + CompositePlan::Union { .. } | CompositePlan::Record { .. } => Ok(None), + CompositePlan::Array { + count, + element_stride, + .. + } => { + if *element_stride == 0 { + Ok(None) + } else { + match count.checked_mul(*element_stride) { + Some(total) => Ok(Some(total)), + None => Err(AlkTypeError::Schema( + "internal: array size product overflowed while sizing a fixed-stride \ + composite (array caps should have rejected this schema earlier)" + .to_string(), + )), + } + } + } + } +} + #[cfg(test)] mod tests { use super::*; @@ -850,6 +908,7 @@ mod tests { disc, shared, variants, + .. } => { assert!(matches!( disc, @@ -886,6 +945,7 @@ mod tests { disc, shared, variants, + .. } => { assert!(matches!( disc, diff --git a/src/sequential_reader.rs b/src/sequential_reader.rs index 428f4e9..d5ed315 100644 --- a/src/sequential_reader.rs +++ b/src/sequential_reader.rs @@ -179,10 +179,43 @@ impl SequentialReader { &mut self, buffer: &'a [u8], ) -> Result)>, AlkTypeError> { + match self.read_next_borrowed(buffer)? { + Some((name, value)) => Ok(Some((name.to_string(), value))), + None => Ok(None), + } + } + + /// Read the next field from `buffer` at the current position, + /// returning the field name **borrowed from the compiled plan** + /// instead of a freshly allocated `String`. + /// + /// Zero-allocation on the happy path — for hot stream-parsing loops + /// where the field name is matched or discarded, this avoids one + /// heap allocation and free per field. The name's lifetime is tied + /// to this reader (the name lives in the shared [`ReadPlan`], which + /// the reader holds via `Arc`), not to `buffer`. + /// + /// The value's behavior is identical to [`Self::read_next`]: + /// `Ok(Some((field_name, value)))` with the internal position + /// advanced, or `Ok(None)` when all fields have been read. + /// + /// # Errors + /// + /// Propagates [`AlkTypeError::Access`] from the underlying + /// [`crate::data_access`] reads when `buffer` is too short or + /// contains invalid data. + /// + /// Additive in 0.3.0 (perf review): [`Self::read_next`] remains the + /// stable owned-name form; this method is the allocation-free + /// alternative for consumers that do not need an owned name. + pub fn read_next_borrowed<'p, 'a>( + &'p mut self, + buffer: &'a [u8], + ) -> Result)>, AlkTypeError> { if self.field_index >= self.plan.fields().len() { return Ok(None); } - let name = self.plan.fields()[self.field_index].name().to_string(); + let name = self.plan.fields()[self.field_index].name(); let (value, new_position) = self.read_field_at(buffer, self.field_index, self.position)?; self.position = new_position; @@ -297,7 +330,13 @@ fn plan_read_field_at<'a>( ))) } }; - let size = plan_walk_struct_size(body, buffer, offset, field_path)?; + let size = match body.fixed_size() { + Some(size) => { + plan_bounds_check(buffer, offset, size, field_path)?; + size + } + None => plan_walk_struct_size(body, buffer, offset, field_path)?, + }; let end = checked_end(offset, size, field_path, "struct")?; Ok((FieldValue::Struct { start: offset, end }, end)) } @@ -427,6 +466,12 @@ fn checked_end( /// Walk a struct plan's fields sequentially from `offset`, returning /// the total byte size. Only advances the cursor (reads length /// prefixes); does not collect values. +/// +/// Field paths are assembled only on the error path: the per-field +/// `format!` of the interpretive walk dominated hot loops, but the walk +/// itself only needs a path when a check fails, so each field carries +/// its plan-compiled name and the enclosing `field_path` is joined +/// lazily. fn plan_walk_struct_size( plan: &ReadPlan, buffer: &[u8], @@ -435,10 +480,10 @@ fn plan_walk_struct_size( ) -> Result { let mut position = offset; for field in plan.fields() { - let sub_path = format!("{field_path}.{}", field.name()); let (_, new_position) = - plan_read_kind(field, buffer, position, &sub_path)?; + plan_read_kind(field, buffer, position, field_path)?; if new_position < position { + let sub_path = format!("{field_path}.{}", field.name()); return Err(AlkTypeError::Access { field_path: sub_path, reason: format!("struct field walked backwards: {position} → {new_position}"), @@ -475,7 +520,13 @@ fn plan_read_kind<'a>( ))) } }; - let size = plan_walk_struct_size(body, buffer, offset, field_path)?; + let size = match body.fixed_size() { + Some(size) => { + plan_bounds_check(buffer, offset, size, field_path)?; + size + } + None => plan_walk_struct_size(body, buffer, offset, field_path)?, + }; let end = checked_end(offset, size, field_path, "struct")?; Ok((FieldValue::Struct { start: offset, end }, end)) } @@ -523,12 +574,13 @@ fn plan_read_union<'a>( field_path: &str, endian: Endian, ) -> Result<(FieldValue<'a>, usize), AlkTypeError> { - let (disc, shared, variants) = match body { + let (disc, shared, variants, int_keys) = match body { CompositePlan::Union { disc, shared, variants, - } => (disc, shared, variants), + int_keys, + } => (disc, shared, variants, int_keys), _ => { return Err(AlkTypeError::Schema(format!( "internal: union body at {field_path} is not CompositePlan::Union" @@ -543,17 +595,60 @@ fn plan_read_union<'a>( let abs_offset = checked_end(offset, *disc_offset, field_path, "discriminator")?; let (disc_value, disc_size) = plan_read_byte_discriminator(buffer, abs_offset, field_path, *disc_type, endian)?; - let key = disc_value.to_string(); - let variant = variants - .iter() - .find(|(k, _)| *k == key) - .map(|(_, v)| v) - .ok_or_else(|| AlkTypeError::Access { - field_path: field_path.to_string(), - reason: format!("unknown union discriminator value: {key}"), - })?; + // Integer dispatch when the mapping is fully numeric: match + // the raw discriminator integer against the pre-parsed + // compile-time keys (no per-read stringify, no string + // compares). The returned `FieldValue::Union` key stays a + // `String` — cloned from the matched mapping key, which for + // numeric keys is the same string the old + // `disc_value.to_string()` produced. + let (key, variant) = match int_keys { + Some(table) => { + let idx = table + .iter() + .find(|(k, _)| *k == u64::from(disc_value)) + .map(|&(_, i)| i) + .ok_or_else(|| AlkTypeError::Access { + field_path: field_path.to_string(), + reason: format!("unknown union discriminator value: {disc_value}"), + })?; + (variants[idx].0.clone(), &variants[idx].1) + } + None => { + // String-keyed fallback: find the variant first, + // then clone the matched mapping key for the + // returned value (no leak, no borrow tangle). + let key_str = disc_value.to_string(); + let idx = variants + .iter() + .position(|(k, _)| *k == key_str) + .ok_or_else(|| AlkTypeError::Access { + field_path: field_path.to_string(), + reason: format!("unknown union discriminator value: {key_str}"), + })?; + (variants[idx].0.clone(), &variants[idx].1) + } + }; let variant_start = checked_end(abs_offset, disc_size, field_path, "variant")?; - let variant_size = plan_walk_variant_size(variant, buffer, variant_start, field_path, endian)?; + let variant_size = match plan_variant_fixed_size(variant) { + Some(size) => { + let end = checked_end(variant_start, size, field_path, "union")?; + if buffer.len() < end { + return Err(AlkTypeError::Access { + field_path: field_path.to_string(), + reason: format!( + "union variant needs bytes [0..{size}) from variant start \ + {variant_start}, buffer has {} remaining", + buffer.len().saturating_sub(variant_start), + ), + }); + } + size + } + None => { + plan_walk_variant_size(variant, buffer, variant_start, field_path, endian)? + } + }; let end = checked_end(variant_start, variant_size, field_path, "union")?; Ok(( FieldValue::Union { @@ -589,9 +684,9 @@ fn plan_read_union<'a>( // shared fields); the variant starts after all of it. let mut position = offset; for field in shared_plan.fields() { - let sub_path = format!("{field_path}.{}", field.name()); - let (_, new_position) = plan_read_kind(field, buffer, position, &sub_path)?; + let (_, new_position) = plan_read_kind(field, buffer, position, field_path)?; if new_position < position { + let sub_path = format!("{field_path}.{}", field.name()); return Err(AlkTypeError::Access { field_path: sub_path, reason: format!( @@ -601,8 +696,25 @@ fn plan_read_union<'a>( } position = new_position; } - let variant_size = - plan_walk_variant_size(variant, buffer, position, field_path, endian)?; + let variant_size = match plan_variant_fixed_size(variant) { + Some(size) => { + let end = checked_end(position, size, field_path, "union")?; + if buffer.len() < end { + return Err(AlkTypeError::Access { + field_path: field_path.to_string(), + reason: format!( + "union variant needs bytes [0..{size}) from variant start \ + {position}, buffer has {} remaining", + buffer.len().saturating_sub(position), + ), + }); + } + size + } + None => { + plan_walk_variant_size(variant, buffer, position, field_path, endian)? + } + }; let end = checked_end(position, variant_size, field_path, "union")?; Ok(( FieldValue::Union { @@ -688,6 +800,42 @@ fn plan_read_byte_discriminator( } } +/// A compiled struct variant's compile-time-known byte size, if any. +/// Union variants are variable by construction (their size depends on +/// the nested discriminator); the compile step already rejects +/// non-struct/union variant bodies, so the catch-all arm is +/// unreachable in practice but keeps the match total. +fn plan_variant_fixed_size(variant: &CompositePlan) -> Option { + match variant { + CompositePlan::Struct(p) => p.fixed_size(), + _ => None, + } +} + +fn plan_bounds_check( + buffer: &[u8], + offset: usize, + size: usize, + field_path: &str, +) -> Result<(), AlkTypeError> { + let end = offset + .checked_add(size) + .ok_or_else(|| AlkTypeError::Access { + field_path: field_path.to_string(), + reason: format!("struct end {offset} + {size} overflows usize"), + })?; + if buffer.len() < end { + return Err(AlkTypeError::Access { + field_path: field_path.to_string(), + reason: format!( + "struct at {offset} needs {size} bytes, buffer has {} remaining", + buffer.len().saturating_sub(offset), + ), + }); + } + Ok(()) +} + /// Resolve a compiled variant body to its byte size starting at /// `variant_start`. Struct variants walk their field list; union /// variants (nested unions) read their discriminator and advance.