format: slice entry points for the facade's hot *_in calls (local listing back to main's speed)
CI / test-arm64 (pull_request) Successful in 1m25s
CI / test (pull_request) Successful in 19m0s

Since the M2 merge the facade handed in-memory files to the generic
`*_in` parsers as `&[u8]` (`with_bytes!`), which instantiates them in
the facade crate, where the format crate's private helpers do not
inline without LTO: listing a 400-group v1 file through `File::open`
was 7-10% slower than main. `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}` now pass a storage with `as_contiguous()` to their
non-generic slice entry point, compiled once in the format crate; other
storages reach the same generic core as before.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
osobh
2026-09-26 22:28:22 -05:00
co-authored by Claude Opus 5.5
parent 4313917b4d
commit ef428d756c
5 changed files with 151 additions and 12 deletions
+26
View File
@@ -149,6 +149,32 @@
inside the file is an error. inside the file is an error.
- **No behaviour change for in-memory and mapped files:** with - **No behaviour change for in-memory and mapped files:** with
`as_contiguous()` every path slices the file as before (checked below). `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): - Tests (2026-09-26, tank):
- `clawhdf5-format/tests/storage_equivalence.rs` now also reads every - `clawhdf5-format/tests/storage_equivalence.rs` now also reads every
dataset — whole, fill-aware, through a chunk cache (twice) and the dataset — whole, fill-aware, through a chunk cache (twice) and the
+35 -4
View File
@@ -470,16 +470,31 @@ pub fn extract_attributes_tolerant(
offset_size: u8, offset_size: u8,
length_size: u8, length_size: u8,
) -> Result<(Vec<AttributeMessage>, Vec<FormatError>), FormatError> { ) -> Result<(Vec<AttributeMessage>, Vec<FormatError>), 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_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<S: Storage + ?Sized>( pub fn extract_attributes_tolerant_in<S: Storage + ?Sized>(
file_data: &S, file_data: &S,
header: &ObjectHeader, header: &ObjectHeader,
offset_size: u8, offset_size: u8,
length_size: u8, length_size: u8,
) -> Result<(Vec<AttributeMessage>, Vec<FormatError>), 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<S: Storage + ?Sized>(
file_data: &S,
header: &ObjectHeader,
offset_size: u8,
length_size: u8,
) -> Result<(Vec<AttributeMessage>, Vec<FormatError>), FormatError> { ) -> Result<(Vec<AttributeMessage>, Vec<FormatError>), FormatError> {
let mut errors = Vec::new(); let mut errors = Vec::new();
let attrs = extract_attributes_with(file_data, header, offset_size, length_size, &mut |e| { 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, offset_size: u8,
length_size: u8, length_size: u8,
) -> Result<Option<AttributeMessage>, FormatError> { ) -> Result<Option<AttributeMessage>, 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 /// [`find_attribute_in_file`] over any [`Storage`] (see
/// [`extract_attributes_full_in`] for dense storage, whose name index still /// [`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<S: Storage + ?Sized>( pub fn find_attribute_in<S: Storage + ?Sized>(
file_data: &S, file_data: &S,
header: &ObjectHeader, header: &ObjectHeader,
name: &str, name: &str,
offset_size: u8, offset_size: u8,
length_size: u8, length_size: u8,
) -> Result<Option<AttributeMessage>, 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<S: Storage + ?Sized>(
file_data: &S,
header: &ObjectHeader,
name: &str,
offset_size: u8,
length_size: u8,
) -> Result<Option<AttributeMessage>, FormatError> { ) -> Result<Option<AttributeMessage>, FormatError> {
let attr_info = find_attribute_info(header, offset_size)?; let attr_info = find_attribute_info(header, offset_size)?;
let dense = attr_info let dense = attr_info
+49 -6
View File
@@ -368,15 +368,30 @@ pub fn resolve_child(
group_address: u64, group_address: u64,
name: &str, name: &str,
) -> Result<u64, FormatError> { ) -> Result<u64, FormatError> {
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<S: Storage + ?Sized>( pub fn resolve_child_in<S: Storage + ?Sized>(
file_data: &S, file_data: &S,
superblock: &Superblock, superblock: &Superblock,
group_address: u64, group_address: u64,
name: &str, name: &str,
) -> Result<u64, FormatError> {
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<S: Storage + ?Sized>(
file_data: &S,
superblock: &Superblock,
group_address: u64,
name: &str,
) -> Result<u64, FormatError> { ) -> Result<u64, FormatError> {
let os = superblock.offset_size; let os = superblock.offset_size;
let ls = superblock.length_size; let ls = superblock.length_size;
@@ -453,14 +468,28 @@ pub fn resolve_path_any(
superblock: &Superblock, superblock: &Superblock,
path: &str, path: &str,
) -> Result<u64, FormatError> { ) -> Result<u64, FormatError> {
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<S: Storage + ?Sized>( pub fn resolve_path_any_in<S: Storage + ?Sized>(
file_data: &S, file_data: &S,
superblock: &Superblock, superblock: &Superblock,
path: &str, path: &str,
) -> Result<u64, FormatError> {
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<S: Storage + ?Sized>(
file_data: &S,
superblock: &Superblock,
path: &str,
) -> Result<u64, FormatError> { ) -> Result<u64, FormatError> {
resolve_path_following_links( resolve_path_following_links(
file_data, file_data,
@@ -513,14 +542,28 @@ pub fn resolve_group_children(
superblock: &Superblock, superblock: &Superblock,
group_address: u64, group_address: u64,
) -> Result<Vec<GroupEntry>, FormatError> { ) -> Result<Vec<GroupEntry>, 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<S: Storage + ?Sized>( pub fn resolve_group_children_in<S: Storage + ?Sized>(
file_data: &S, file_data: &S,
superblock: &Superblock, superblock: &Superblock,
group_address: u64, group_address: u64,
) -> Result<Vec<GroupEntry>, 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<S: Storage + ?Sized>(
file_data: &S,
superblock: &Superblock,
group_address: u64,
) -> Result<Vec<GroupEntry>, FormatError> { ) -> Result<Vec<GroupEntry>, FormatError> {
let os = superblock.offset_size; let os = superblock.offset_size;
let ls = superblock.length_size; let ls = superblock.length_size;
+29 -2
View File
@@ -116,25 +116,52 @@ impl ObjectHeader {
/// Parse an object header at the given offset in the data buffer. /// Parse an object header at the given offset in the data buffer.
/// ///
/// `offset_size` and `length_size` come from the superblock. /// `offset_size` and `length_size` come from the superblock.
#[inline]
pub fn parse( pub fn parse(
data: &[u8], data: &[u8],
offset: usize, offset: usize,
offset_size: u8, offset_size: u8,
length_size: u8, length_size: u8,
) -> Result<ObjectHeader, FormatError> { ) -> Result<ObjectHeader, FormatError> {
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`]. /// [`Self::parse`] over any [`Storage`].
/// ///
/// Reads the prefix (at most [`V2_PREFIX_MAX`] bytes, signature /// Reads the prefix (at most [`V2_PREFIX_MAX`] bytes, signature
/// included), then each chunk as one bounded read, continuation chunks /// 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<S: Storage + ?Sized>( pub fn parse_in<S: Storage + ?Sized>(
file: &S, file: &S,
offset: u64, offset: u64,
offset_size: u8, offset_size: u8,
length_size: u8, length_size: u8,
) -> Result<ObjectHeader, FormatError> {
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<ObjectHeader, FormatError> {
Self::parse_storage(data, offset, offset_size, length_size)
}
fn parse_storage<S: Storage + ?Sized>(
file: &S,
offset: u64,
offset_size: u8,
length_size: u8,
) -> Result<ObjectHeader, FormatError> { ) -> Result<ObjectHeader, FormatError> {
// The longest prefix of either version, in one read. It holds the // 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 // whole prefix or ends at the end of the file, so its bounds checks
+12
View File
@@ -20,6 +20,18 @@
//! still works (`S = dyn Storage`), and a remote backend pays one indirect //! still works (`S = dyn Storage`), and a remote backend pays one indirect
//! call per structure read. //! 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 //! The trait is synchronous and `no_std`: parsing is CPU work, and a remote
//! backend bridges to its own I/O. //! backend bridges to its own I/O.