diff --git a/CHANGELOG.md b/CHANGELOG.md index 4556f91..664a831 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -149,6 +149,32 @@ inside the file is an error. - **No behaviour change for in-memory and mapped files:** with `as_contiguous()` every path slices the file as before (checked below). +- **Speed on local files** (provisional: tank was shared with other jobs, + load 4–16 during the runs; Criterion `local_metadata_bench`, `main` + `8f59b2e` and this branch as separate binaries, 8 alternating rounds, + best round of each). Listing the 400-group version-1 file through the + facade (`File::open`, mmap) had become 7–10% slower than `main` (8.95–9.04 + vs 8.14–8.36 ms), from the M2 merge on (bisected over the merges: `main` + 8.17–8.27, `2893b6c` 8.69–8.76, `93e2d5f`/`7447dce`/`4313917` the same). + The parsers were unchanged: the facade now called the generic `*_in` + entry points with the slice (`with_bytes!`), which instantiates each + parser in the facade crate, where the format crate's private helpers do + not inline (no LTO); `main` called the `&[u8]` wrappers, compiled in the + format crate. Routing just `resolve_child`/`resolve_group_children` to + the wrappers took the listing from 8.66–8.70 to 8.33–8.46 ms. Fixed in + the format crate: `ObjectHeader::parse_in`, `group_v2::{resolve_child_in, + resolve_group_children_in, resolve_path_any_in}` and + `attribute::{extract_attributes_tolerant_in, find_attribute_in}` hand a + storage with `as_contiguous()` to their non-generic slice entry point, + compiled once in the format crate; everything else goes to the same + generic core as before, so `File::open_storage` is unchanged. Best + rounds, `main` / before / after: facade listing 8.49 / 9.07 / 8.68 ms + (+2.2% on `main`, was +6.9%); `ObjectHeader::parse` ×401 24.91 / 25.90 / + 25.51 µs; symbol-table nodes 1.99 / 1.94 / 1.94 µs; group B-tree walk + 360.6 / 365.0 / 361.1 ns. The B-tree walk's `btree_v1.rs` is identical to + `main` and it calls only format-crate code; its earlier +5–10% (323–355 + vs 353–363 ns) was run-to-run layout noise (at `2893b6c` it measured + 357–374 ns against `main`'s 355–371 in the same rounds). - Tests (2026-09-26, tank): - `clawhdf5-format/tests/storage_equivalence.rs` now also reads every dataset — whole, fill-aware, through a chunk cache (twice) and the diff --git a/crates/clawhdf5-format/src/attribute.rs b/crates/clawhdf5-format/src/attribute.rs index c8f69b5..8887bbb 100644 --- a/crates/clawhdf5-format/src/attribute.rs +++ b/crates/clawhdf5-format/src/attribute.rs @@ -470,16 +470,31 @@ 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_core(file_data, header, offset_size, length_size) } /// [`extract_attributes_tolerant`] over any [`Storage`] (see -/// [`extract_attributes_full_in`] for dense storage). +/// [`extract_attributes_full_in`] for dense storage). One with the whole +/// file in memory is read as the slice, by code compiled in this crate (see +/// [`crate::storage`], "Slice entry points"). +#[inline] pub fn extract_attributes_tolerant_in( file_data: &S, header: &ObjectHeader, offset_size: u8, length_size: u8, +) -> Result<(Vec, Vec), FormatError> { + match file_data.as_contiguous() { + Some(all) => extract_attributes_tolerant(all, header, offset_size, length_size), + None => extract_attributes_tolerant_core(file_data, header, offset_size, length_size), + } +} + +fn extract_attributes_tolerant_core( + file_data: &S, + header: &ObjectHeader, + offset_size: u8, + length_size: u8, ) -> Result<(Vec, Vec), FormatError> { let mut errors = Vec::new(); let attrs = extract_attributes_with(file_data, header, offset_size, length_size, &mut |e| { @@ -561,18 +576,34 @@ pub fn find_attribute_in_file( offset_size: u8, length_size: u8, ) -> Result, FormatError> { - find_attribute_in(file_data, header, name, offset_size, length_size) + find_attribute_core(file_data, header, name, offset_size, length_size) } /// [`find_attribute_in_file`] over any [`Storage`] (see /// [`extract_attributes_full_in`] for dense storage, whose name index still -/// needs the whole file in memory). +/// needs the whole file in memory). One with the whole file in memory is +/// read as the slice, by code compiled in this crate (see +/// [`crate::storage`], "Slice entry points"). +#[inline] pub fn find_attribute_in( file_data: &S, header: &ObjectHeader, name: &str, offset_size: u8, length_size: u8, +) -> Result, FormatError> { + match file_data.as_contiguous() { + Some(all) => find_attribute_in_file(all, header, name, offset_size, length_size), + None => find_attribute_core(file_data, header, name, offset_size, length_size), + } +} + +fn find_attribute_core( + file_data: &S, + header: &ObjectHeader, + name: &str, + offset_size: u8, + length_size: u8, ) -> Result, FormatError> { let attr_info = find_attribute_info(header, offset_size)?; let dense = attr_info diff --git a/crates/clawhdf5-format/src/group_v2.rs b/crates/clawhdf5-format/src/group_v2.rs index 9dda843..dbc1f08 100644 --- a/crates/clawhdf5-format/src/group_v2.rs +++ b/crates/clawhdf5-format/src/group_v2.rs @@ -368,15 +368,30 @@ pub fn resolve_child( group_address: u64, name: &str, ) -> Result { - resolve_child_in(file_data, superblock, group_address, name) + resolve_child_core(file_data, superblock, group_address, name) } -/// [`resolve_child`] over any [`Storage`]. +/// [`resolve_child`] over any [`Storage`]. One with the whole file in memory +/// is read as the slice, by code compiled in this crate (see +/// [`crate::storage`], "Slice entry points"). +#[inline] pub fn resolve_child_in( file_data: &S, superblock: &Superblock, group_address: u64, name: &str, +) -> Result { + match file_data.as_contiguous() { + Some(all) => resolve_child(all, superblock, group_address, name), + None => resolve_child_core(file_data, superblock, group_address, name), + } +} + +fn resolve_child_core( + file_data: &S, + superblock: &Superblock, + group_address: u64, + name: &str, ) -> Result { let os = superblock.offset_size; let ls = superblock.length_size; @@ -453,14 +468,28 @@ pub fn resolve_path_any( superblock: &Superblock, path: &str, ) -> Result { - resolve_path_any_in(file_data, superblock, path) + resolve_path_any_core(file_data, superblock, path) } -/// [`resolve_path_any`] over any [`Storage`]. +/// [`resolve_path_any`] over any [`Storage`]. One with the whole file in memory +/// is read as the slice, by code compiled in this crate (see +/// [`crate::storage`], "Slice entry points"). +#[inline] pub fn resolve_path_any_in( file_data: &S, superblock: &Superblock, path: &str, +) -> Result { + match file_data.as_contiguous() { + Some(all) => resolve_path_any(all, superblock, path), + None => resolve_path_any_core(file_data, superblock, path), + } +} + +fn resolve_path_any_core( + file_data: &S, + superblock: &Superblock, + path: &str, ) -> Result { resolve_path_following_links( file_data, @@ -513,14 +542,28 @@ pub fn resolve_group_children( superblock: &Superblock, group_address: u64, ) -> Result, FormatError> { - resolve_group_children_in(file_data, superblock, group_address) + resolve_group_children_core(file_data, superblock, group_address) } -/// [`resolve_group_children`] over any [`Storage`]. +/// [`resolve_group_children`] over any [`Storage`]. One with the whole file in memory +/// is read as the slice, by code compiled in this crate (see +/// [`crate::storage`], "Slice entry points"). +#[inline] pub fn resolve_group_children_in( file_data: &S, superblock: &Superblock, group_address: u64, +) -> Result, FormatError> { + match file_data.as_contiguous() { + Some(all) => resolve_group_children(all, superblock, group_address), + None => resolve_group_children_core(file_data, superblock, group_address), + } +} + +fn resolve_group_children_core( + file_data: &S, + superblock: &Superblock, + group_address: u64, ) -> Result, FormatError> { let os = superblock.offset_size; let ls = superblock.length_size; diff --git a/crates/clawhdf5-format/src/object_header.rs b/crates/clawhdf5-format/src/object_header.rs index d421ea0..afcb9c5 100644 --- a/crates/clawhdf5-format/src/object_header.rs +++ b/crates/clawhdf5-format/src/object_header.rs @@ -116,25 +116,52 @@ impl ObjectHeader { /// Parse an object header at the given offset in the data buffer. /// /// `offset_size` and `length_size` come from the superblock. + #[inline] pub fn parse( data: &[u8], offset: usize, offset_size: u8, length_size: u8, ) -> Result { - Self::parse_in(data, offset as u64, offset_size, length_size) + Self::parse_slice(data, offset as u64, offset_size, length_size) } /// [`Self::parse`] over any [`Storage`]. /// /// Reads the prefix (at most [`V2_PREFIX_MAX`] bytes, signature /// included), then each chunk as one bounded read, continuation chunks - /// included. + /// included. A storage with the whole file in memory is parsed as its + /// slice, by code compiled in this crate (see + /// [`crate::storage`], "Slice entry points"). + #[inline] pub fn parse_in( file: &S, offset: u64, offset_size: u8, length_size: u8, + ) -> Result { + match file.as_contiguous() { + Some(all) => Self::parse_slice(all, offset, offset_size, length_size), + None => Self::parse_storage(file, offset, offset_size, length_size), + } + } + + /// [`Self::parse_storage`] for the slice, compiled in this crate: the + /// one copy [`Self::parse`] and [`Self::parse_in`] (in memory) call. + fn parse_slice( + data: &[u8], + offset: u64, + offset_size: u8, + length_size: u8, + ) -> Result { + Self::parse_storage(data, offset, offset_size, length_size) + } + + fn parse_storage( + file: &S, + offset: u64, + offset_size: u8, + length_size: u8, ) -> Result { // 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 diff --git a/crates/clawhdf5-format/src/storage.rs b/crates/clawhdf5-format/src/storage.rs index 08720ab..d53bdc7 100644 --- a/crates/clawhdf5-format/src/storage.rs +++ b/crates/clawhdf5-format/src/storage.rs @@ -20,6 +20,18 @@ //! still works (`S = dyn Storage`), and a remote backend pays one indirect //! call per structure read. //! +//! # Slice entry points +//! +//! A generic core is instantiated in the crate that calls it, so a +//! downstream crate calling `parse_in::<[u8]>` gets its own copy of the +//! parser, compiled without this crate's private helpers inlined (there is +//! no cross-crate inlining of non-`#[inline]` functions without LTO): a +//! metadata walk through the facade ran about 6% slower that way than +//! through the `&[u8]` wrappers. The `*_in` entry points on the facade's hot +//! paths (object headers, group listing and lookup, attributes) therefore +//! check [`Storage::as_contiguous`] first and hand an in-memory file to +//! their non-generic `&[u8]` wrapper, compiled here; both run the one core. +//! //! The trait is synchronous and `no_std`: parsing is CPU work, and a remote //! backend bridges to its own I/O.