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) <[email protected]>
This commit is contained in:
@@ -295,6 +295,14 @@
|
|||||||
chunks nested in each other made storage with owned buffers read and
|
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
|
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.
|
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
|
- Tests: `crates/clawhdf5-tools/tests/edit_coverage_interop.rs` (h5py
|
||||||
`earliest`/`v110`/`latest` and clawhdf5-written files; structure
|
`earliest`/`v110`/`latest` and clawhdf5-written files; structure
|
||||||
comparisons with libhdf5 for version-2 B-trees, shrink on every index,
|
comparisons with libhdf5 for version-2 B-trees, shrink on every index,
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
//! HDF5 Object Header parsing (v1 and v2).
|
//! HDF5 Object Header parsing (v1 and v2).
|
||||||
|
|
||||||
#[cfg(not(feature = "std"))]
|
#[cfg(not(feature = "std"))]
|
||||||
use alloc::{collections::BTreeSet, vec, vec::Vec};
|
use alloc::{boxed::Box, collections::BTreeSet, vec::Vec};
|
||||||
#[cfg(feature = "std")]
|
#[cfg(feature = "std")]
|
||||||
use std::collections::BTreeSet;
|
use std::collections::BTreeSet;
|
||||||
|
|
||||||
@@ -248,13 +248,34 @@ impl ObjectHeader {
|
|||||||
length_size: u8,
|
length_size: u8,
|
||||||
messages: &mut Vec<HeaderMessage>,
|
messages: &mut Vec<HeaderMessage>,
|
||||||
) -> Result<usize, FormatError> {
|
) -> Result<usize, FormatError> {
|
||||||
|
// The chunks found so far are also the queue of chunks to read.
|
||||||
let mut spans = ChunkSpans::new(file.len(), offset, length)?;
|
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 chunk0_count = 0usize;
|
||||||
let mut next = 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 chunk = read_exact_at(file, chunk_offset, chunk_length)?;
|
||||||
let data: &[u8] = &chunk;
|
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;
|
||||||
|
}
|
||||||
|
next += 1;
|
||||||
|
}
|
||||||
|
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<HeaderMessage>,
|
||||||
|
spans: &mut ChunkSpans,
|
||||||
|
) -> Result<usize, FormatError> {
|
||||||
let end = data.len();
|
let end = data.len();
|
||||||
let mut pos = 0usize;
|
let mut pos = 0usize;
|
||||||
let mut count = 0usize;
|
let mut count = 0usize;
|
||||||
@@ -295,20 +316,12 @@ impl ObjectHeader {
|
|||||||
// messages, no signature); check_message has checked the body.
|
// messages, no signature); check_message has checked the body.
|
||||||
if msg_type == MessageType::ObjectHeaderContinuation {
|
if msg_type == MessageType::ObjectHeaderContinuation {
|
||||||
let cont_offset = read_offset(body, 0, offset_size)?;
|
let cont_offset = read_offset(body, 0, offset_size)?;
|
||||||
let cont_length =
|
let cont_length = to_usize(read_offset(body, offset_size as usize, length_size)?)?;
|
||||||
to_usize(read_offset(body, offset_size as usize, length_size)?)?;
|
|
||||||
spans.add(cont_offset, cont_length)?;
|
spans.add(cont_offset, cont_length)?;
|
||||||
queue.push((cont_offset, cont_length));
|
|
||||||
}
|
}
|
||||||
pos += msg_data_size;
|
pos += msg_data_size;
|
||||||
}
|
}
|
||||||
// Only the first chunk's messages are held to the prefix count.
|
Ok(count)
|
||||||
if next == 0 {
|
|
||||||
chunk0_count = count;
|
|
||||||
}
|
|
||||||
next += 1;
|
|
||||||
}
|
|
||||||
Ok(chunk0_count)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
fn parse_v2<S: Storage + ?Sized>(
|
fn parse_v2<S: Storage + ?Sized>(
|
||||||
@@ -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).
|
/// Size of a version-1 message header: type(2) + size(2) + flags(1) + reserved(3).
|
||||||
const V1_MSG_HEADER_SIZE: usize = 8;
|
const V1_MSG_HEADER_SIZE: usize = 8;
|
||||||
|
|
||||||
/// The chunks of one object header read so far. A chunk starting where
|
/// The chunks of one object header read so far, in the order they were
|
||||||
/// another did is a cycle. Chunks of a valid header do not overlap, so
|
/// 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
|
/// 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
|
/// 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
|
/// (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
|
/// the file over and over). Overlap itself is not refused: libhdf5 reads
|
||||||
/// such headers (`cve-2025-7067.h5` has one).
|
/// 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 {
|
struct ChunkSpans {
|
||||||
starts: BTreeSet<u64>,
|
inline: [(u64, usize); INLINE_CHUNKS],
|
||||||
|
/// Chunks after the first [`INLINE_CHUNKS`], and every chunk start.
|
||||||
|
spill: Option<Box<SpilledSpans>>,
|
||||||
|
/// How many chunks there are.
|
||||||
|
len: usize,
|
||||||
/// Bytes of the chunks so far, and the most they may add up to.
|
/// Bytes of the chunks so far, and the most they may add up to.
|
||||||
total: u64,
|
total: u64,
|
||||||
budget: u64,
|
budget: u64,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// The chunks of a [`ChunkSpans`] beyond its inline ones.
|
||||||
|
struct SpilledSpans {
|
||||||
|
chunks: Vec<(u64, usize)>,
|
||||||
|
starts: BTreeSet<u64>,
|
||||||
|
}
|
||||||
|
|
||||||
|
/// How many chunks [`ChunkSpans`] holds without allocating.
|
||||||
|
const INLINE_CHUNKS: usize = 8;
|
||||||
|
|
||||||
impl ChunkSpans {
|
impl ChunkSpans {
|
||||||
|
#[inline]
|
||||||
fn new(file_len: u64, start: u64, len: usize) -> Result<Self, FormatError> {
|
fn new(file_len: u64, start: u64, len: usize) -> Result<Self, FormatError> {
|
||||||
let mut s = Self {
|
let mut s = Self {
|
||||||
starts: BTreeSet::new(),
|
inline: [(0, 0); INLINE_CHUNKS],
|
||||||
|
spill: None,
|
||||||
|
len: 0,
|
||||||
total: 0,
|
total: 0,
|
||||||
budget: file_len,
|
budget: file_len,
|
||||||
};
|
};
|
||||||
@@ -626,11 +662,19 @@ impl ChunkSpans {
|
|||||||
Ok(s)
|
Ok(s)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Record the chunk `len` bytes at `start`.
|
||||||
|
#[inline]
|
||||||
fn add(&mut self, start: u64, len: usize) -> Result<(), FormatError> {
|
fn add(&mut self, start: u64, len: usize) -> Result<(), FormatError> {
|
||||||
if !self.starts.insert(start) || self.starts.len() > MAX_V1_CHUNKS {
|
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);
|
return Err(FormatError::NestingDepthExceeded);
|
||||||
}
|
}
|
||||||
self.total = self.total.saturating_add(len as u64);
|
self.inline[self.len] = (start, len);
|
||||||
|
} else {
|
||||||
|
self.add_spilled(start, len)?;
|
||||||
|
}
|
||||||
|
self.len += 1;
|
||||||
if self.total > self.budget {
|
if self.total > self.budget {
|
||||||
return Err(FormatError::InvalidObjectHeader(
|
return Err(FormatError::InvalidObjectHeader(
|
||||||
"object header chunks larger than the file",
|
"object header chunks larger than the file",
|
||||||
@@ -638,6 +682,33 @@ impl ChunkSpans {
|
|||||||
}
|
}
|
||||||
Ok(())
|
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;
|
/// Most chunks a version-1 object header may have (malformed-data guard;
|
||||||
@@ -1105,11 +1176,18 @@ mod tests {
|
|||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn v1_continuation_cycles_are_refused() {
|
fn v1_continuation_cycles_are_refused() {
|
||||||
let data = v1_chain(5, true);
|
// Within the inline chunk list, and past it (the cycle returns to
|
||||||
assert!(matches!(
|
// 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),
|
ObjectHeader::parse(&data, 0, 8, 8),
|
||||||
Err(FormatError::NestingDepthExceeded)
|
Err(FormatError::NestingDepthExceeded)
|
||||||
));
|
),
|
||||||
|
"{n} chunks"
|
||||||
|
);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
|
|||||||
Reference in New Issue
Block a user