diff --git a/crates/clawhdf5-format/src/attribute.rs b/crates/clawhdf5-format/src/attribute.rs index 815719e..8724d7e 100644 --- a/crates/clawhdf5-format/src/attribute.rs +++ b/crates/clawhdf5-format/src/attribute.rs @@ -51,7 +51,7 @@ impl AttributeMessage { /// /// `length_size` is needed for dataspace dimension parsing. pub fn parse(data: &[u8], length_size: u8) -> Result { - Self::parse_impl(data, length_size, None) + Self::parse_impl(data, length_size, None::<(&[u8], u8)>) } /// [`AttributeMessage::parse`] with access to the rest of the file, which @@ -66,24 +66,24 @@ impl AttributeMessage { offset_size: u8, length_size: u8, ) -> Result { - Self::parse_in_storage(data, &file_data, offset_size, length_size) + Self::parse_in_storage(data, file_data, offset_size, length_size) } /// [`AttributeMessage::parse_in_file`] with the file behind any /// [`Storage`]. - pub fn parse_in_storage( + pub fn parse_in_storage( data: &[u8], - file: &dyn Storage, + file: &S, offset_size: u8, length_size: u8, ) -> Result { Self::parse_impl(data, length_size, Some((file, offset_size))) } - fn parse_impl( + fn parse_impl( data: &[u8], length_size: u8, - file: Option<(&dyn Storage, u8)>, + file: Option<(&S, u8)>, ) -> Result { ensure_len(data, 0, 2)?; let version = data[0]; @@ -98,12 +98,12 @@ impl AttributeMessage { /// The bytes of an embedded datatype/dataspace message, following the /// shared-message reference when `shared` is set. - fn embedded_message<'a>( + fn embedded_message<'a, S: Storage + ?Sized>( bytes: &'a [u8], shared: bool, msg_type: MessageType, length_size: u8, - file: Option<(&dyn Storage, u8)>, + file: Option<(&S, u8)>, ) -> Result, FormatError> { if !shared { return Ok(Cow::Borrowed(bytes)); @@ -155,10 +155,10 @@ impl AttributeMessage { }) } - fn parse_v2( + fn parse_v2( data: &[u8], length_size: u8, - file: Option<(&dyn Storage, u8)>, + file: Option<(&S, u8)>, ) -> Result { // Flags: bit 0 = datatype is shared, bit 1 = dataspace is shared. let flags = data.get(1).copied().unwrap_or(0); @@ -209,10 +209,10 @@ impl AttributeMessage { }) } - fn parse_v3( + fn parse_v3( data: &[u8], length_size: u8, - file: Option<(&dyn Storage, u8)>, + file: Option<(&S, u8)>, ) -> Result { // Flags: bit 0 = datatype is shared, bit 1 = dataspace is shared. let flags = data.get(1).copied().unwrap_or(0); @@ -427,15 +427,15 @@ pub fn extract_attributes_full( offset_size: u8, length_size: u8, ) -> Result, FormatError> { - extract_attributes_full_in(&file_data, header, offset_size, length_size) + extract_attributes_full_in(file_data, header, offset_size, length_size) } /// [`extract_attributes_full`] over any [`Storage`]. Dense attribute /// storage is indexed by a v2 B-tree, which is not read over [`Storage`] /// yet: on a backend without the whole file in memory an object with dense /// attributes is [`FormatError::ContiguousStorageRequired`]. -pub fn extract_attributes_full_in( - file: &dyn Storage, +pub fn extract_attributes_full_in( + file: &S, header: &ObjectHeader, offset_size: u8, length_size: u8, @@ -457,13 +457,13 @@ pub fn extract_attributes_tolerant( offset_size: u8, length_size: u8, ) -> Result<(Vec, Vec), FormatError> { - extract_attributes_tolerant_in(&file_data, header, offset_size, length_size) + extract_attributes_tolerant_in(file_data, header, offset_size, length_size) } /// [`extract_attributes_tolerant`] over any [`Storage`] (see /// [`extract_attributes_full_in`] for dense storage). -pub fn extract_attributes_tolerant_in( - file_data: &dyn Storage, +pub fn extract_attributes_tolerant_in( + file_data: &S, header: &ObjectHeader, offset_size: u8, length_size: u8, @@ -478,8 +478,8 @@ pub fn extract_attributes_tolerant_in( /// Read every attribute; each one that fails goes to `on_error`, which /// either stops the read (returns the error) or skips that attribute. -fn extract_attributes_with( - file_data: &dyn Storage, +fn extract_attributes_with( + file_data: &S, header: &ObjectHeader, offset_size: u8, length_size: u8, @@ -572,8 +572,8 @@ fn find_attribute_info( /// Extract attributes from dense storage (fractal heap + B-tree v2), and /// each one's creation order into `orders`. #[allow(clippy::too_many_arguments)] -fn extract_dense_attributes( - file_data: &dyn Storage, +fn extract_dense_attributes( + file_data: &S, attr_info: &AttributeInfoMessage, fh_addr: u64, offset_size: u8, diff --git a/crates/clawhdf5-format/src/btree_v1.rs b/crates/clawhdf5-format/src/btree_v1.rs index 3378131..4853ca1 100644 --- a/crates/clawhdf5-format/src/btree_v1.rs +++ b/crates/clawhdf5-format/src/btree_v1.rs @@ -77,13 +77,13 @@ impl BTreeV1Node { offset_size: u8, length_size: u8, ) -> Result { - Self::parse_in(&file_data, offset as u64, offset_size, length_size) + Self::parse_in(file_data, offset as u64, offset_size, length_size) } /// [`Self::parse`] over any [`Storage`]: one read of the node's header, /// one of its keys and children. - pub fn parse_in( - file: &dyn Storage, + pub fn parse_in( + file: &S, offset: u64, offset_size: u8, _length_size: u8, @@ -126,24 +126,22 @@ impl BTreeV1Node { let needed = eu * (key_size + os) + key_size; // eu children + (eu+1) keys let body = read_exact_at(file, body_start, needed)?; let file_data: &[u8] = &body; - let mut pos = 0usize; let mut keys = Vec::with_capacity(eu + 1); let mut children = Vec::with_capacity(eu); - for _i in 0..eu { - // key[i] - let key = read_offset(file_data, pos, offset_size)?; - keys.push(key); - pos += key_size; - // child[i] - let child = read_offset(file_data, pos, offset_size)?; - children.push(child); - pos += os; + if os == 0 { + // What reading the first key reports (and keeps `chunks_exact` + // below from being given a zero size). + return Err(FormatError::InvalidOffsetSize(offset_size)); } - // final key - let key = read_offset(file_data, pos, offset_size)?; - keys.push(key); + // `needed` bytes: key[0], child[0], ..., child[eu - 1], key[eu]. + let (pairs, last) = file_data.split_at(eu * (key_size + os)); + for pair in pairs.chunks_exact(key_size + os) { + keys.push(read_offset(pair, 0, offset_size)?); + children.push(read_offset(pair, key_size, offset_size)?); + } + keys.push(read_offset(last, 0, offset_size)?); Ok(BTreeV1Node { node_type, @@ -167,12 +165,12 @@ pub fn collect_symbol_table_nodes( offset_size: u8, length_size: u8, ) -> Result, FormatError> { - collect_symbol_table_nodes_in(&file_data, btree_address, offset_size, length_size) + collect_symbol_table_nodes_in(file_data, btree_address, offset_size, length_size) } /// [`collect_symbol_table_nodes`] over any [`Storage`]: two reads per node. -pub fn collect_symbol_table_nodes_in( - file: &dyn Storage, +pub fn collect_symbol_table_nodes_in( + file: &S, btree_address: u64, offset_size: u8, length_size: u8, @@ -180,8 +178,8 @@ pub fn collect_symbol_table_nodes_in( collect_symbol_table_nodes_inner(file, btree_address, offset_size, length_size, 0) } -fn collect_symbol_table_nodes_inner( - file: &dyn Storage, +fn collect_symbol_table_nodes_inner( + file: &S, btree_address: u64, offset_size: u8, length_size: u8, diff --git a/crates/clawhdf5-format/src/data_layout.rs b/crates/clawhdf5-format/src/data_layout.rs index 480f459..e4873c5 100644 --- a/crates/clawhdf5-format/src/data_layout.rs +++ b/crates/clawhdf5-format/src/data_layout.rs @@ -311,14 +311,14 @@ impl DataLayout { file_data: &[u8], length_size: u8, ) -> Result<(), FormatError> { - self.resolve_vds_mappings_in(&file_data, length_size) + self.resolve_vds_mappings_in(file_data, length_size) } /// [`Self::resolve_vds_mappings`] over any [`Storage`]: one read of the /// global heap collection holding the mappings. - pub fn resolve_vds_mappings_in( + pub fn resolve_vds_mappings_in( &mut self, - file_data: &dyn Storage, + file_data: &S, length_size: u8, ) -> Result<(), FormatError> { if let DataLayout::Virtual { diff --git a/crates/clawhdf5-format/src/extensible_array.rs b/crates/clawhdf5-format/src/extensible_array.rs index 529955d..a95c124 100644 --- a/crates/clawhdf5-format/src/extensible_array.rs +++ b/crates/clawhdf5-format/src/extensible_array.rs @@ -121,12 +121,12 @@ impl ExtensibleArrayHeader { offset_size: u8, length_size: u8, ) -> Result { - Self::parse_in(&file_data, offset as u64, offset_size, length_size) + Self::parse_in(file_data, offset as u64, offset_size, length_size) } /// [`Self::parse`] over any [`Storage`]: one read of the header. - pub fn parse_in( - file: &dyn Storage, + pub fn parse_in( + file: &S, offset: u64, offset_size: u8, length_size: u8, @@ -304,8 +304,8 @@ fn page_nelmts(header: &ExtensibleArrayHeader) -> Option { /// paged. The bitmap lives in the super block, not here — a paged data block /// stores only its prefix, then one slot per page. #[allow(clippy::too_many_arguments)] -fn read_data_block_elements( - file: &dyn Storage, +fn read_data_block_elements( + file: &S, db_offset: u64, nelmts: usize, header: &ExtensibleArrayHeader, @@ -448,8 +448,8 @@ pub fn read_extensible_array_chunks( /// index block's prefix, one of the whole index block, and the same for /// every super block and data block it references. #[allow(clippy::too_many_arguments)] -pub fn read_extensible_array_chunks_in( - file: &dyn Storage, +pub fn read_extensible_array_chunks_in( + file: &S, header: &ExtensibleArrayHeader, dataset_dims: &[u64], max_dims: Option<&[u64]>, @@ -642,8 +642,8 @@ pub fn read_extensible_array_chunks_in( /// + block offset + the page-init bitmap for every data block it owns /// + one address per data block + checksum. #[allow(clippy::too_many_arguments)] -fn read_super_block( - file: &dyn Storage, +fn read_super_block( + file: &S, sb_offset: u64, ndblks: usize, dblk_nelmts: usize, diff --git a/crates/clawhdf5-format/src/fixed_array.rs b/crates/clawhdf5-format/src/fixed_array.rs index de936a7..e6be63b 100644 --- a/crates/clawhdf5-format/src/fixed_array.rs +++ b/crates/clawhdf5-format/src/fixed_array.rs @@ -92,12 +92,12 @@ impl FixedArrayHeader { offset_size: u8, length_size: u8, ) -> Result { - Self::parse_in(&file_data, offset as u64, offset_size, length_size) + Self::parse_in(file_data, offset as u64, offset_size, length_size) } /// [`Self::parse`] over any [`Storage`]: one read of the header. - pub fn parse_in( - file: &dyn Storage, + pub fn parse_in( + file: &S, offset: u64, offset_size: u8, length_size: u8, @@ -174,8 +174,8 @@ pub fn read_fixed_array_chunks( /// [`read_fixed_array_chunks`] over any [`Storage`]: one read of the data /// block's prefix, one of the whole data block (pages included). #[allow(clippy::too_many_arguments)] -pub fn read_fixed_array_chunks_in( - file: &dyn Storage, +pub fn read_fixed_array_chunks_in( + file: &S, header: &FixedArrayHeader, dataset_dims: &[u64], max_dims: Option<&[u64]>, diff --git a/crates/clawhdf5-format/src/fractal_heap.rs b/crates/clawhdf5-format/src/fractal_heap.rs index 1c6a77c..e47c2c9 100644 --- a/crates/clawhdf5-format/src/fractal_heap.rs +++ b/crates/clawhdf5-format/src/fractal_heap.rs @@ -139,13 +139,13 @@ impl FractalHeapHeader { offset_size: u8, length_size: u8, ) -> Result { - Self::parse_in(&file_data, offset as u64, offset_size, length_size) + Self::parse_in(file_data, offset as u64, offset_size, length_size) } /// [`Self::parse`] over any [`Storage`]: one read of the header (two /// when it holds an I/O filter pipeline). - pub fn parse_in( - file: &dyn Storage, + pub fn parse_in( + file: &S, offset: u64, offset_size: u8, length_size: u8, @@ -382,15 +382,15 @@ impl FractalHeapHeader { id_bytes: &[u8], offset_size: u8, ) -> Result, FormatError> { - self.read_managed_object_in(&file_data, id_bytes, offset_size) + self.read_managed_object_in(file_data, id_bytes, offset_size) } /// [`Self::read_managed_object`] over any [`Storage`]. A huge object /// found through the huge-object v2 B-tree still needs the whole file /// in memory ([`FormatError::ContiguousStorageRequired`] otherwise). - pub fn read_managed_object_in( + pub fn read_managed_object_in( &self, - file_data: &dyn Storage, + file_data: &S, id_bytes: &[u8], offset_size: u8, ) -> Result, FormatError> { @@ -423,7 +423,11 @@ impl FractalHeapHeader { } /// Read a huge object (heap ID type 1). - fn read_huge_object(&self, file: &dyn Storage, id: &[u8]) -> Result, FormatError> { + fn read_huge_object( + &self, + file: &S, + id: &[u8], + ) -> Result, FormatError> { let os = usize::from(self.offset_size); let ls = usize::from(self.length_size); // (address, stored length, filter mask, decoded length); the last two @@ -475,9 +479,9 @@ impl FractalHeapHeader { /// Look up huge object `key` in the huge-object v2 B-tree, returning /// (address, stored length, filter mask, decoded length). - fn find_huge_record( + fn find_huge_record( &self, - file: &dyn Storage, + file: &S, key: u64, ) -> Result<(u64, u64, u32, u64), FormatError> { if is_undefined(self.huge_btree_address, self.offset_size) { @@ -554,9 +558,9 @@ impl FractalHeapHeader { } /// Read a managed object (heap ID type 0). - fn read_heap_managed( + fn read_heap_managed( &self, - file_data: &dyn Storage, + file_data: &S, id_bytes: &[u8], offset_size: u8, ) -> Result, FormatError> { @@ -604,9 +608,9 @@ impl FractalHeapHeader { /// header), so we just add it to the block address minus the block's heap /// offset. A filtered heap stores each direct block (header included) /// through its filter pipeline, so the block is decoded first. - fn read_from_direct_block( + fn read_from_direct_block( &self, - file: &dyn Storage, + file: &S, block: DirectBlock, target_offset: u64, length: usize, @@ -645,9 +649,9 @@ impl FractalHeapHeader { /// Read an object by traversing an indirect block to find the right direct block. #[allow(clippy::too_many_arguments)] - fn read_from_indirect_block( + fn read_from_indirect_block( &self, - file: &dyn Storage, + file: &S, iblock_addr: usize, nrows: u16, iblock_heap_offset: u64, diff --git a/crates/clawhdf5-format/src/global_heap.rs b/crates/clawhdf5-format/src/global_heap.rs index e635d4f..9861e8f 100644 --- a/crates/clawhdf5-format/src/global_heap.rs +++ b/crates/clawhdf5-format/src/global_heap.rs @@ -99,13 +99,13 @@ impl GlobalHeapCollection { offset: usize, length_size: u8, ) -> Result { - Self::parse_in(&file_data, offset as u64, length_size) + Self::parse_in(file_data, offset as u64, length_size) } /// [`Self::parse`] over any [`Storage`]: one read of the header, one of /// the collection. - pub fn parse_in( - file: &dyn Storage, + pub fn parse_in( + file: &S, offset: u64, length_size: u8, ) -> Result { @@ -136,13 +136,13 @@ impl GlobalHeapCollection { offset: usize, length_size: u8, ) -> Result { - Self::parse_index_in(&file_data, offset as u64, length_size) + Self::parse_index_in(file_data, offset as u64, length_size) } /// [`Self::parse_index`] over any [`Storage`]: one read of the header, /// one of the collection. The object offsets are file offsets. - pub fn parse_index_in( - file: &dyn Storage, + pub fn parse_index_in( + file: &S, offset: u64, length_size: u8, ) -> Result { @@ -152,8 +152,8 @@ impl GlobalHeapCollection { /// Read the collection at `offset` and index its objects: the /// collection's bytes, its offset as a `usize`, and the index (with /// file offsets). - fn read_collection( - file: &dyn Storage, + fn read_collection( + file: &S, offset: u64, length_size: u8, ) -> Result<(Cow<'_, [u8]>, usize, GlobalHeapIndex), FormatError> { diff --git a/crates/clawhdf5-format/src/local_heap.rs b/crates/clawhdf5-format/src/local_heap.rs index a4e3480..952a9d4 100644 --- a/crates/clawhdf5-format/src/local_heap.rs +++ b/crates/clawhdf5-format/src/local_heap.rs @@ -44,12 +44,12 @@ impl LocalHeap { offset_size: u8, length_size: u8, ) -> Result { - Self::parse_in(&file_data, offset as u64, offset_size, length_size) + Self::parse_in(file_data, offset as u64, offset_size, length_size) } /// [`Self::parse`] over any [`Storage`]: one read of the header. - pub fn parse_in( - file: &dyn Storage, + pub fn parse_in( + file: &S, offset: u64, offset_size: u8, length_size: u8, @@ -97,14 +97,14 @@ impl LocalHeap { /// The end of the list is `H5HL_FREE_NULL` (1); an all-ones value (the /// undefined address) is accepted as "no free list" too. pub fn validate_free_list(&self, file_data: &[u8], length_size: u8) -> Result<(), FormatError> { - self.validate_free_list_in(&file_data, length_size) + self.validate_free_list_in(file_data, length_size) } /// [`Self::validate_free_list`] over any [`Storage`]: two small reads /// per free block. - pub fn validate_free_list_in( + pub fn validate_free_list_in( &self, - file: &dyn Storage, + file: &S, length_size: u8, ) -> Result<(), FormatError> { const FREE_NULL: u64 = 1; @@ -149,14 +149,14 @@ impl LocalHeap { /// Read a null-terminated string from the heap's data segment at the given byte offset. pub fn read_string(&self, file_data: &[u8], string_offset: u64) -> Result { - self.read_string_in(&file_data, string_offset) + self.read_string_in(file_data, string_offset) } /// [`Self::read_string`] over any [`Storage`]: one read, from the /// string to the end of the data segment. - pub fn read_string_in( + pub fn read_string_in( &self, - file: &dyn Storage, + file: &S, string_offset: u64, ) -> Result { let file_len = len_usize(file); diff --git a/crates/clawhdf5-format/src/object_header.rs b/crates/clawhdf5-format/src/object_header.rs index 1fa4b41..4c21eb1 100644 --- a/crates/clawhdf5-format/src/object_header.rs +++ b/crates/clawhdf5-format/src/object_header.rs @@ -7,7 +7,7 @@ use byteorder::{ByteOrder, LittleEndian}; use crate::error::FormatError; use crate::message_type::MessageType; -use crate::storage::{Storage, len_usize, read_exact_at, read_upto}; +use crate::storage::{Storage, Window, len_usize, read_exact_at}; /// OHDR signature for v2 object headers. const OHDR_SIGNATURE: [u8; 4] = *b"OHDR"; @@ -119,36 +119,43 @@ impl ObjectHeader { offset_size: u8, length_size: u8, ) -> Result { - Self::parse_in(&data, offset as u64, offset_size, length_size) + Self::parse_in(data, offset as u64, offset_size, length_size) } /// [`Self::parse`] over any [`Storage`]. /// - /// Reads the signature, the prefix (at most [`V2_PREFIX_MAX`] bytes), - /// then each chunk as one bounded read, continuation chunks included. - pub fn parse_in( - file: &dyn Storage, + /// Reads the prefix (at most [`V2_PREFIX_MAX`] bytes, signature + /// included), then each chunk as one bounded read, continuation chunks + /// included. + pub fn parse_in( + file: &S, offset: u64, offset_size: u8, length_size: u8, ) -> Result { - let sig = read_exact_at(file, offset, 4)?; - if *sig == OHDR_SIGNATURE { - Self::parse_v2(file, offset, offset_size, length_size) + // The longest prefix of either version, in one read. It holds the + // whole prefix or ends at the end of the file, so its bounds checks + // are the whole-file ones. + let prefix = Window::read(file, offset, V2_PREFIX_MAX)?; + prefix.ensure(0, 4)?; + if prefix.bytes[..4] == OHDR_SIGNATURE { + Self::parse_v2(file, offset, &prefix, offset_size, length_size) } else { - Self::parse_v1(file, offset, offset_size, length_size) + Self::parse_v1(file, offset, &prefix, offset_size, length_size) } } - fn parse_v1( - file: &dyn Storage, + fn parse_v1( + file: &S, offset: u64, + prefix: &Window<'_>, offset_size: u8, length_size: u8, ) -> Result { // version(1) + reserved(1) + num_messages(2) + ref_count(4) + header_size(4) = 12 // then pad to 8-byte alignment from start of header - let prefix = read_exact_at(file, offset, 12)?; + prefix.ensure(0, 12)?; + let prefix = &prefix.bytes[..12]; let version = prefix[0]; if version != 1 { @@ -179,7 +186,9 @@ impl ObjectHeader { })?; // parse_v1_chunk reads the chunk, with the bounds check that was here. - let mut messages = Vec::new(); + // The prefix's count (NIL messages included, capped: it is untrusted) + // sizes the list once instead of growing it message by message. + let mut messages = Vec::with_capacity(num_messages.min(64)); let chunk0_count = Self::parse_v1_chunk( file, msg_start, @@ -221,8 +230,8 @@ impl ObjectHeader { /// end of the chunk, or leftover bytes too few for a message header (a /// "gap", which only version 2 allows). #[allow(clippy::too_many_arguments)] - fn parse_v1_chunk( - file: &dyn Storage, + fn parse_v1_chunk( + file: &S, offset: u64, length: usize, offset_size: u8, @@ -295,31 +304,21 @@ impl ObjectHeader { Ok(count) } - fn parse_v2( - file: &dyn Storage, + fn parse_v2( + file: &S, offset: u64, + prefix: &Window<'_>, offset_size: u8, length_size: u8, ) -> Result { - // The prefix, read as one window. The window holds the whole prefix - // or ends at the end of the file, so a position past the window is - // past the end of the file: `ensure_len` checks positions relative - // to the header against it and reports them as the whole-file check - // did, with absolute positions and the file's length. - let window = read_upto(file, offset, V2_PREFIX_MAX)?; - let data: &[u8] = &window; + // `ensure_len` checks positions relative to the header against the + // prefix window and reports them as the whole-file check did, with + // absolute positions and the file's length. + let data: &[u8] = &prefix.bytes; let file_len = len_usize(file); let base = usize::try_from(offset).unwrap_or(usize::MAX); let abs = |rel: usize| base.saturating_add(rel); - let ensure_len = |_: &[u8], rel: usize, needed: usize| -> Result<(), FormatError> { - match rel.checked_add(needed) { - Some(end) if end <= data.len() => Ok(()), - _ => Err(FormatError::UnexpectedEof { - expected: abs(rel).saturating_add(needed), - available: file_len, - }), - } - }; + let ensure_len = |_: &[u8], rel: usize, needed: usize| prefix.ensure(rel, needed); let offset = 0usize; // signature(4) + version(1) + flags(1) = 6 ensure_len(data, offset, 6)?; @@ -533,8 +532,8 @@ impl ObjectHeader { } #[allow(clippy::too_many_arguments)] - fn parse_v2_continuation( - file: &dyn Storage, + fn parse_v2_continuation( + file: &S, offset: u64, length: usize, has_creation_order: bool, @@ -1328,7 +1327,7 @@ mod tests { /// Every header, and every truncation of it, parses to the same result /// (or the same error) through a `read_at`-only storage as from a slice; - /// a header in one chunk takes three reads (signature, prefix, chunk). + /// a header in one chunk takes two reads (prefix, chunk). #[test] fn parse_in_matches_slice_parse() { use crate::storage::CountingStorage; @@ -1375,6 +1374,6 @@ mod tests { let one_chunk = build_v2_header(0x00, &[(0x01, &[42], 0)], None); let storage = CountingStorage::new(one_chunk); ObjectHeader::parse_in(&storage, 0, 8, 8).unwrap(); - assert_eq!(storage.reads(), 3); + assert_eq!(storage.reads(), 2); } } diff --git a/crates/clawhdf5-format/src/shared_message.rs b/crates/clawhdf5-format/src/shared_message.rs index 5c8b1cb..05ee649 100644 --- a/crates/clawhdf5-format/src/shared_message.rs +++ b/crates/clawhdf5-format/src/shared_message.rs @@ -254,13 +254,13 @@ pub fn parse_sohm_table( nindexes: u8, offset_size: u8, ) -> Result { - parse_sohm_table_in(&file_data, table_addr as u64, nindexes, offset_size) + parse_sohm_table_in(file_data, table_addr as u64, nindexes, offset_size) } /// [`parse_sohm_table`] over any [`Storage`]: one read of the signature, /// one of every index entry. -pub fn parse_sohm_table_in( - file: &dyn Storage, +pub fn parse_sohm_table_in( + file: &S, table_addr: u64, nindexes: u8, offset_size: u8, @@ -384,13 +384,13 @@ pub fn parse_sohm_list( num_messages: u16, offset_size: u8, ) -> Result, FormatError> { - parse_sohm_list_in(&file_data, list_addr as u64, num_messages, offset_size) + parse_sohm_list_in(file_data, list_addr as u64, num_messages, offset_size) } /// [`parse_sohm_list`] over any [`Storage`]: one read of the signature, one /// of every entry. -pub fn parse_sohm_list_in( - file: &dyn Storage, +pub fn parse_sohm_list_in( + file: &S, list_addr: u64, num_messages: u16, offset_size: u8, @@ -420,14 +420,14 @@ pub fn parse_sohm_btree_entries( offset_size: u8, length_size: u8, ) -> Result, FormatError> { - parse_sohm_btree_entries_in(&file_data, btree_addr as u64, offset_size, length_size) + parse_sohm_btree_entries_in(file_data, btree_addr as u64, offset_size, length_size) } /// [`parse_sohm_btree_entries`] over any [`Storage`]. The v2 B-tree is not /// read over [`Storage`] yet, so this needs the whole file in memory /// ([`FormatError::ContiguousStorageRequired`] otherwise). -pub fn parse_sohm_btree_entries_in( - file: &dyn Storage, +pub fn parse_sohm_btree_entries_in( + file: &S, btree_addr: u64, offset_size: u8, length_size: u8, @@ -455,12 +455,12 @@ pub fn load_sohm_table( offset_size: u8, length_size: u8, ) -> Result, FormatError> { - load_sohm_table_in(&file_data, offset_size, length_size) + load_sohm_table_in(file_data, offset_size, length_size) } /// [`load_sohm_table`] over any [`Storage`]. -pub fn load_sohm_table_in( - file_data: &dyn Storage, +pub fn load_sohm_table_in( + file_data: &S, offset_size: u8, length_size: u8, ) -> Result, FormatError> { @@ -498,12 +498,12 @@ pub fn message_data_with_sohm<'a>( offset_size: u8, length_size: u8, ) -> Result, FormatError> { - message_data_with_sohm_in(&file_data, msg, offset_size, length_size) + message_data_with_sohm_in(file_data, msg, offset_size, length_size) } /// [`message_data_with_sohm`] over any [`Storage`]. -pub fn message_data_with_sohm_in<'a>( - file_data: &dyn Storage, +pub fn message_data_with_sohm_in<'a, S: Storage + ?Sized>( + file_data: &S, msg: &'a crate::object_header::HeaderMessage, offset_size: u8, length_size: u8, @@ -568,8 +568,8 @@ pub fn resolve_sohm_message( } /// [`resolve_sohm_message`] over any [`Storage`]. -pub fn resolve_sohm_message_in( - file_data: &dyn Storage, +pub fn resolve_sohm_message_in( + file_data: &S, heap_id: &[u8; FHEAP_ID_LEN], sohm_table: &SohmTable, target_msg_type: MessageType, @@ -603,12 +603,12 @@ pub fn message_data<'a>( offset_size: u8, length_size: u8, ) -> Result, FormatError> { - message_data_in(&file_data, msg, offset_size, length_size) + message_data_in(file_data, msg, offset_size, length_size) } /// [`message_data`] over any [`Storage`]. -pub fn message_data_in<'a>( - file_data: &dyn Storage, +pub fn message_data_in<'a, S: Storage + ?Sized>( + file_data: &S, msg: &'a crate::object_header::HeaderMessage, offset_size: u8, length_size: u8, @@ -650,8 +650,8 @@ pub fn resolve_shared_message( } /// [`resolve_shared_message`] over any [`Storage`]. -pub fn resolve_shared_message_in( - file_data: &dyn Storage, +pub fn resolve_shared_message_in( + file_data: &S, shared_ref: &SharedMessageRef, target_msg_type: MessageType, offset_size: u8, @@ -692,8 +692,8 @@ pub fn resolve_shared_message_with_sohm( } /// [`resolve_shared_message_with_sohm`] over any [`Storage`]. -pub fn resolve_shared_message_with_sohm_in( - file_data: &dyn Storage, +pub fn resolve_shared_message_with_sohm_in( + file_data: &S, shared_ref: &SharedMessageRef, target_msg_type: MessageType, offset_size: u8, diff --git a/crates/clawhdf5-format/src/signature.rs b/crates/clawhdf5-format/src/signature.rs index 4b152ee..3a2e5a1 100644 --- a/crates/clawhdf5-format/src/signature.rs +++ b/crates/clawhdf5-format/src/signature.rs @@ -42,7 +42,7 @@ pub fn find_signature(data: &[u8]) -> Result { /// [`find_signature`] over any [`Storage`]: one 8-byte read per candidate /// offset. -pub fn find_signature_in(file: &dyn Storage) -> Result { +pub fn find_signature_in(file: &S) -> Result { let len = file.len(); let mut offset = 0u64; while offset.checked_add(8).is_some_and(|end| end <= len) { diff --git a/crates/clawhdf5-format/src/storage.rs b/crates/clawhdf5-format/src/storage.rs index 7d68eeb..6d09e7e 100644 --- a/crates/clawhdf5-format/src/storage.rs +++ b/crates/clawhdf5-format/src/storage.rs @@ -10,8 +10,15 @@ //! `impl Storage for [u8]` serves the in-memory case with no copy, and //! [`Storage::as_contiguous`] lets a hot loop borrow the whole file at once //! when the backend has it. Modules are converted one at a time: a converted -//! parser has an `*_in(file: &dyn Storage, ..)` core and keeps its old -//! `&[u8]` signature as a thin wrapper, so callers do not change. +//! parser has an `*_in(file: &S, ..)` core and keeps +//! its old `&[u8]` signature as a thin wrapper, so callers do not change. +//! +//! The cores are generic rather than taking `&dyn Storage` so that the +//! wrappers monomorphise for `[u8]`: the bounds check of each structure read +//! inlines to what the slice code did, with no indirect call and no copy, +//! which keeps local files as fast as before the migration. A `&dyn Storage` +//! still works (`S = dyn Storage`), and a remote backend pays one indirect +//! call per structure read. //! //! The trait is synchronous and `no_std`: parsing is CPU work, and a remote //! backend bridges to its own I/O. @@ -176,7 +183,7 @@ impl Storage for std::sync::Arc { /// `storage.len()` as the `usize` the parsers' end-of-file errors report /// (saturating on targets where the file is larger than the address space). #[inline] -pub(crate) fn len_usize(file: &dyn Storage) -> usize { +pub(crate) fn len_usize(file: &S) -> usize { usize::try_from(file.len()).unwrap_or(usize::MAX) } @@ -187,8 +194,8 @@ pub(crate) fn len_usize(file: &dyn Storage) -> usize { /// `available = storage length` — the error the `&[u8]` parsers give for /// the same bounds check (`offset + len > file_data.len()`). #[inline] -pub fn read_exact_at( - file: &dyn Storage, +pub fn read_exact_at( + file: &S, offset: u64, len: usize, ) -> Result, FormatError> { @@ -198,7 +205,8 @@ pub fn read_exact_at( .saturating_add(len), available: len_usize(file), }; - // In-memory fast path: one dynamic call, then plain slicing. + // In-memory fast path: plain slicing (for `S = [u8]` this inlines to + // the slice code's bounds check). if let Some(all) = file.as_contiguous() { return usize::try_from(offset) .ok() @@ -219,6 +227,8 @@ pub fn read_exact_at( Ok(bytes) } +#[cold] +#[inline(never)] fn short_read() -> FormatError { FormatError::Storage( "short read inside the file (the storage shrank or the backend failed)".into(), @@ -240,7 +250,11 @@ pub(crate) struct Window<'a> { impl<'a> Window<'a> { /// Read up to `max` bytes at `base`. - pub fn read(file: &'a dyn Storage, base: u64, max: usize) -> Result { + pub fn read( + file: &'a S, + base: u64, + max: usize, + ) -> Result { Ok(Window { bytes: read_upto(file, base, max)?, base: usize::try_from(base).unwrap_or(usize::MAX), @@ -259,6 +273,7 @@ impl<'a> Window<'a> { } /// Check that `[rel, rel + needed)` (relative to `base`) is in the file. + #[inline] pub fn ensure(&self, rel: usize, needed: usize) -> Result<(), FormatError> { match rel.checked_add(needed) { Some(end) if end <= self.bytes.len() => Ok(()), @@ -274,8 +289,8 @@ impl<'a> Window<'a> { /// storage. For structures whose size is only known once their prefix has /// been parsed and whose parsers bound-check what they are given. #[inline] -pub fn read_upto( - file: &dyn Storage, +pub fn read_upto( + file: &S, offset: u64, max: usize, ) -> Result, FormatError> { @@ -297,8 +312,8 @@ pub fn read_upto( /// [`Storage`] yet. On a backend without a contiguous view this is the /// clean [`FormatError::ContiguousStorageRequired`] error, never a guess. #[inline] -pub fn require_contiguous<'a>( - file: &'a dyn Storage, +pub fn require_contiguous<'a, S: Storage + ?Sized>( + file: &'a S, what: &'static str, ) -> Result<&'a [u8], FormatError> { file.as_contiguous() diff --git a/crates/clawhdf5-format/src/superblock.rs b/crates/clawhdf5-format/src/superblock.rs index 318c945..dc21f7c 100644 --- a/crates/clawhdf5-format/src/superblock.rs +++ b/crates/clawhdf5-format/src/superblock.rs @@ -166,13 +166,13 @@ impl Superblock { file_data: &[u8], signature_offset: usize, ) -> Result { - self.refresh_eof_in(&file_data, signature_offset as u64) + self.refresh_eof_in(file_data, signature_offset as u64) } /// [`Self::refresh_eof`] over any [`Storage`]. - pub fn refresh_eof_in( + pub fn refresh_eof_in( &mut self, - file: &dyn Storage, + file: &S, signature_offset: u64, ) -> Result { let refreshed = Superblock::parse_in(file, signature_offset)?; @@ -233,13 +233,16 @@ impl Superblock { /// [`FormatError::UserBlockNotStripped`] because the addresses in the /// returned superblock would otherwise be applied to the wrong bytes. pub fn parse(data: &[u8], signature_offset: usize) -> Result { - Self::parse_in(&data, signature_offset as u64) + Self::parse_in(data, signature_offset as u64) } /// [`Self::parse`] over any [`Storage`]: one read of the first /// [`SUPERBLOCK_READ_LEN`] bytes (fewer when the file is shorter, which /// is then refused with the same end-of-file errors as a short slice). - pub fn parse_in(file: &dyn Storage, signature_offset: u64) -> Result { + pub fn parse_in( + file: &S, + signature_offset: u64, + ) -> Result { if signature_offset != 0 { return Err(FormatError::UserBlockNotStripped(signature_offset)); } diff --git a/crates/clawhdf5-format/src/superblock_ext.rs b/crates/clawhdf5-format/src/superblock_ext.rs index 1a684f6..8f09aff 100644 --- a/crates/clawhdf5-format/src/superblock_ext.rs +++ b/crates/clawhdf5-format/src/superblock_ext.rs @@ -171,13 +171,13 @@ pub fn read_superblock_extension( data: &[u8], sb: &Superblock, ) -> Result, FormatError> { - read_superblock_extension_in(&data, sb) + read_superblock_extension_in(data, sb) } /// [`read_superblock_extension`] over any [`Storage`]; its length is the /// end of file. -pub fn read_superblock_extension_in( - file: &dyn Storage, +pub fn read_superblock_extension_in( + file: &S, sb: &Superblock, ) -> Result, FormatError> { let os = sb.offset_size; @@ -351,12 +351,12 @@ impl CacheImage { location: CacheImageLocation, sb: &Superblock, ) -> Result { - Self::decode_in(&data, location, sb) + Self::decode_in(data, location, sb) } /// [`Self::decode`] over any [`Storage`]: one read of the image block. - pub fn decode_in( - file: &dyn Storage, + pub fn decode_in( + file: &S, location: CacheImageLocation, sb: &Superblock, ) -> Result { @@ -483,7 +483,10 @@ impl CacheImage { } /// [`Self::block`] over any [`Storage`]. - pub fn block_in<'a>(&self, file: &'a dyn Storage) -> Result, FormatError> { + pub fn block_in<'a, S: Storage + ?Sized>( + &self, + file: &'a S, + ) -> Result, FormatError> { image_block_in(file, self.location) } @@ -511,8 +514,8 @@ fn image_block(data: &[u8], location: CacheImageLocation) -> Result<&[u8], Forma Ok(&data[start as usize..start as usize + len]) } -fn image_block_in( - file: &dyn Storage, +fn image_block_in( + file: &S, location: CacheImageLocation, ) -> Result, FormatError> { let (start, len) = image_block_range(file.len(), location)?; @@ -540,12 +543,12 @@ fn image_block_range( /// ([`CacheImage::decode`]). `data` is the file from the superblock on, up /// to its recorded end of file. pub fn cache_image_state(data: &[u8], sb: &Superblock) -> Result { - cache_image_state_in(&data, sb) + cache_image_state_in(data, sb) } /// [`cache_image_state`] over any [`Storage`]. -pub fn cache_image_state_in( - file: &dyn Storage, +pub fn cache_image_state_in( + file: &S, sb: &Superblock, ) -> Result { match read_superblock_extension_in(file, sb)? { diff --git a/crates/clawhdf5-format/src/symbol_table.rs b/crates/clawhdf5-format/src/symbol_table.rs index 8fb4453..467f43b 100644 --- a/crates/clawhdf5-format/src/symbol_table.rs +++ b/crates/clawhdf5-format/src/symbol_table.rs @@ -4,7 +4,7 @@ use alloc::vec::Vec; use crate::error::FormatError; -use crate::storage::{Storage, len_usize, read_exact_at}; +use crate::storage::{Storage, read_exact_at}; /// Symbol Table message (type 0x0011) found in v1 group object headers. #[derive(Debug, Clone, PartialEq)] @@ -80,17 +80,16 @@ impl SymbolTableNode { offset: usize, offset_size: u8, ) -> Result { - Self::parse_in(&file_data, offset as u64, offset_size) + Self::parse_in(file_data, offset as u64, offset_size) } /// [`Self::parse`] over any [`Storage`]: one read of the node's header, /// one of its entries. - pub fn parse_in( - file: &dyn Storage, + pub fn parse_in( + file: &S, offset: u64, offset_size: u8, ) -> Result { - let file_len = len_usize(file); // signature(4) + version(1) + reserved(1) + number_of_symbols(2) = 8 let header = read_exact_at(file, offset, 8)?; @@ -108,42 +107,22 @@ impl SymbolTableNode { let os = offset_size as usize; // Each entry: link_name_offset(os) + obj_hdr_addr(os) + cache_type(4) + reserved(4) + scratch(16) let entry_size = os + os + 4 + 4 + 16; - // `offset + 8` fits: the header's read checked it. - let entries_start = offset as usize + 8; - let needed = entries_start.checked_add(num_symbols * entry_size).ok_or( - FormatError::UnexpectedEof { - expected: usize::MAX, - available: file_len, - }, - )?; - if needed > file_len { - return Err(FormatError::UnexpectedEof { - expected: needed, - available: file_len, - }); - } - let body = read_exact_at(file, entries_start as u64, num_symbols * entry_size)?; + // `offset + 8` fits: the header's read checked it. The entries' + // read is the bounds check (`offset + 8 + entries > file length`, + // which cannot overflow: at most 65535 entries of 40 bytes). + let body = read_exact_at(file, offset + 8, num_symbols * entry_size)?; let file_data: &[u8] = &body; let mut entries = Vec::with_capacity(num_symbols); - let mut pos = 0usize; - for _ in 0..num_symbols { - let link_name_offset = read_offset(file_data, pos, offset_size)?; - pos += os; - let object_header_address = read_offset(file_data, pos, offset_size)?; - pos += os; - let cache_type = u32::from_le_bytes([ - file_data[pos], - file_data[pos + 1], - file_data[pos + 2], - file_data[pos + 3], - ]); - pos += 4; + for entry in file_data.chunks_exact(entry_size) { + let link_name_offset = read_offset(entry, 0, offset_size)?; + let object_header_address = read_offset(entry, os, offset_size)?; + let pos = 2 * os; + let cache_type = + u32::from_le_bytes([entry[pos], entry[pos + 1], entry[pos + 2], entry[pos + 3]]); // reserved 4 bytes - pos += 4; let mut scratch_pad = [0u8; 16]; - scratch_pad.copy_from_slice(&file_data[pos..pos + 16]); - pos += 16; + scratch_pad.copy_from_slice(&entry[pos + 8..pos + 24]); entries.push(SymbolTableEntry { link_name_offset, diff --git a/crates/clawhdf5-format/tests/fixtures/v1_groups_400.h5 b/crates/clawhdf5-format/tests/fixtures/v1_groups_400.h5 new file mode 100644 index 0000000..5237559 Binary files /dev/null and b/crates/clawhdf5-format/tests/fixtures/v1_groups_400.h5 differ diff --git a/crates/clawhdf5/Cargo.toml b/crates/clawhdf5/Cargo.toml index 1eca373..2378aa9 100644 --- a/crates/clawhdf5/Cargo.toml +++ b/crates/clawhdf5/Cargo.toml @@ -30,6 +30,10 @@ harness = false name = "parallel_bench" harness = false +[[bench]] +name = "local_metadata_bench" +harness = false + [features] default = ["mmap", "provenance", "lzf"] mmap = ["clawhdf5-io/mmap"] diff --git a/crates/clawhdf5/benches/local_metadata_bench.rs b/crates/clawhdf5/benches/local_metadata_bench.rs new file mode 100644 index 0000000..0b2af1f --- /dev/null +++ b/crates/clawhdf5/benches/local_metadata_bench.rs @@ -0,0 +1,91 @@ +//! Metadata parsing over an in-memory file: the local fast path that the +//! range-read `Storage` migration must not slow down +//! (`docs/design/range-reads.md`, "Keeping the local fast path"). +//! +//! Only the `&[u8]` APIs are used, so the same file builds against older +//! revisions for an A/B comparison. The input is a version-1 (symbol table) +//! file with 400 groups, written by h5py with `libver='earliest'` and a +//! 512-byte user block (`clawhdf5-format/tests/fixtures/v1_groups_400.h5`). + +use clawhdf5::{File, Group}; +use clawhdf5_format::btree_v1::collect_symbol_table_nodes; +use clawhdf5_format::message_type::MessageType; +use clawhdf5_format::object_header::ObjectHeader; +use clawhdf5_format::superblock::Superblock; +use clawhdf5_format::symbol_table::{SymbolTableMessage, SymbolTableNode}; +use criterion::{Criterion, criterion_group, criterion_main}; +use std::hint::black_box; + +const FIXTURE: &str = concat!( + env!("CARGO_MANIFEST_DIR"), + "/../clawhdf5-format/tests/fixtures/v1_groups_400.h5" +); + +fn walk(g: &Group<'_>, objs: &mut usize) { + for name in g.datasets().unwrap_or_default() { + *objs += 1; + if let Ok(ds) = g.dataset(&name) { + let _ = black_box(ds.shape()); + let _ = black_box(ds.dtype()); + let _ = black_box(ds.attrs()); + } + } + for name in g.groups().unwrap_or_default() { + *objs += 1; + if let Ok(sub) = g.group(&name) { + walk(&sub, objs); + } + } +} + +fn bench_local_metadata(c: &mut Criterion) { + let bytes = std::fs::read(FIXTURE).unwrap(); + let (_, f) = clawhdf5_format::signature::split_user_block(&bytes).unwrap(); + let sb = Superblock::parse(f, 0).unwrap(); + let (os, ls) = (sb.offset_size, sb.length_size); + let root = ObjectHeader::parse(f, sb.root_group_address as usize, os, ls).unwrap(); + let stm = root + .messages + .iter() + .find(|m| m.msg_type == MessageType::SymbolTable) + .map(|m| SymbolTableMessage::parse(&m.data, os).unwrap()) + .unwrap(); + let nodes = collect_symbol_table_nodes(f, stm.btree_address, os, ls).unwrap(); + let headers: Vec = nodes + .iter() + .flat_map(|&a| SymbolTableNode::parse(f, a as usize, os).unwrap().entries) + .map(|e| e.object_header_address) + .collect(); + assert_eq!(headers.len(), 401); + + let mut g = c.benchmark_group("local_metadata"); + g.bench_function("object_header_parse_x401", |b| { + b.iter(|| { + for &a in &headers { + black_box(ObjectHeader::parse(f, a as usize, os, ls).unwrap()); + } + }) + }); + g.bench_function("snod_parse_all", |b| { + b.iter(|| { + for &a in &nodes { + black_box(SymbolTableNode::parse(f, a as usize, os).unwrap()); + } + }) + }); + g.bench_function("btree_v1_walk", |b| { + b.iter(|| black_box(collect_symbol_table_nodes(f, stm.btree_address, os, ls).unwrap())) + }); + let file = File::open(FIXTURE).unwrap(); + g.bench_function("facade_list_400_groups", |b| { + b.iter(|| { + let mut n = 0; + walk(&file.root(), &mut n); + assert_eq!(n, 401); + }) + }); + g.finish(); +} + +criterion_group!(benches, bench_local_metadata); +criterion_main!(benches);