format: monomorphise the Storage parsers so local files stay as fast
Every `*_in` core and the read helpers take `file: &S` with `S: Storage + ?Sized` instead of `&dyn Storage`, and the `&[u8]` wrappers pass the slice itself, so they compile to a `[u8]` instance: `as_contiguous()` inlines to `Some(self)` and each structure read is the slice code's bounds check again, with no indirect call. `&dyn Storage` still works (`S = dyn Storage`); there is one parser implementation. Also, so the structure reads cost no more than the slice checks did: - ObjectHeader::parse_in reads the prefix once (signature included) instead of the signature and then the prefix: two reads for a one-chunk header instead of three on a range backend; - the symbol-table node and group B-tree (v1) loops walk their entries with chunks_exact over the bytes read, and the node's redundant second bounds check is gone (the entries' read is the check, same error); - a version-1 header's message list is sized from its (capped) count. Same results and errors; the unit and equivalence tests are unchanged. New Criterion bench `clawhdf5/benches/local_metadata_bench.rs` over a 400-group version-1 file written by h5py (new fixture `v1_groups_400.h5`): ObjectHeader::parse, symbol-table nodes, the group B-tree walk and a facade listing, using only APIs that exist atf2ff2c4so it builds there for an A/B. Provisional A/B againstf2ff2c4(busy machine, not for docs): both builds linked into one binary and timed in alternation, 200 rounds; median ratio new/old: facade listing -0.5% to -3.5% (was +14%), ObjectHeader::parse +1% to +2% (was +25%), symbol-table nodes -18%, group B-tree walk -18%, local-heap names and resolve_group_children within +-1.5%. An old-vs-old-copy run shows +-2% from code layout alone. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
@@ -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<S: Storage + ?Sized>(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<T: Storage + ?Sized> Storage for std::sync::Arc<T> {
|
||||
/// `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<S: Storage + ?Sized>(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<S: Storage + ?Sized>(
|
||||
file: &S,
|
||||
offset: u64,
|
||||
len: usize,
|
||||
) -> Result<Cow<'_, [u8]>, 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<Self, FormatError> {
|
||||
pub fn read<S: Storage + ?Sized>(
|
||||
file: &'a S,
|
||||
base: u64,
|
||||
max: usize,
|
||||
) -> Result<Self, FormatError> {
|
||||
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<S: Storage + ?Sized>(
|
||||
file: &S,
|
||||
offset: u64,
|
||||
max: usize,
|
||||
) -> Result<Cow<'_, [u8]>, 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()
|
||||
|
||||
Reference in New Issue
Block a user