From 4313917b4dd1d5c76c285e9d01151a21ba6613b3 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 20:30:16 -0500 Subject: [PATCH] format: object headers without per-header allocations for their chunk list Reading continuation chunks from a queue (7e5e920, a69c5be) allocated a queue Vec and a BTreeSet of chunk starts for every header, and inlined the per-chunk message loop into the generic parser: ObjectHeader::parse over 401 version-1 headers went from 24.8 to 45.7 us. ChunkSpans now keeps the first 8 chunks in an inline array (cycle check by scan) and is also the read queue; only a header of more chunks allocates (a boxed spill list and start set). The message loop of one version-1 chunk is its own non-generic function. Same checks as before: any number of chunks up to 65,536, cycles refused, chunks bounded by the file size, one chunk buffer alive at a time, libhdf5 message order, overlap allowed. The cycle test now also covers spilled chunk lists. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 8 + crates/clawhdf5-format/src/object_header.rs | 204 ++++++++++++++------ 2 files changed, 149 insertions(+), 63 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b1a92a..4556f91 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -295,6 +295,14 @@ chunks nested in each other made storage with owned buffers read and hold the square of the file's size), or more than 65 536 chunks are refused, so a header's chunks read at most the file's size. + The first version allocated a queue and a set of chunk starts for every + header and made `ObjectHeader::parse` of 401 small headers 1.8x slower + (`local_metadata_bench`: 45.7 vs 24.8 µs on `main` `8f59b2e`). The first + 8 chunks now live in an inline array (cycle check by scan; only a longer + header allocates), and the per-chunk message loop is its own function + instead of being inlined into the generic parser. Provisional (tank load + 3–4; Criterion, separate binaries, 2 alternating rounds): 24.96–25.12 vs + 24.85–25.15 µs on `main`. - Tests: `crates/clawhdf5-tools/tests/edit_coverage_interop.rs` (h5py `earliest`/`v110`/`latest` and clawhdf5-written files; structure comparisons with libhdf5 for version-2 B-trees, shrink on every index, diff --git a/crates/clawhdf5-format/src/object_header.rs b/crates/clawhdf5-format/src/object_header.rs index ab652cb..d421ea0 100644 --- a/crates/clawhdf5-format/src/object_header.rs +++ b/crates/clawhdf5-format/src/object_header.rs @@ -1,7 +1,7 @@ //! HDF5 Object Header parsing (v1 and v2). #[cfg(not(feature = "std"))] -use alloc::{collections::BTreeSet, vec, vec::Vec}; +use alloc::{boxed::Box, collections::BTreeSet, vec::Vec}; #[cfg(feature = "std")] use std::collections::BTreeSet; @@ -248,60 +248,14 @@ impl ObjectHeader { length_size: u8, messages: &mut Vec, ) -> Result { + // The chunks found so far are also the queue of chunks to read. let mut spans = ChunkSpans::new(file.len(), offset, length)?; - let mut queue: Vec<(u64, usize)> = vec![(offset, length)]; let mut chunk0_count = 0usize; let mut next = 0usize; - while let Some(&(chunk_offset, chunk_length)) = queue.get(next) { + while let Some((chunk_offset, chunk_length)) = spans.get(next) { let chunk = read_exact_at(file, chunk_offset, chunk_length)?; - let data: &[u8] = &chunk; - let end = data.len(); - let mut pos = 0usize; - let mut count = 0usize; - while pos < end { - if end - pos < V1_MSG_HEADER_SIZE { - return Err(FormatError::InvalidObjectHeader( - "gap found in early version of file format", - )); - } - let msg_type_raw = LittleEndian::read_u16(&data[pos..pos + 2]); - let msg_data_size = LittleEndian::read_u16(&data[pos + 2..pos + 4]) as usize; - let msg_flags = data[pos + 4]; - // reserved(3) at pos+5..pos+8 - pos += V1_MSG_HEADER_SIZE; - - if !msg_data_size.is_multiple_of(8) { - return Err(FormatError::InvalidObjectHeader("message not aligned")); - } - if msg_data_size > end - pos { - return Err(FormatError::InvalidObjectHeader( - "message size exceeds buffer end", - )); - } - let body = &data[pos..pos + msg_data_size]; - check_message(1, msg_type_raw, msg_flags, body, offset_size, length_size)?; - count += 1; - let msg_type = MessageType::from_u16(msg_type_raw); - if msg_type != MessageType::Nil { - messages.push(HeaderMessage { - msg_type, - size: msg_data_size, - flags: msg_flags, - creation_order: None, - data: body.to_vec(), - }); - } - // Queue continuations (v1 continuation chunks are just raw - // messages, no signature); check_message has checked the body. - if msg_type == MessageType::ObjectHeaderContinuation { - let cont_offset = read_offset(body, 0, offset_size)?; - let cont_length = - to_usize(read_offset(body, offset_size as usize, length_size)?)?; - spans.add(cont_offset, cont_length)?; - queue.push((cont_offset, cont_length)); - } - pos += msg_data_size; - } + let count = + Self::parse_v1_messages(&chunk, offset_size, length_size, messages, &mut spans)?; // Only the first chunk's messages are held to the prefix count. if next == 0 { chunk0_count = count; @@ -311,6 +265,65 @@ impl ObjectHeader { Ok(chunk0_count) } + /// The messages of one version-1 chunk: each checked and appended to + /// `messages` (NIL ones dropped), each continuation added to `spans`. + /// Returns how many messages (NIL ones included) the chunk holds. + #[inline(never)] + fn parse_v1_messages( + data: &[u8], + offset_size: u8, + length_size: u8, + messages: &mut Vec, + spans: &mut ChunkSpans, + ) -> Result { + let end = data.len(); + let mut pos = 0usize; + let mut count = 0usize; + while pos < end { + if end - pos < V1_MSG_HEADER_SIZE { + return Err(FormatError::InvalidObjectHeader( + "gap found in early version of file format", + )); + } + let msg_type_raw = LittleEndian::read_u16(&data[pos..pos + 2]); + let msg_data_size = LittleEndian::read_u16(&data[pos + 2..pos + 4]) as usize; + let msg_flags = data[pos + 4]; + // reserved(3) at pos+5..pos+8 + pos += V1_MSG_HEADER_SIZE; + + if !msg_data_size.is_multiple_of(8) { + return Err(FormatError::InvalidObjectHeader("message not aligned")); + } + if msg_data_size > end - pos { + return Err(FormatError::InvalidObjectHeader( + "message size exceeds buffer end", + )); + } + let body = &data[pos..pos + msg_data_size]; + check_message(1, msg_type_raw, msg_flags, body, offset_size, length_size)?; + count += 1; + let msg_type = MessageType::from_u16(msg_type_raw); + if msg_type != MessageType::Nil { + messages.push(HeaderMessage { + msg_type, + size: msg_data_size, + flags: msg_flags, + creation_order: None, + data: body.to_vec(), + }); + } + // Queue continuations (v1 continuation chunks are just raw + // messages, no signature); check_message has checked the body. + if msg_type == MessageType::ObjectHeaderContinuation { + let cont_offset = read_offset(body, 0, offset_size)?; + let cont_length = to_usize(read_offset(body, offset_size as usize, length_size)?)?; + spans.add(cont_offset, cont_length)?; + } + pos += msg_data_size; + } + Ok(count) + } + fn parse_v2( file: &S, offset: u64, @@ -601,24 +614,47 @@ const V2_PREFIX_MAX: usize = 34; /// Size of a version-1 message header: type(2) + size(2) + flags(1) + reserved(3). const V1_MSG_HEADER_SIZE: usize = 8; -/// The chunks of one object header read so far. A chunk starting where -/// another did is a cycle. Chunks of a valid header do not overlap, so +/// The chunks of one object header read so far, in the order they were +/// found (which is the order version-1 chunks are read in). A chunk starting +/// where another did is a cycle. Chunks of a valid header do not overlap, so /// together they are no larger than the file; a header whose chunks add up /// to more is refused, which bounds what its chunks can make a reader read /// (a crafted chain of chunks each nested in the last would otherwise read /// the file over and over). Overlap itself is not refused: libhdf5 reads /// such headers (`cve-2025-7067.h5` has one). +/// +/// Almost every header has at most a few chunks, and this runs once per +/// header, so the first [`INLINE_CHUNKS`] live in an inline array and are +/// checked for cycles by a scan; only a longer header allocates (the rest +/// of the list, and a set of starts). Allocating a queue and a set for +/// every header made parsing 401 small headers 1.8x slower. struct ChunkSpans { - starts: BTreeSet, + inline: [(u64, usize); INLINE_CHUNKS], + /// Chunks after the first [`INLINE_CHUNKS`], and every chunk start. + spill: Option>, + /// How many chunks there are. + len: usize, /// Bytes of the chunks so far, and the most they may add up to. total: u64, budget: u64, } +/// The chunks of a [`ChunkSpans`] beyond its inline ones. +struct SpilledSpans { + chunks: Vec<(u64, usize)>, + starts: BTreeSet, +} + +/// How many chunks [`ChunkSpans`] holds without allocating. +const INLINE_CHUNKS: usize = 8; + impl ChunkSpans { + #[inline] fn new(file_len: u64, start: u64, len: usize) -> Result { let mut s = Self { - starts: BTreeSet::new(), + inline: [(0, 0); INLINE_CHUNKS], + spill: None, + len: 0, total: 0, budget: file_len, }; @@ -626,11 +662,19 @@ impl ChunkSpans { Ok(s) } + /// Record the chunk `len` bytes at `start`. + #[inline] fn add(&mut self, start: u64, len: usize) -> Result<(), FormatError> { - if !self.starts.insert(start) || self.starts.len() > MAX_V1_CHUNKS { - return Err(FormatError::NestingDepthExceeded); - } self.total = self.total.saturating_add(len as u64); + if self.len < INLINE_CHUNKS { + if self.inline[..self.len].iter().any(|&(s, _)| s == start) { + return Err(FormatError::NestingDepthExceeded); + } + self.inline[self.len] = (start, len); + } else { + self.add_spilled(start, len)?; + } + self.len += 1; if self.total > self.budget { return Err(FormatError::InvalidObjectHeader( "object header chunks larger than the file", @@ -638,6 +682,33 @@ impl ChunkSpans { } Ok(()) } + + #[cold] + #[inline(never)] + fn add_spilled(&mut self, start: u64, len: usize) -> Result<(), FormatError> { + let inline = &self.inline; + let spill = self.spill.get_or_insert_with(|| { + Box::new(SpilledSpans { + chunks: Vec::new(), + starts: inline.iter().map(|&(s, _)| s).collect(), + }) + }); + if !spill.starts.insert(start) || self.len >= MAX_V1_CHUNKS { + return Err(FormatError::NestingDepthExceeded); + } + spill.chunks.push((start, len)); + Ok(()) + } + + /// The `i`th chunk recorded. + #[inline] + fn get(&self, i: usize) -> Option<(u64, usize)> { + if i < INLINE_CHUNKS { + (i < self.len).then(|| self.inline[i]) + } else { + self.spill.as_ref()?.chunks.get(i - INLINE_CHUNKS).copied() + } + } } /// Most chunks a version-1 object header may have (malformed-data guard; @@ -1105,11 +1176,18 @@ mod tests { #[test] fn v1_continuation_cycles_are_refused() { - let data = v1_chain(5, true); - assert!(matches!( - ObjectHeader::parse(&data, 0, 8, 8), - Err(FormatError::NestingDepthExceeded) - )); + // Within the inline chunk list, and past it (the cycle returns to + // an inline chunk once the list has spilled). + for n in [5, 7, 8, 9, 40] { + let data = v1_chain(n, true); + assert!( + matches!( + ObjectHeader::parse(&data, 0, 8, 8), + Err(FormatError::NestingDepthExceeded) + ), + "{n} chunks" + ); + } } #[test]