From 761bdbf24fbf99e10a5da7c4bb4315da9b1952a9 Mon Sep 17 00:00:00 2001 From: osobh Date: Sun, 27 Sep 2026 22:47:47 -0500 Subject: [PATCH] format: look a name up in a v1 group down its B-tree, as libhdf5 does Opening one dataset of a v1 (symbol table) group read every symbol table node and every name of the group to find it: over openUrl, 74 requests and 193 MB to read one 64 KiB dataset of the reviewer's 3000-dataset h5py file (libver earliest) at 1 MiB blocks, 515 requests and 34 MB at 64 KiB. Locally it made a lookup O(entries). `group_v1::find_v1_entry` looks the name up as libhdf5's `H5G__stab_lookup` does: `H5B_find`'s binary search at each B-tree node with `H5G__node_cmp3` (left key < name <= right key, keys being names in the local heap, compared bytewise like strcmp), then the one symbol table node, after the heap's free list is checked as the listing does. The heap's data segment (up to 1 MiB) is hinted, since the keys are read one after another. Path resolution uses it for a v1 group; only when it does not find a hard link of that name (a soft link, or a B-tree out of name order, damaged or hand-made, where libhdf5 would report the name missing) does it read every entry as before. A storage error is returned as is (over the lazy reader, a miss: reading every entry would not get further). The result differs from before only in a group holding two entries of one name, where the B-tree's is now the one found, as in libhdf5. Measured with tests/lazy.rs listing_cost_of_a_given_file, open + read one 64 KiB dataset of the reviewer-like file, passes/requests/bytes, before -> after (open included): earliest, 1 MiB: 7/74/193.6 MB -> 6/5/5.2 MB earliest, 64 KiB: 9/515/34.1 MB -> 8/7/524 KB latest (dense groups, already a name-index lookup): 1 MiB 8/7/6.7 MB -> 7/7/6.7 MB, 64 KiB 9/8/581 KB -> 8/8/581 KB (the previous commit's hints: the name index header with the heap header) New tests, failing before: every child of v1_groups_400.h5 resolves to its listed address reading under 1/8 of the listing's bytes, and missing names are not found; a name moved out of B-tree order is still found (by the fallback); reading one of 2000 datasets lazily at 512-byte blocks takes at most 7 passes and 6 requests (earliest; 529 requests, 333 kB before) and 8 passes, 9 requests (latest). Conformance 600 of 697 (baseline 600), no file's class or detail changed against a run of main. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/clawhdf5-format/src/btree_v1.rs | 2 +- crates/clawhdf5-format/src/group_v1.rs | 91 +++++++++++++++++++++- crates/clawhdf5-format/src/group_v2.rs | 101 +++++++++++++++++++++++++ crates/clawhdf5-wasm/tests/lazy.rs | 81 ++++++++++++++++++-- 4 files changed, 265 insertions(+), 10 deletions(-) diff --git a/crates/clawhdf5-format/src/btree_v1.rs b/crates/clawhdf5-format/src/btree_v1.rs index 7ae2af7..78b2329 100644 --- a/crates/clawhdf5-format/src/btree_v1.rs +++ b/crates/clawhdf5-format/src/btree_v1.rs @@ -158,7 +158,7 @@ impl BTreeV1Node { } /// Maximum recursion depth for B-tree traversal (malformed data protection). -const MAX_BTREE_DEPTH: usize = 64; +pub(crate) const MAX_BTREE_DEPTH: usize = 64; /// What a symbol table node takes with libhdf5's default group leaf K (4): /// its 8-byte header and 2K entries of 40 bytes (8-byte offsets). Hinted diff --git a/crates/clawhdf5-format/src/group_v1.rs b/crates/clawhdf5-format/src/group_v1.rs index f131acc..4a930dc 100644 --- a/crates/clawhdf5-format/src/group_v1.rs +++ b/crates/clawhdf5-format/src/group_v1.rs @@ -4,7 +4,7 @@ use alloc::{string::String, vec::Vec}; use crate::addr::checked_addr; -use crate::btree_v1::collect_symbol_table_nodes_in; +use crate::btree_v1::{BTreeV1Node, collect_symbol_table_nodes_in}; use crate::error::FormatError; use crate::local_heap::LocalHeap; use crate::message_type::MessageType; @@ -142,6 +142,95 @@ pub(crate) fn v1_group_entries( } } +/// The entry called `name` in a v1 group, looked up as libhdf5 looks it up +/// (`H5G__stab_lookup`: `H5B_find` down the group's B-tree, then +/// `H5G__node_found` in one symbol table node) instead of by reading every +/// entry: at each node, the child whose key interval holds the name +/// (left key < name <= right key, keys being names in the local heap, +/// compared bytewise as `strcmp` does) is found by binary search. Reads +/// O(depth) nodes and names, where listing reads the whole group. +/// +/// `Ok(None)` when the search does not lead to the name. In a group whose +/// B-tree is out of name order (damaged, or made by hand) that does not +/// prove it absent, so callers then fall back to reading every entry; +/// libhdf5 would report it missing. +pub(crate) fn find_v1_entry( + file_data: &S, + sym_table_msg: &SymbolTableMessage, + name: &str, + offset_size: u8, + length_size: u8, +) -> Result, FormatError> { + let heap = LocalHeap::parse_in( + file_data, + checked_addr(sym_table_msg.local_heap_address)?, + offset_size, + length_size, + )?; + // The keys and names are read from the heap's data segment one at a + // time: hint it (see `Storage::hint`). + file_data.hint( + heap.data_segment_address, + usize::try_from(heap.data_segment_size).map_or(1 << 20, |n| n.min(1 << 20)), + ); + // As in listing: the heap's free list is checked before a name is used. + let mut heap_checked = false; + let mut name_at = |offset: u64| -> Result { + if !heap_checked { + heap.validate_free_list_in(file_data, length_size)?; + heap_checked = true; + } + heap.read_string_in(file_data, offset) + }; + let want = name.as_bytes(); + let mut address = sym_table_msg.btree_address; + for _ in 0..=crate::btree_v1::MAX_BTREE_DEPTH { + let node = + BTreeV1Node::parse_in(file_data, checked_addr(address)?, offset_size, length_size)?; + if node.node_type != 0 { + return Err(FormatError::InvalidBTreeNodeType(node.node_type)); + } + // H5B_find's binary search with H5G__node_cmp3: go left when the + // name sorts at or before the left key, right when after the right + // key; otherwise this child holds it. + let (mut lo, mut hi) = (0usize, node.children.len()); + let mut child = None; + while lo < hi { + let i = lo + (hi - lo) / 2; + let (Some(&left), Some(&right)) = (node.keys.get(i), node.keys.get(i + 1)) else { + return Ok(None); + }; + if want <= name_at(left)?.as_bytes() { + hi = i; + } else if want > name_at(right)?.as_bytes() { + lo = i + 1; + } else { + child = Some(node.children[i]); + break; + } + } + let Some(child) = child else { + return Ok(None); + }; + if node.node_level > 0 { + address = child; + continue; + } + let snod = SymbolTableNode::parse_in(file_data, checked_addr(child)?, offset_size)?; + for entry in &snod.entries { + if name_at(entry.link_name_offset)?.as_bytes() == want { + return Ok(Some(GroupEntry { + name: String::from(name), + object_header_address: entry.object_header_address, + cache_type: entry.cache_type, + })); + } + } + return Ok(None); + } + Err(FormatError::NestingDepthExceeded) +} + /// Symbol table cache type for a soft link: the scratch pad's first four bytes /// are the local-heap offset of the link's target path, and the entry's object /// header address is undefined. diff --git a/crates/clawhdf5-format/src/group_v2.rs b/crates/clawhdf5-format/src/group_v2.rs index bc1a705..e902239 100644 --- a/crates/clawhdf5-format/src/group_v2.rs +++ b/crates/clawhdf5-format/src/group_v2.rs @@ -385,6 +385,27 @@ fn lookup_link( length_size: u8, ) -> Result, FormatError> { if is_v1_group(object_header) { + // Down the group's B-tree, as libhdf5 looks a name up; only when + // that does not find a hard link of that name is every entry read + // (a soft link, a group whose B-tree is out of order). A storage + // error (a read a restartable storage has not fetched yet) is + // returned as is: reading every entry would not get further. + if let Some(sym_msg) = object_header + .messages + .iter() + .find(|m| m.msg_type == MessageType::SymbolTable) + { + let stm = SymbolTableMessage::parse(&sym_msg.data, offset_size)?; + match group_v1::find_v1_entry(file_data, &stm, name, offset_size, length_size) { + Ok(Some(e)) if e.object_header_address != u64::MAX => { + return Ok(Some(LinkTarget::Hard { + object_header_address: e.object_header_address, + })); + } + Err(e @ FormatError::Storage(_)) => return Err(e), + _ => {} + } + } let entries = resolve_group_entries(file_data, object_header, offset_size, length_size)?; if let Some(e) = entries .iter() @@ -984,6 +1005,86 @@ mod tests { assert_eq!(values, vec![22.5, 23.1, 21.8]); } + /// `v1_groups_400.h5` from its superblock on (it has a user block). + fn v1_groups_400() -> (Vec, Superblock) { + let all: &[u8] = include_bytes!("../tests/fixtures/v1_groups_400.h5"); + let data = all[signature::find_signature(all).unwrap()..].to_vec(); + let sb = Superblock::parse(&data, 0).unwrap(); + (data, sb) + } + + /// Every child of a v1 group resolves by name, down the group's B-tree, + /// to the address the listing gives, reading a small part of what the + /// listing reads; a name the group does not hold is not found. + #[test] + fn v1_lookup_down_the_btree_agrees_with_the_listing() { + let (data, sb) = v1_groups_400(); + let children = resolve_group_children(&data, &sb, sb.root_group_address).unwrap(); + assert_eq!(children.len(), 401); + for c in &children { + let path = format!("/{}", c.name); + assert_eq!( + resolve_path_any(&data, &sb, &path).unwrap(), + c.object_header_address, + "{path}" + ); + } + for missing in ["/g0400", "/a", "/g", "/g00000", "/zz", "/x0"] { + assert!( + matches!( + resolve_path_any(&data, &sb, missing), + Err(FormatError::PathNotFound(_)) + ), + "{missing}" + ); + } + let st = crate::storage::CountingStorage::new(data.clone()); + resolve_group_children_in(&st, &sb, sb.root_group_address).unwrap(); + let listing = st.bytes_read(); + st.reset(); + let last = children.last().unwrap(); + assert_eq!( + resolve_path_any_in(&st, &sb, &format!("/{}", last.name)).unwrap(), + last.object_header_address + ); + assert!( + st.bytes_read() * 8 < listing, + "lookup read {} bytes, listing {listing}", + st.bytes_read() + ); + } + + /// A v1 group whose B-tree is out of name order (a name changed in the + /// heap so that it sorts past every key) is still looked up by reading + /// every entry, as before the lookup went down the B-tree. + #[test] + fn v1_lookup_falls_back_when_the_btree_is_out_of_order() { + let (mut data, sb) = v1_groups_400(); + let at: Vec = data + .windows(6) + .enumerate() + .filter(|(_, w)| *w == b"g0200\0") + .map(|(i, _)| i) + .collect(); + assert_eq!(at.len(), 1, "one heap string"); + data[at[0]] = b'~'; + let children = resolve_group_children(&data, &sb, sb.root_group_address).unwrap(); + let moved = children.iter().find(|c| c.name == "~0200").unwrap(); + assert_eq!( + resolve_path_any(&data, &sb, "/~0200").unwrap(), + moved.object_header_address + ); + assert!(resolve_path_any(&data, &sb, "/g0200").is_err()); + for c in &children { + let path = format!("/{}", c.name); + assert_eq!( + resolve_path_any(&data, &sb, &path).unwrap(), + c.object_header_address, + "{path}" + ); + } + } + #[test] fn path_not_found_v2() { let file_data: &[u8] = include_bytes!("../tests/fixtures/v2_groups.h5"); diff --git a/crates/clawhdf5-wasm/tests/lazy.rs b/crates/clawhdf5-wasm/tests/lazy.rs index 9c972e0..26e10af 100644 --- a/crates/clawhdf5-wasm/tests/lazy.rs +++ b/crates/clawhdf5-wasm/tests/lazy.rs @@ -475,11 +475,17 @@ fn corpus_files_read_the_same_lazily() { ); } -/// Passes and requests `list(path)` takes on a file opened lazily at -/// `block`-byte blocks (the open not counted), checking the listing against -/// the in-memory one. -fn listing_cost(data: &[u8], path: &str, block: u64) -> (u64, u64) { - let want = Reader::open(data.to_vec()).unwrap().list(path).unwrap(); +/// What one call cost on a file opened lazily (the open not counted). +#[derive(Debug, Clone, Copy)] +struct Cost { + passes: u64, + requests: u64, + bytes: u64, +} + +/// Open `data` lazily at `block`-byte blocks, run `op`, and return its +/// result and what it cost (the open not counted). +fn cost_of(data: &[u8], block: u64, op: impl Fn(&Reader) -> T) -> (T, Cost) { let lazy = Lazy::open( data.to_vec(), LazyConfig { @@ -489,14 +495,41 @@ fn listing_cost(data: &[u8], path: &str, block: u64) -> (u64, u64) { ) .unwrap(); let before = lazy.storage.stats(); - assert_eq!(lazy.call(|r| r.list(path)).unwrap(), want); + let out = lazy.call(op); let after = lazy.storage.stats(); ( - after.passes - before.passes, - after.requests - before.requests, + out, + Cost { + passes: after.passes - before.passes, + requests: after.requests - before.requests, + bytes: after.bytes_fetched - before.bytes_fetched, + }, ) } +/// Passes and requests `list(path)` takes on a file opened lazily at +/// `block`-byte blocks (the open not counted), checking the listing against +/// the in-memory one. +fn listing_cost(data: &[u8], path: &str, block: u64) -> (u64, u64) { + let want = Reader::open(data.to_vec()).unwrap().list(path).unwrap(); + let (got, cost) = cost_of(data, block, |r| r.list(path)); + assert_eq!(got.unwrap(), want); + (cost.passes, cost.requests) +} + +/// What reading the dataset at `path` whole takes on a file opened lazily +/// at `block`-byte blocks (the open not counted), checking the values +/// against the in-memory read. +fn read_cost(data: &[u8], path: &str, block: u64) -> Cost { + let want = Reader::open(data.to_vec()) + .unwrap() + .read(path, None) + .unwrap(); + let (got, cost) = cost_of(data, block, |r| r.read(path, None)); + assert_eq!(got.unwrap(), want, "{path}"); + cost +} + /// An h5py file of `n` datasets of 256 `f32` each (`d0` ... ) in the root /// group, written with `libver`. fn h5py_many(dir: &Path, libver: &str, n: usize) -> Vec { @@ -561,6 +594,38 @@ fn listing_a_large_group_takes_a_few_passes() { } } +/// Opening one dataset of a large group looks its name up, not the whole +/// group: in a v1 (symbol table) group down its B-tree as libhdf5 does, in +/// a dense group down its name index. Reading one small dataset of 2000 +/// fetches a few blocks, where it used to read every symbol table node and +/// name of a v1 group (529 requests, 333 kB at 512-byte blocks, before +/// 2026-09-27). +#[test] +fn reading_one_dataset_of_a_large_group_fetches_a_few_blocks() { + if !python_available() { + assert!( + !std::env::var("CLAWHDF5_REQUIRE_INTEROP").is_ok_and(|v| v == "1"), + "CLAWHDF5_REQUIRE_INTEROP=1 but {} lacks h5py/netCDF4/numpy", + python() + ); + eprintln!("skipping: {} lacks h5py/netCDF4/numpy", python()); + return; + } + let dir = tempfile::tempdir().unwrap(); + for (libver, most_passes, most_requests) in [("earliest", 7, 6), ("latest", 8, 9)] { + let data = h5py_many(dir.path(), libver, 2000); + for path in ["/d0", "/d1234", "/d1999"] { + let cost = read_cost(&data, path, 512); + eprintln!("h5py libver={libver}, 2000 children, read {path}: {cost:?}"); + assert!( + cost.passes <= most_passes && cost.requests <= most_requests, + "{libver} {path}: {cost:?}" + ); + assert!(cost.bytes <= 64 * 1024, "{libver} {path}: {cost:?}"); + } + } +} + /// `CLAWHDF5_WASM_LIST_FILE=file.h5`: what listing the root group of that /// file costs lazily, and opening it and reading one dataset whole /// (`CLAWHDF5_WASM_READ`, by default the middle dataset of the listing),