From 7acfb795848704db88c171a6f3110752ce83e668 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 10:03:22 -0500 Subject: [PATCH 1/3] writer: order dense name indexes by hash, then name libhdf5 compares the name when two hashes are equal; the writer broke ties by insertion order, and libhdf5 could not find one of two names whose lookup3 hashes collide (k69209 / k155448). Test fails before the fix with h5py's KeyError. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 11 +++++ crates/clawhdf5-format/src/file_writer.rs | 43 ++++++++++++------- .../clawhdf5/tests/writer_groups_interop.rs | 39 +++++++++++++++++ 3 files changed, 77 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 29804fc..0e50755 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,17 @@ ## Unreleased +### Writer: large dense indexes (2026-09-26) +- **Links and attributes whose name hashes collide are found by name.** The + dense name indexes (a group's links: B-tree v2 type 5; an object's + attributes: type 8) are ordered by the name's lookup3 hash and, when two + hashes are equal, by the name itself, as libhdf5 compares them. The writer + broke ties by insertion order, so libhdf5 could not open one of two + colliding names (`"k69209"` and `"k155448"` share hash `0x3a0b13e6`; + collisions are likely from about 77 000 names). Regression test + `names_whose_hashes_collide_are_found_by_name` in + `crates/clawhdf5/tests/writer_groups_interop.rs`. + ### Concurrent reads (2026-09-26) - **Full reads of chunked datasets scale with threads again when rayon's pool has one thread.** Each full read handed its chunks to rayon to diff --git a/crates/clawhdf5-format/src/file_writer.rs b/crates/clawhdf5-format/src/file_writer.rs index 11345a1..80a296b 100644 --- a/crates/clawhdf5-format/src/file_writer.rs +++ b/crates/clawhdf5-format/src/file_writer.rs @@ -910,20 +910,27 @@ pub(crate) fn build_dense_attrs( let heap_id_length = heap.heap_id_length; let heap_ids = &heap.heap_ids; - // Build B-tree v2 type 8 records (17 bytes each) + // Build B-tree v2 type 8 records (17 bytes each), in the index's key + // order: libhdf5 compares the name hash, then — for names whose hashes + // collide — the names themselves (`strcmp`). let record_size: u16 = heap_id_length + 1 + 4 + 4; - let mut records: Vec<(u32, u32, Vec)> = Vec::with_capacity(attrs.len()); - for (i, heap_id) in heap_ids.iter().enumerate() { - let mut rec = Vec::with_capacity(record_size as usize); - rec.extend_from_slice(heap_id); - rec.push(0); // msg_flags - rec.extend_from_slice(&(i as u32).to_le_bytes()); // creation_order - rec.extend_from_slice(&name_hashes[i].to_le_bytes()); // hash - records.push((name_hashes[i], i as u32, rec)); - } - records.sort_by(|a, b| a.0.cmp(&b.0).then(a.1.cmp(&b.1))); - - let records: Vec> = records.into_iter().map(|(_, _, rec)| rec).collect(); + let mut order: Vec = (0..attrs.len()).collect(); + order.sort_by(|&a, &b| { + name_hashes[a] + .cmp(&name_hashes[b]) + .then_with(|| attrs[a].name.as_bytes().cmp(attrs[b].name.as_bytes())) + }); + let records: Vec> = order + .into_iter() + .map(|i| { + let mut rec = Vec::with_capacity(record_size as usize); + rec.extend_from_slice(&heap_ids[i]); + rec.push(0); // msg_flags + rec.extend_from_slice(&(i as u32).to_le_bytes()); // creation_order + rec.extend_from_slice(&name_hashes[i].to_le_bytes()); // hash + rec + }) + .collect(); let bthd_addr = btree_addr; let mut blob = heap.blob; blob.extend_from_slice(&single_leaf_v2_btree( @@ -1039,14 +1046,18 @@ pub(crate) fn build_dense_links( let heap = build_single_block_fractal_heap(&serialized, base_address, 32, 7)?; let heap_id_length = heap.heap_id_length; - // Type 5 records: hash(4) + heap_id. The B-tree's key is the name hash, - // so records are sorted by (hash, order). + // Type 5 records: hash(4) + heap_id. The B-tree's key is the name hash + // and, for names whose hashes collide, the name (libhdf5 compares them + // with `strcmp`): records out of that order are not found by name. let mut by_name: Vec<(u32, usize)> = links .iter() .enumerate() .map(|(i, l)| (crate::checksum::jenkins_lookup3(l.name.as_bytes()), i)) .collect(); - by_name.sort_unstable(); + by_name.sort_unstable_by(|&(ha, a), &(hb, b)| { + ha.cmp(&hb) + .then_with(|| links[a].name.as_bytes().cmp(links[b].name.as_bytes())) + }); let name_records: Vec> = by_name .iter() .map(|&(hash, i)| { diff --git a/crates/clawhdf5/tests/writer_groups_interop.rs b/crates/clawhdf5/tests/writer_groups_interop.rs index 97e3544..46d155b 100644 --- a/crates/clawhdf5/tests/writer_groups_interop.rs +++ b/crates/clawhdf5/tests/writer_groups_interop.rs @@ -627,6 +627,45 @@ fn non_ascii_names_are_utf8() { assert_eq!(f.dataset("größe/wert").unwrap().read_i32().unwrap(), [1]); } +#[test] +fn names_whose_hashes_collide_are_found_by_name() { + skip_if_no_python!(); + // "k69209" and "k155448" have the same lookup3 hash (0x3a0b13e6). The + // dense name indexes (links: type 5, attributes: type 8) are ordered by + // hash and then by name, and libhdf5's lookup relies on it. The writer + // broke ties by insertion order, so with "k69209" added first libhdf5 + // could not open "k155448" by name. + let dir = tempfile::tempdir().unwrap(); + let mut b = FileBuilder::new(); + let mut g = b.create_group("g"); + for (i, n) in ["k69209", "k155448"] + .into_iter() + .chain((0..10).map(|_| "")) + .enumerate() + { + let name = if n.is_empty() { format!("d{i}") } else { n.into() }; + g.create_dataset(&name).with_i32_data(&[i as i32]); + } + b.add_group(g.finish()); + let x = b.create_dataset("x"); + x.with_i32_data(&[0]); + x.set_attr("k69209", AttrValue::I64(1)); + x.set_attr("k155448", AttrValue::I64(2)); + for i in 0..10 { + x.set_attr(&format!("a{i}"), AttrValue::I64(10 + i)); + } + let path = write(&dir, "collide.h5", b); + let out = h5py( + &path, + "with h5py.File(path, 'r') as f:\n\ + \x20 g, a = f['g'], f['x'].attrs\n\ + \x20 print(json.dumps([int(g['k69209'][0]), int(g['k155448'][0]), 'k155448' in g,\n\ + \x20 int(a['k69209']), int(a['k155448']), 'k155448' in a]))", + ); + assert_eq!(out, "[0, 1, true, 1, 2, true]"); + h5dump_ok(&path); +} + #[test] fn a_group_attribute_set_again_takes_the_new_value() { skip_if_no_python!(); From d63c76e7ab42f20f3b41b2f409969db5e595133f Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 10:12:07 -0500 Subject: [PATCH 2/3] writer: v2 B-trees with internal nodes (no 65 535-record limit) Dense link and attribute indexes and the chunk index for several unlimited dimensions were single leaves, capping them at 65 535 records. btree_v2_write builds trees of any depth, with node capacities and pointer widths from libhdf5's H5B2__hdr_init arithmetic (now shared with the reader as btree_v2::node_info) and libhdf5's node sizes (512 dense, 2048 chunks). Indexes that fit the old one-leaf layout are written byte for byte as before (compared for 10..65 535 links, attrs and chunks, tracked and filtered). Tests: 100 000 links (short names; long names with creation order), 70 000 attributes, 200 000 chunks (and 80 000 deflated), read by h5py, h5dump and clawhdf5 and edited by h5py r+; h5rs check on the same shapes, asserting depths 2-3. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 14 + crates/clawhdf5-format/src/btree_v2.rs | 81 +++- crates/clawhdf5-format/src/btree_v2_write.rs | 396 ++++++++++++++++++ crates/clawhdf5-format/src/chunked_write.rs | 90 ++-- crates/clawhdf5-format/src/file_writer.rs | 119 +++--- crates/clawhdf5-format/src/lib.rs | 1 + crates/clawhdf5-tools/tests/h5rs_interop.rs | 72 ++++ crates/clawhdf5/tests/chunk_index_interop.rs | 14 - crates/clawhdf5/tests/deep_btree_interop.rs | 343 +++++++++++++++ .../clawhdf5/tests/writer_groups_interop.rs | 32 +- docs/known-issues.md | 28 +- 11 files changed, 1005 insertions(+), 185 deletions(-) create mode 100644 crates/clawhdf5-format/src/btree_v2_write.rs create mode 100644 crates/clawhdf5/tests/deep_btree_interop.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e50755..448d2a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,20 @@ ## Unreleased ### Writer: large dense indexes (2026-09-26) +- **No more 65 535-record limit on the writer's v2 B-trees.** Dense link + storage (name index and creation-order index), dense attribute storage + and the chunk index of datasets with more than one unlimited dimension + were written as a single leaf, so a group with more than 65 535 links, an + object with more than 65 535 dense attributes, or such a dataset with more + than 65 535 chunks was an error. The writer now builds internal nodes to + any depth (`clawhdf5_format::btree_v2_write`), with node capacities and + child-pointer widths from the same arithmetic as libhdf5's + `H5B2__hdr_init` (shared with the reader, `btree_v2::node_info`) and + libhdf5's node sizes (512 bytes for dense storage, 2048 for chunks). + Indexes that fit the old one-leaf layout are written byte for byte as + before. New tests `crates/clawhdf5/tests/deep_btree_interop.rs` (100 000 + links, 70 000 attributes, 200 000 chunks; h5py, h5dump, clawhdf5, and + h5py "r+" edits) and `check_files_with_deep_btrees` (`h5rs check`). - **Links and attributes whose name hashes collide are found by name.** The dense name indexes (a group's links: B-tree v2 type 5; an object's attributes: type 8) are ordered by the name's lookup3 hash and, when two diff --git a/crates/clawhdf5-format/src/btree_v2.rs b/crates/clawhdf5-format/src/btree_v2.rs index 0b70981..9159253 100644 --- a/crates/clawhdf5-format/src/btree_v2.rs +++ b/crates/clawhdf5-format/src/btree_v2.rs @@ -71,7 +71,7 @@ fn ensure_len(data: &[u8], pos: usize, needed: usize) -> Result<(), FormatError> /// Compute the number of bytes needed to represent a count, using variable-width encoding. /// B-tree v2 uses this for the number of records fields in internal nodes. -fn bytes_for_max_records(max_nrec: u64) -> usize { +pub(crate) fn bytes_for_max_records(max_nrec: u64) -> usize { if max_nrec == 0 { return 1; } @@ -163,7 +163,7 @@ impl BTreeV2Header { /// Compute maximum records per node for a given depth level. /// leaf: (node_size - overhead) / record_size /// internal: depends on pointers -fn max_records_leaf(node_size: u32, record_size: u16) -> u64 { +pub(crate) fn max_records_leaf(node_size: u32, record_size: u16) -> u64 { // Leaf overhead: signature(4) + version(1) + type(1) + checksum(4) = 10 let overhead = 10u32; if node_size <= overhead || record_size == 0 { @@ -418,10 +418,7 @@ fn collect_internal_records( } /// Most records a subtree whose root is at `depth` can hold (libhdf5's -/// `cum_max_nrec`): a leaf holds `max_leaf_nrec`; an internal node at depth -/// `d` holds `max_nrec(d)` records and `max_nrec(d) + 1` subtrees of depth -/// `d - 1`, where `max_nrec(d)` is what fits in a node once each record is -/// paired with a child pointer of the width depth `d` needs. +/// `cum_max_nrec`). See [`node_info`]. fn cum_max_records( node_size: u32, record_size: u16, @@ -429,24 +426,82 @@ fn cum_max_records( max_leaf_nrec: u64, depth: u16, ) -> u64 { + node_info_from_leaf(node_size, record_size, offset_size, max_leaf_nrec, depth) + .last() + .map_or(max_leaf_nrec, |n| n.cum_max_nrec) +} + +/// Capacity of a B-tree v2 node at one depth, as libhdf5 computes it +/// (`H5B2__hdr_init`'s `node_info`). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) struct NodeInfo { + /// Most records one node at this depth holds. + pub(crate) max_nrec: u64, + /// Most records a subtree rooted at this depth holds. + pub(crate) cum_max_nrec: u64, + /// Bytes a subtree's total record count takes in a pointer to a node + /// at this depth (0 for a leaf, whose count is its own). + pub(crate) cum_max_nrec_size: usize, +} + +/// Node capacities for depths `0..=depth` (entry `d` for depth `d`): a leaf +/// holds `max_nrec(0)` records; an internal node at depth `d` holds +/// `max_nrec(d)` records and `max_nrec(d) + 1` subtrees of depth `d - 1`, +/// where `max_nrec(d)` is what fits in a node once each record is paired +/// with a child pointer of the width depth `d` needs (address, the child's +/// record count in the width a *leaf's* maximum needs, and below the first +/// internal level the child subtree's total in the width its maximum +/// needs), with one pointer more than records. +pub(crate) fn node_info( + node_size: u32, + record_size: u16, + offset_size: u8, + depth: u16, +) -> Vec { + let max_leaf = max_records_leaf(node_size, record_size); + node_info_from_leaf(node_size, record_size, offset_size, max_leaf, depth) +} + +fn node_info_from_leaf( + node_size: u32, + record_size: u16, + offset_size: u8, + max_leaf_nrec: u64, + depth: u16, +) -> Vec { // Internal node overhead: signature(4) + version(1) + type(1) + checksum(4). const PREFIX: u64 = 10; let nrec_width = bytes_for_max_records(max_leaf_nrec) as u64; - let mut cum = max_leaf_nrec; - let mut cum_width = 0u64; + let mut info = Vec::with_capacity(usize::from(depth) + 1); + info.push(NodeInfo { + max_nrec: max_leaf_nrec, + cum_max_nrec: max_leaf_nrec, + cum_max_nrec_size: 0, + }); for d in 1..=depth { - let ptr = u64::from(offset_size) + nrec_width + if d > 1 { cum_width } else { 0 }; + let below = info[usize::from(d) - 1]; + let ptr = u64::from(offset_size) + + nrec_width + + if d > 1 { + below.cum_max_nrec_size as u64 + } else { + 0 + }; let max_nrec = u64::from(node_size) .saturating_sub(PREFIX) .saturating_sub(ptr) / (u64::from(record_size) + ptr).max(1); - cum = max_nrec + let cum = max_nrec .saturating_add(1) - .saturating_mul(cum) + .saturating_mul(below.cum_max_nrec) .saturating_add(max_nrec); - cum_width = bytes_for_max_records(cum) as u64; + info.push(NodeInfo { + max_nrec, + cum_max_nrec: cum, + cum_max_nrec_size: bytes_for_max_records(cum), + }); } - cum + info } #[cfg(test)] diff --git a/crates/clawhdf5-format/src/btree_v2_write.rs b/crates/clawhdf5-format/src/btree_v2_write.rs new file mode 100644 index 0000000..1a18984 --- /dev/null +++ b/crates/clawhdf5-format/src/btree_v2_write.rs @@ -0,0 +1,396 @@ +//! Writing version-2 B-trees: a header (`BTHD`) and its nodes, leaves +//! (`BTLF`) and, for more records than one leaf holds, internal nodes +//! (`BTIN`) to any depth. +//! +//! Node capacities come from [`crate::btree_v2::node_info`], the arithmetic +//! libhdf5 uses (`H5B2__hdr_init`) and the reader decodes pointers with, so +//! the pointer widths the writer encodes are the ones every reader expects. + +#[cfg(not(feature = "std"))] +use alloc::{format, vec, vec::Vec}; + +use crate::btree_v2::{NodeInfo, bytes_for_max_records, node_info}; +use crate::checksum::jenkins_lookup3; +use crate::error::FormatError; + +/// How a B-tree is laid out: its record type and node geometry, as the +/// header records them. +#[derive(Debug, Clone, Copy)] +pub(crate) struct BTreeV2Params { + /// Record type (5: link names, 6: link creation order, 8: attribute + /// names, 9: attribute creation order, 10/11: chunks). + pub(crate) tree_type: u8, + /// Bytes per node. + pub(crate) node_size: u32, + /// Bytes per record. + pub(crate) record_size: u16, + /// Split and merge percentages. The writer fills nodes itself; these + /// only tell libhdf5 when to split and merge as it modifies the tree. + pub(crate) split_percent: u8, + pub(crate) merge_percent: u8, +} + +/// Size of a B-tree v2 header. +pub(crate) fn header_size(offset_size: u8, length_size: u8) -> usize { + 4 + 1 + 1 + 4 + 2 + 2 + 1 + 1 + offset_size as usize + 2 + length_size as usize + 4 +} + +/// Deepest tree the writer builds. Even at the smallest fan-out libhdf5's +/// arithmetic allows, a few levels hold more records than any file could. +const MAX_WRITE_DEPTH: u16 = 32; + +/// Write a B-tree v2 holding `records` (`record_size` bytes each, +/// concatenated, already in the tree's key order) at `addr`: the header, +/// then its nodes, each `node_size` bytes. No records gives a header with +/// an undefined root. +/// +/// The tree is as shallow as the node size allows: a single leaf when the +/// records fit one, otherwise internal nodes above leaves. Records are +/// spread evenly over each node's children, so every node but the root is +/// at least about half full (above libhdf5's merge threshold, which is below +/// half), and each node holds at most its depth's maximum. +pub(crate) fn build_btree_v2( + p: BTreeV2Params, + records: &[u8], + addr: u64, + offset_size: u8, + length_size: u8, +) -> Result, FormatError> { + let rs = usize::from(p.record_size); + if rs == 0 || !records.len().is_multiple_of(rs) { + return Err(FormatError::SerializationError(format!( + "B-tree v2 records are {} bytes, not a multiple of the record size {rs}", + records.len() + ))); + } + let n = (records.len() / rs) as u64; + let hdr_len = header_size(offset_size, length_size); + + // The shallowest depth whose subtree can hold every record. + let mut info = node_info(p.node_size, p.record_size, offset_size, 0); + let max_leaf = info[0].max_nrec; + if max_leaf == 0 || max_leaf > u64::from(u16::MAX) { + return Err(FormatError::SerializationError(format!( + "a {}-byte B-tree v2 node holds {max_leaf} {}-byte records; \ + a node holds 1 to 65535", + p.node_size, p.record_size + ))); + } + let mut depth = 0u16; + while info[usize::from(depth)].cum_max_nrec < n { + depth += 1; + if depth > MAX_WRITE_DEPTH { + return Err(FormatError::SerializationError(format!( + "{n} records do not fit a B-tree v2 of {}-byte nodes", + p.node_size + ))); + } + info = node_info(p.node_size, p.record_size, offset_size, depth); + let max = info[usize::from(depth)].max_nrec; + if max == 0 || max > u64::from(u16::MAX) { + return Err(FormatError::SerializationError(format!( + "a {}-byte B-tree v2 internal node holds {max} records; \ + a node holds 1 to 65535", + p.node_size + ))); + } + } + + let mut w = TreeWriter { + p, + records, + info: &info, + nrec_width: bytes_for_max_records(max_leaf), + offset_size, + first_node: addr + hdr_len as u64, + nodes: Vec::new(), + }; + let root = (n > 0).then(|| w.node(depth, 0, n as usize)).transpose()?; + + let mut out = Vec::with_capacity(hdr_len + w.nodes.len() * p.node_size as usize); + out.extend_from_slice(b"BTHD"); + out.push(0); // version + out.push(p.tree_type); + out.extend_from_slice(&p.node_size.to_le_bytes()); + out.extend_from_slice(&p.record_size.to_le_bytes()); + out.extend_from_slice(&depth.to_le_bytes()); + out.push(p.split_percent); + out.push(p.merge_percent); + match root { + Some(r) => push_uint(&mut out, r.addr, offset_size as usize), + None => out.extend(core::iter::repeat_n(0xFF, offset_size as usize)), + } + let root_nrec = root.map_or(0, |r| r.nrec); + out.extend_from_slice(&(root_nrec as u16).to_le_bytes()); + push_uint(&mut out, n, length_size as usize); + let sum = jenkins_lookup3(&out); + out.extend_from_slice(&sum.to_le_bytes()); + debug_assert_eq!(out.len(), hdr_len); + for node in &w.nodes { + out.extend_from_slice(node); + } + Ok(out) +} + +/// A written node, as its parent points at it. +#[derive(Debug, Clone, Copy)] +struct NodeRef { + addr: u64, + /// Records in the node itself. + nrec: u64, + /// Records in the subtree it roots. + all_nrec: u64, +} + +struct TreeWriter<'a> { + p: BTreeV2Params, + records: &'a [u8], + info: &'a [NodeInfo], + /// Width of a child's record count: what a leaf's maximum needs. + nrec_width: usize, + offset_size: u8, + /// Address of the first node (right after the header). + first_node: u64, + /// Nodes in file order (children before their parent). + nodes: Vec>, +} + +impl TreeWriter<'_> { + fn record(&self, i: usize) -> &[u8] { + let rs = usize::from(self.p.record_size); + &self.records[i * rs..(i + 1) * rs] + } + + fn push_node(&mut self, mut node: Vec) -> u64 { + // The checksum covers the node up to it, not the padding after. + let sum = jenkins_lookup3(&node); + node.extend_from_slice(&sum.to_le_bytes()); + debug_assert!(node.len() <= self.p.node_size as usize); + node.resize(self.p.node_size as usize, 0); + let addr = self.first_node + self.nodes.len() as u64 * u64::from(self.p.node_size); + self.nodes.push(node); + addr + } + + /// Write the subtree of `depth` holding records `first..first + n`. + fn node(&mut self, depth: u16, first: usize, n: usize) -> Result { + let rs = usize::from(self.p.record_size); + let mut node = Vec::with_capacity(self.p.node_size as usize); + if depth == 0 { + debug_assert!(n as u64 <= self.info[0].max_nrec); + node.extend_from_slice(b"BTLF"); + node.push(0); // version + node.push(self.p.tree_type); + node.extend_from_slice(&self.records[first * rs..(first + n) * rs]); + let addr = self.push_node(node); + return Ok(NodeRef { + addr, + nrec: n as u64, + all_nrec: n as u64, + }); + } + + // As few children as hold the records, at least two, with the + // records spread evenly: `k` children and `k - 1` records between + // them. + let below = self.info[usize::from(depth) - 1].cum_max_nrec; + let k = (n as u64 + 1).div_ceil(below + 1).max(2); + let max = self.info[usize::from(depth)].max_nrec; + if k - 1 > max || (n as u64) < k - 1 + k { + return Err(FormatError::SerializationError(format!( + "cannot spread {n} B-tree v2 records over {k} children at depth {depth}" + ))); + } + let k = k as usize; + let in_children = n - (k - 1); + let (base, extra) = (in_children / k, in_children % k); + + let mut children = Vec::with_capacity(k); + let mut separators = Vec::with_capacity(k - 1); + let mut next = first; + for c in 0..k { + let m = base + usize::from(c < extra); + children.push(self.node(depth - 1, next, m)?); + next += m; + if c + 1 < k { + separators.push(next); + next += 1; + } + } + debug_assert_eq!(next, first + n); + + node.extend_from_slice(b"BTIN"); + node.push(0); // version + node.push(self.p.tree_type); + for &s in &separators { + node.extend_from_slice(self.record(s)); + } + let total_width = if depth > 1 { + self.info[usize::from(depth) - 1].cum_max_nrec_size + } else { + 0 + }; + for c in &children { + push_uint(&mut node, c.addr, self.offset_size as usize); + push_uint(&mut node, c.nrec, self.nrec_width); + if depth > 1 { + push_uint(&mut node, c.all_nrec, total_width); + } + } + let addr = self.push_node(node); + Ok(NodeRef { + addr, + nrec: (k - 1) as u64, + all_nrec: n as u64, + }) + } +} + +/// Append `v` as a `width`-byte little-endian integer. +fn push_uint(buf: &mut Vec, v: u64, width: usize) { + let bytes = v.to_le_bytes(); + buf.extend_from_slice(&bytes[..width.min(8)]); + buf.extend(vec![0u8; width.saturating_sub(8)]); +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::btree_v2::{BTreeV2Header, collect_btree_v2_records}; + + fn params(node_size: u32, record_size: u16) -> BTreeV2Params { + BTreeV2Params { + tree_type: 5, + node_size, + record_size, + split_percent: 100, + merge_percent: 40, + } + } + + /// `n` 11-byte records: a big-endian counter, so byte order is key order. + fn records(n: usize, rs: usize) -> Vec { + let mut out = Vec::with_capacity(n * rs); + for i in 0..n { + let mut r = vec![0u8; rs]; + r[..8].copy_from_slice(&(i as u64).to_be_bytes()); + out.extend_from_slice(&r); + } + out + } + + fn roundtrip(node_size: u32, rs: u16, n: usize, os: u8, ls: u8) -> BTreeV2Header { + let recs = records(n, usize::from(rs)); + let base = 4096u64; + let tree = build_btree_v2(params(node_size, rs), &recs, base, os, ls).unwrap(); + let mut file = vec![0u8; base as usize]; + file.extend_from_slice(&tree); + let hdr = BTreeV2Header::parse(&file, base as usize, os, ls).unwrap(); + assert_eq!(hdr.total_records, n as u64); + let got = collect_btree_v2_records(&file, &hdr, os, ls).unwrap(); + assert_eq!(got.len(), n); + let flat: Vec = got.into_iter().flat_map(|r| r.data).collect(); + assert_eq!(flat, recs, "node {node_size} rs {rs} n {n}"); + hdr + } + + #[test] + fn one_leaf_then_deeper_trees_read_back_in_order() { + // 512-byte nodes of 11-byte records: 45 per leaf, 1149 at depth 1, + // 26 449 at depth 2. + let info = node_info(512, 11, 8, 3); + assert_eq!( + info.iter().map(|i| i.cum_max_nrec).collect::>(), + [45, 1149, 26_449, 608_349] + ); + for (n, depth) in [ + (0, 0), + (1, 0), + (45, 0), + (46, 1), + (1149, 1), + (1150, 2), + (26_449, 2), + (26_450, 3), + (100_000, 3), + ] { + let hdr = roundtrip(512, 11, n, 8, 8); + assert_eq!(hdr.depth, depth, "{n} records"); + } + } + + #[test] + fn pointer_widths_follow_the_offset_and_length_sizes() { + for (os, ls) in [(4, 4), (8, 4), (4, 8), (2, 2)] { + roundtrip(512, 11, 5000, os, ls); + } + // Wide counts: a leaf of 2048 bytes / 9-byte records (226, one byte) + // and deeper subtree totals of three bytes. + roundtrip(2048, 9, 300_000, 8, 8); + } + + #[test] + fn every_node_is_within_its_capacity_and_above_the_merge_threshold() { + let rs = 17u16; + let n = 70_000usize; + let info = node_info(512, rs, 8, 3); + let recs = records(n, usize::from(rs)); + let tree = build_btree_v2(params(512, rs), &recs, 0, 8, 8).unwrap(); + let hdr_len = header_size(8, 8); + let nodes = (tree.len() - hdr_len) / 512; + for i in 0..nodes { + let node = &tree[hdr_len + i * 512..hdr_len + (i + 1) * 512]; + let sig = &node[..4]; + if sig == b"BTLF" { + continue; // counts checked through the parents below + } + assert_eq!(sig, b"BTIN"); + } + // Walk from the header: each child's count within [40%, 100%]. + let hdr = BTreeV2Header::parse(&tree, 0, 8, 8).unwrap(); + assert_eq!(hdr.depth, 3); + assert!(u64::from(hdr.num_records_in_root) <= info[3].max_nrec); + fn walk(tree: &[u8], addr: usize, nrec: usize, depth: usize, info: &[NodeInfo], rs: usize) { + if depth == 0 { + return; + } + let nrec_w = bytes_for_max_records(info[0].max_nrec); + let tot_w = if depth > 1 { + info[depth - 1].cum_max_nrec_size + } else { + 0 + }; + let mut pos = addr + 6 + nrec * rs; + for _ in 0..=nrec { + let a = u64::from_le_bytes(tree[pos..pos + 8].try_into().unwrap()) as usize; + pos += 8; + let mut c = 0usize; + for b in 0..nrec_w { + c |= usize::from(tree[pos + b]) << (8 * b); + } + pos += nrec_w + tot_w; + let max = info[depth - 1].max_nrec as usize; + assert!(c <= max && c * 100 > max * 40, "{c} of {max}"); + walk(tree, a, c, depth - 1, info, rs); + } + } + walk( + &tree, + hdr.root_node_address as usize, + usize::from(hdr.num_records_in_root), + 3, + &info, + usize::from(rs), + ); + assert!(nodes > 0); + } + + #[test] + fn a_node_too_small_or_too_big_is_an_error() { + assert!(build_btree_v2(params(16, 11), &records(1, 11), 0, 8, 8).is_err()); + // A leaf with room for more than 65 535 records. + assert!(build_btree_v2(params(1 << 20, 11), &records(1, 11), 0, 8, 8).is_err()); + // Records that are not whole. + assert!(build_btree_v2(params(512, 11), &[0u8; 12], 0, 8, 8).is_err()); + } +} diff --git a/crates/clawhdf5-format/src/chunked_write.rs b/crates/clawhdf5-format/src/chunked_write.rs index 05383a6..2a8bd09 100644 --- a/crates/clawhdf5-format/src/chunked_write.rs +++ b/crates/clawhdf5-format/src/chunked_write.rs @@ -6,6 +6,7 @@ extern crate alloc; #[cfg(not(feature = "std"))] use alloc::{format, vec, vec::Vec}; +use crate::btree_v2_write::{BTreeV2Params, build_btree_v2}; use crate::checksum::jenkins_lookup3; use crate::chunk_cache::{CACHE_LINE_SIZE, align_to_cache_line}; use crate::chunk_grid::ChunkGrid; @@ -1105,11 +1106,12 @@ const BT2_CHUNK_FILTERED: u8 = 11; /// /// `records` are `(scaled coordinates, chunk)` in lexicographic order of the /// coordinates, which is the order the library's comparator -/// (`H5VM_vector_cmp_u`) keeps them in. The tree is a single leaf: the -/// library's 2048-byte node when the records fit, otherwise a leaf node -/// sized to hold them all (the root's record count is 16-bit, so at most -/// 65535 chunks). Returns the bytes and the node size the layout message -/// must record. +/// (`H5VM_vector_cmp_u`) keeps them in. Up to 65 535 chunks go in a single +/// leaf: the library's 2048-byte node when the records fit, otherwise a leaf +/// node sized to hold them all (the layout the writer has always used, kept +/// so those files do not change). More chunks get the library's 2048-byte +/// nodes with internal nodes above the leaves. Returns the bytes and the +/// node size the layout message must record. fn build_btree_v2_chunk_index_at( rank: usize, records: &[(Vec, &WrittenChunk)], @@ -1119,73 +1121,49 @@ fn build_btree_v2_chunk_index_at( base_address: u64, ) -> Result<(Vec, u32), FormatError> { let os = offset_size as usize; - let nrec = u16::try_from(records.len()).map_err(|_| { - FormatError::ChunkedReadError( - "more than 65535 chunks with more than one unlimited dimension: \ - use larger chunks" - .into(), - ) - })?; let chunk_size_bytes = has_filters.then(|| { let slots: Vec> = records.iter().map(|(_, c)| Some((*c).clone())).collect(); filtered_chunk_size_len(&slots) }); let record_size = os + chunk_size_bytes.map_or(0, |n| n + 4) + 8 * rank; - // Leaf: signature, version, type, records, checksum. - let leaf_len = 4 + 1 + 1 + records.len() * record_size + 4; - let node_size = u32::try_from(leaf_len) - .map_err(|_| FormatError::Overflow("B-tree v2 leaf size".into()))? - .max(BT2_NODE_SIZE); + let record_size_u16 = u16::try_from(record_size) + .map_err(|_| FormatError::Overflow("B-tree v2 record size".into()))?; + let node_size = if records.len() <= usize::from(u16::MAX) { + // Leaf: signature, version, type, records, checksum. + let leaf_len = 4 + 1 + 1 + records.len() * record_size + 4; + u32::try_from(leaf_len) + .map_err(|_| FormatError::Overflow("B-tree v2 leaf size".into()))? + .max(BT2_NODE_SIZE) + } else { + BT2_NODE_SIZE + }; let tree_type = if has_filters { BT2_CHUNK_FILTERED } else { BT2_CHUNK_UNFILTERED }; - let hdr_len = 4 + 1 + 1 + 4 + 2 + 2 + 1 + 1 + os + 2 + length_size as usize + 4; - let leaf_address = base_address + hdr_len as u64; - - let mut out = Vec::with_capacity(hdr_len + node_size as usize); - out.extend_from_slice(b"BTHD"); - out.push(0); // version - out.push(tree_type); - out.extend_from_slice(&node_size.to_le_bytes()); - out.extend_from_slice(&(record_size as u16).to_le_bytes()); - out.extend_from_slice(&0u16.to_le_bytes()); // depth - out.push(BT2_SPLIT_PERCENT); - out.push(BT2_MERGE_PERCENT); - if records.is_empty() { - out.extend(core::iter::repeat_n(0xFF, os)); - } else { - push_addr(&mut out, leaf_address, offset_size); - } - out.extend_from_slice(&nrec.to_le_bytes()); - match length_size { - 4 => out.extend_from_slice(&(records.len() as u32).to_le_bytes()), - _ => out.extend_from_slice(&(records.len() as u64).to_le_bytes()), - } - let sum = jenkins_lookup3(&out); - out.extend_from_slice(&sum.to_le_bytes()); - debug_assert_eq!(out.len(), hdr_len); - if records.is_empty() { - return Ok((out, node_size)); - } - - let leaf_start = out.len(); - out.extend_from_slice(b"BTLF"); - out.push(0); // version - out.push(tree_type); + let mut flat = Vec::with_capacity(records.len() * record_size); for (scaled, chunk) in records { - push_index_element(&mut out, Some(chunk), offset_size, chunk_size_bytes); + push_index_element(&mut flat, Some(chunk), offset_size, chunk_size_bytes); for &c in scaled { - out.extend_from_slice(&c.to_le_bytes()); + flat.extend_from_slice(&c.to_le_bytes()); } } - let sum = jenkins_lookup3(&out[leaf_start..]); - out.extend_from_slice(&sum.to_le_bytes()); - // The library reads whole nodes; pad the leaf out to the node size. - out.resize(leaf_start + node_size as usize, 0); + let out = build_btree_v2( + BTreeV2Params { + tree_type, + node_size, + record_size: record_size_u16, + split_percent: BT2_SPLIT_PERCENT, + merge_percent: BT2_MERGE_PERCENT, + }, + &flat, + base_address, + offset_size, + length_size, + )?; Ok((out, node_size)) } diff --git a/crates/clawhdf5-format/src/file_writer.rs b/crates/clawhdf5-format/src/file_writer.rs index 80a296b..a9b3af5 100644 --- a/crates/clawhdf5-format/src/file_writer.rs +++ b/crates/clawhdf5-format/src/file_writer.rs @@ -7,6 +7,7 @@ use alloc::{format, vec, vec::Vec}; use crate::attribute::AttributeMessage; +use crate::btree_v2_write::{BTreeV2Params, build_btree_v2}; use crate::chunked_write::{ ChunkOptions, PrecompressedChunks, build_chunked_data_from_precompressed, precompress_chunks, }; @@ -933,13 +934,7 @@ pub(crate) fn build_dense_attrs( .collect(); let bthd_addr = btree_addr; let mut blob = heap.blob; - blob.extend_from_slice(&single_leaf_v2_btree( - 8, - record_size, - &records, - bthd_addr, - "attributes on one object", - )?); + blob.extend_from_slice(&dense_v2_btree(8, record_size, &records, bthd_addr)?); let attr_info = serialize_attribute_info(frhp_addr, bthd_addr); @@ -960,69 +955,57 @@ pub(crate) struct DenseLinkBlob { pub(crate) blob: Vec, } -/// A v2 B-tree of `btree_type` holding `records` (already in key order) in a -/// single leaf, laid out at `addr`: the header, then the leaf. `what` names -/// the records in the error for too many ("links in one group"). -fn single_leaf_v2_btree( +/// libhdf5's node size for the dense link and attribute indexes it creates +/// (`H5G_NAME_BT2_NODE_SIZE`, `H5A_NAME_BT2_NODE_SIZE`, and the +/// creation-order indexes'), with their split and merge percentages. +const DENSE_BT2_NODE_SIZE: u32 = 512; +const DENSE_BT2_SPLIT_PERCENT: u8 = 100; +const DENSE_BT2_MERGE_PERCENT: u8 = 40; + +/// A dense-storage v2 B-tree of `btree_type` holding `records` (already in +/// key order), laid out at `addr`: the header, then its nodes. +/// +/// Up to 65 535 records go in one leaf node sized to hold them (the layout +/// the writer has always used, kept so those files do not change). A leaf's +/// record count is a 2-byte field, and libhdf5 sizes a leaf's capacity from +/// the node size: a node with room for more than 65 535 records makes it +/// overflow that count when it adds one, so the node is capped at a full +/// leaf. More records get libhdf5's own 512-byte nodes, with internal nodes +/// above the leaves. +fn dense_v2_btree( btree_type: u8, record_size: u16, records: &[Vec], addr: u64, - what: &str, ) -> Result, FormatError> { - let os = OFFSET_SIZE as usize; - let ls = LENGTH_SIZE as usize; - // The root node's record count is a 2-byte field; more records need - // internal nodes, which the writer does not build. - let num_records = u16::try_from(records.len()).map_err(|_| { - FormatError::SerializationError(format!( - "{} {what}: at most {} can be written \ - (a deeper B-tree index is not implemented)", - records.len(), - u16::MAX - )) - })?; - let bthd_size = 4 + 1 + 1 + 4 + 2 + 2 + 1 + 1 + os + 2 + ls + 4; - let btlf_size = 4 + 1 + 1 + (records.len() * record_size as usize) + 4; - // libhdf5 sizes a leaf's capacity from the node size, and a leaf's - // record count is a 2-byte field: a node with room for more than - // 65 535 records makes it overflow that count when it adds one (the - // group can then no longer be listed). Cap the node at a full leaf. - let max_node = btlf_size - records.len() * record_size as usize - + usize::from(u16::MAX) * record_size as usize; - let node_size = btlf_size.next_power_of_two().max(512).min(max_node) as u32; - let btlf_addr = addr + bthd_size as u64; - - let mut out = Vec::with_capacity(bthd_size + node_size as usize); - out.extend_from_slice(b"BTHD"); - out.push(0); // version - out.push(btree_type); - out.extend_from_slice(&node_size.to_le_bytes()); - out.extend_from_slice(&record_size.to_le_bytes()); - out.extend_from_slice(&0u16.to_le_bytes()); // depth = 0 (single leaf) - out.push(100); // split_percent - out.push(40); // merge_percent - write_offset(&mut out, btlf_addr, OFFSET_SIZE); - out.extend_from_slice(&num_records.to_le_bytes()); - write_length(&mut out, records.len() as u64, LENGTH_SIZE); - let checksum = crate::checksum::jenkins_lookup3(&out); - out.extend_from_slice(&checksum.to_le_bytes()); - debug_assert_eq!(out.len(), bthd_size); - - let mut btlf = Vec::with_capacity(node_size as usize); - btlf.extend_from_slice(b"BTLF"); - btlf.push(0); // version - btlf.push(btree_type); - for rec in records { - debug_assert_eq!(rec.len(), record_size as usize); - btlf.extend_from_slice(rec); - } - // The checksum follows the records, not the end of the node. - let checksum = crate::checksum::jenkins_lookup3(&btlf); - btlf.extend_from_slice(&checksum.to_le_bytes()); - btlf.resize(node_size as usize, 0); - out.extend_from_slice(&btlf); - Ok(out) + let rs = usize::from(record_size); + let n = records.len(); + let node_size = if n <= usize::from(u16::MAX) { + let btlf_size = 4 + 1 + 1 + n * rs + 4; + let max_node = 4 + 1 + 1 + usize::from(u16::MAX) * rs + 4; + u32::try_from(btlf_size.next_power_of_two().max(512).min(max_node)) + .map_err(|_| FormatError::Overflow("B-tree v2 node size".into()))? + } else { + DENSE_BT2_NODE_SIZE + }; + let flat: Vec = records + .iter() + .inspect(|r| debug_assert_eq!(r.len(), rs)) + .flat_map(|r| r.iter().copied()) + .collect(); + build_btree_v2( + BTreeV2Params { + tree_type: btree_type, + node_size, + record_size, + split_percent: DENSE_BT2_SPLIT_PERCENT, + merge_percent: DENSE_BT2_MERGE_PERCENT, + }, + &flat, + addr, + OFFSET_SIZE, + LENGTH_SIZE, + ) } /// Build dense link storage for a group's links, laid out at `base_address`. @@ -1068,12 +1051,11 @@ pub(crate) fn build_dense_links( .collect(); let name_bt_addr = heap.btree_addr; let mut blob = heap.blob; - blob.extend_from_slice(&single_leaf_v2_btree( + blob.extend_from_slice(&dense_v2_btree( 5, 4 + heap_id_length, &name_records, name_bt_addr, - "links in one group", )?); let link_info_message = if track_order { @@ -1093,12 +1075,11 @@ pub(crate) fn build_dense_links( }) .collect(); let order_bt_addr = base_address + blob.len() as u64; - blob.extend_from_slice(&single_leaf_v2_btree( + blob.extend_from_slice(&dense_v2_btree( 6, 8 + heap_id_length, &order_records, order_bt_addr, - "links in one group", )?); let next_order = by_order.last().map_or(0, |&(o, _)| o + 1); serialize_link_info( diff --git a/crates/clawhdf5-format/src/lib.rs b/crates/clawhdf5-format/src/lib.rs index 3e30f9d..92954bc 100644 --- a/crates/clawhdf5-format/src/lib.rs +++ b/crates/clawhdf5-format/src/lib.rs @@ -61,6 +61,7 @@ pub mod attribute; pub mod attribute_info; pub mod btree_v1; pub mod btree_v2; +mod btree_v2_write; mod bulk_alloc; pub mod checksum; pub mod chunk_cache; diff --git a/crates/clawhdf5-tools/tests/h5rs_interop.rs b/crates/clawhdf5-tools/tests/h5rs_interop.rs index fcd205a..32f6fb2 100644 --- a/crates/clawhdf5-tools/tests/h5rs_interop.rs +++ b/crates/clawhdf5-tools/tests/h5rs_interop.rs @@ -1024,3 +1024,75 @@ fn check_files_with_big_dense_storage() { assert_eq!(code(&o), 0, "{p}:\n{}", stdout(&o)); assert!(stdout(&o).contains("no problems found"), "{}", stdout(&o)); } + +/// `(type, depth)` of every v2 B-tree header in a file written with 8-byte +/// offsets and lengths (found by signature and checksum). +fn btree_v2_depths(data: &[u8]) -> Vec<(u8, u16)> { + const LEN: usize = 4 + 1 + 1 + 4 + 2 + 2 + 1 + 1 + 8 + 2 + 8; + let mut out = Vec::new(); + for at in 0..data.len().saturating_sub(LEN + 4) { + if &data[at..at + 4] != b"BTHD" { + continue; + } + let stored = u32::from_le_bytes(data[at + LEN..at + LEN + 4].try_into().unwrap()); + if jenkins_lookup3(&data[at..at + LEN]) == stored { + out.push(( + data[at + 5], + u16::from_le_bytes([data[at + 12], data[at + 13]]), + )); + } + } + out +} + +#[test] +fn check_files_with_deep_btrees() { + // Dense indexes and a chunk index too big for one leaf: the writer then + // builds internal nodes, whose child pointers carry record counts in + // widths derived from the node size. `check` reads every record through + // them and compares the count with the header's. + use clawhdf5::{AttrValue, FileBuilder}; + const U: u64 = u64::MAX; + let dir = tempfile::tempdir().unwrap(); + let mut b = FileBuilder::new(); + let x = b.create_dataset("x"); + x.with_i32_data(&[7]); + for i in 0..70_000 { + x.set_attr(&format!("attr_{i}"), AttrValue::I64(i)); + } + let mut g = b.create_group("g"); + g.track_order(true); + for i in 0..100_000 { + g.add_hard_link(&format!("k{i}"), "/x"); + } + b.add_group(g.finish()); + let p = dir.path().join("deep.h5").to_string_lossy().into_owned(); + b.write(&p).unwrap(); + let o = h5rs(&["check", &p]); + assert_eq!(code(&o), 0, "{p}:\n{}", stdout(&o)); + assert!(stdout(&o).contains("no problems found"), "{}", stdout(&o)); + let mut depths = btree_v2_depths(&std::fs::read(&p).unwrap()); + depths.sort(); + assert_eq!(depths, [(5, 3), (6, 3), (8, 3)]); + + let mut b = FileBuilder::new(); + b.create_dataset("d") + .with_i32_data(&(0..200_000).collect::>()) + .with_shape(&[400, 500]) + .with_chunks(&[1, 1]) + .with_maxshape(&[U, U]); + b.create_dataset("z") + .with_i32_data(&(0..70_000).collect::>()) + .with_shape(&[70, 1000]) + .with_chunks(&[1, 1]) + .with_maxshape(&[U, U]) + .with_deflate(1); + let p = dir.path().join("chunks.h5").to_string_lossy().into_owned(); + b.write(&p).unwrap(); + let o = h5rs(&["check", "--data", &p]); + assert_eq!(code(&o), 0, "{p}:\n{}", stdout(&o)); + assert!(stdout(&o).contains("no problems found"), "{}", stdout(&o)); + let mut depths = btree_v2_depths(&std::fs::read(&p).unwrap()); + depths.sort(); + assert_eq!(depths, [(10, 2), (11, 2)]); +} diff --git a/crates/clawhdf5/tests/chunk_index_interop.rs b/crates/clawhdf5/tests/chunk_index_interop.rs index 3ff0420..96ecffb 100644 --- a/crates/clawhdf5/tests/chunk_index_interop.rs +++ b/crates/clawhdf5/tests/chunk_index_interop.rs @@ -559,20 +559,6 @@ fn we_write_btree_v2_for_several_unlimited_dims() { check_we_write(&cases); } -/// A single-leaf B-tree has a 16-bit record count; beyond it the writer -/// refuses rather than writing a tree libhdf5 would misread. -#[test] -fn btree_v2_index_past_one_leaf_is_refused() { - let mut b = FileBuilder::new(); - b.create_dataset("d") - .with_i32_data(&vec![0i32; 70_000]) - .with_shape(&[70_000, 1]) - .with_chunks(&[1, 1]) - .with_maxshape(&[u64::MAX, u64::MAX]); - let dir = tempfile::tempdir().unwrap(); - assert!(b.write(dir.path().join("too_many.h5")).is_err()); -} - /// A maxshape equal to the shape cannot grow, so it needs no chunks: the /// dataset stays contiguous (as h5py makes it) unless chunks are requested. #[test] diff --git a/crates/clawhdf5/tests/deep_btree_interop.rs b/crates/clawhdf5/tests/deep_btree_interop.rs new file mode 100644 index 0000000..64b6e9d --- /dev/null +++ b/crates/clawhdf5/tests/deep_btree_interop.rs @@ -0,0 +1,343 @@ +//! Version-2 B-trees deeper than one leaf, as `FileBuilder` writes them for +//! big dense indexes: a group's links (name index, type 5, and creation +//! order index, type 6), an object's attributes (name index, type 8) and +//! the chunk index of a dataset with two unlimited dimensions (type 10 and, +//! with a filter, 11). Read back by h5py (libhdf5), h5dump and clawhdf5, +//! then modified by h5py in "r+" mode, which splits, merges and +//! redistributes the nodes the writer built. +//! +//! Skipped when python3 with h5py is unavailable, unless +//! `CLAWHDF5_REQUIRE_INTEROP=1`. + +use std::process::Command; + +use clawhdf5::{AttrValue, File, FileBuilder}; + +fn python() -> String { + std::env::var("CLAWHDF5_PYTHON").unwrap_or_else(|_| "python3".to_string()) +} + +fn interop_required() -> bool { + std::env::var("CLAWHDF5_REQUIRE_INTEROP").is_ok_and(|v| v == "1") +} + +fn python_available() -> bool { + Command::new(python()) + .args(["-c", "import h5py"]) + .output() + .map(|o| o.status.success()) + .unwrap_or(false) +} + +macro_rules! skip_if_no_python { + () => { + if !python_available() { + assert!( + !interop_required(), + "CLAWHDF5_REQUIRE_INTEROP=1 but python3 with h5py is not available" + ); + eprintln!("SKIP: python3 with h5py not available"); + return; + } + }; +} + +/// Run `body` under h5py with `path` bound to the file's path. +fn h5py(path: &str, body: &str) -> String { + let script = format!("import h5py, numpy as np, json\npath = r'{path}'\n{body}"); + let output = Command::new(python()) + .args(["-c", &script]) + .output() + .expect("failed to run python"); + if !output.status.success() { + panic!( + "Python script failed:\nSTDOUT: {}\nSTDERR: {}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + } + String::from_utf8_lossy(&output.stdout).trim().to_string() +} + +/// h5dump of `args` must succeed; returns its output. +fn h5dump(args: &[&str]) -> String { + let ok = Command::new("h5dump") + .arg("--version") + .output() + .is_ok_and(|o| o.status.success()); + if !ok { + assert!(!interop_required(), "h5dump is not available"); + return String::new(); + } + let o = Command::new("h5dump").args(args).output().unwrap(); + let out = String::from_utf8_lossy(&o.stdout).to_string(); + assert!( + o.status.success(), + "h5dump {args:?} failed:\n{out}{}", + String::from_utf8_lossy(&o.stderr) + ); + out +} + +// ---- 100 000 links in one group ---- + +const NLINKS: usize = 100_000; + +/// The compact names: `k0`..`k99998` and `k155448`, whose name hash equals +/// that of `k69209` — a collision inside a many-level name index. +fn compact_name(i: usize) -> String { + if i == NLINKS - 1 { + "k155448".into() + } else { + format!("k{i}") + } +} + +/// The long names (111 bytes), added in a scrambled order so creation order +/// is not name order: the `i`th created is `long_name(scramble(i))`. +fn long_name(j: usize) -> String { + format!("link_{j:06}_{}", "x".repeat(100)) +} + +fn scramble(i: usize) -> usize { + i * 7919 % NLINKS +} + +const PY_NAMES: &str = "\ +N = 100000\n\ +compact = ['k%d' % i for i in range(N - 1)] + ['k155448']\n\ +def long_name(j): return 'link_%06d_' % j + 'x' * 100\n\ +created = [long_name(i * 7919 % N) for i in range(N)]\n"; + +#[test] +fn a_hundred_thousand_links_in_one_group() { + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("links.h5").display().to_string(); + let mut b = FileBuilder::new(); + for v in 0..10 { + b.create_dataset(&format!("v{v}")).with_i32_data(&[v]); + } + // Name index only (type 5), 11-byte records: depth 3 in 512-byte nodes. + let mut g = b.create_group("compact"); + for i in 0..NLINKS { + g.add_hard_link(&compact_name(i), &format!("/v{}", i % 10)); + } + b.add_group(g.finish()); + // Creation order tracked and indexed: a type-6 index as well. + let mut g = b.create_group("long"); + g.track_order(true); + for i in 0..NLINKS { + let j = scramble(i); + g.add_hard_link(&long_name(j), &format!("/v{}", j % 10)); + } + b.add_group(g.finish()); + b.write(&path).unwrap(); + + let check = format!( + "{PY_NAMES}\ + with h5py.File(path, 'r') as f:\n\ + \x20 g, l = f['compact'], f['long']\n\ + \x20 out = [len(g), list(g) == sorted(compact), len(l), list(l) == created]\n\ + \x20 out.append(all(int(g[compact[i]][0]) == i % 10 for i in range(0, N, 997)))\n\ + \x20 out.append([int(g['k69209'][0]), int(g['k155448'][0]), 'k155448' in g, 'k100000' in g])\n\ + \x20 out.append(all(int(l[long_name(j)][0]) == j % 10 for j in range(0, N, 1009)))\n\ + \x20 out.append(h5py.h5o.get_info(f['v3'].id).rc)\n\ + \x20 print(json.dumps(out))" + ); + assert_eq!( + h5py(&path, &check), + "[100000, true, 100000, true, true, [9, 9, true, false], true, 20001]" + ); + let d = h5dump(&["-d", "/compact/k155448", &path]); + assert!(d.is_empty() || d.contains("(0): 9"), "{d}"); + let d = h5dump(&["-d", &format!("/long/{}", long_name(99_999)), &path]); + assert!(d.is_empty() || d.contains("(0): 9"), "{d}"); + + // clawhdf5 reads both indexes back. + let f = File::open(&path).unwrap(); + let g = f.group("compact").unwrap(); + let mut names = g.datasets().unwrap(); + names.sort(); + let mut want: Vec = (0..NLINKS).map(compact_name).collect(); + want.sort(); + assert_eq!(names, want); + assert_eq!(g.dataset("k155448").unwrap().read_i32().unwrap(), [9]); + let l = f.group("long").unwrap(); + assert_eq!(l.datasets().unwrap().len(), NLINKS); + assert_eq!( + l.dataset(&long_name(12_345)).unwrap().read_i32().unwrap(), + [5] + ); + drop(f); + + // libhdf5 inserts into and removes from the trees we wrote. + let modify = format!( + "{PY_NAMES}\ + with h5py.File(path, 'r+') as f:\n\ + \x20 g, l = f['compact'], f['long']\n\ + \x20 for i in range(3000):\n\ + \x20 g['new%d' % i] = f['v1']\n\ + \x20 for i in range(0, N, 7):\n\ + \x20 del g[compact[i]]\n\ + \x20 l['zz_new'] = f['v2']\n\ + \x20 for j in range(0, N, 3):\n\ + \x20 del l[long_name(j)]\n\ + with h5py.File(path, 'r') as f:\n\ + \x20 g, l = f['compact'], f['long']\n\ + \x20 left = sorted([n for i, n in enumerate(compact) if i % 7] + ['new%d' % i for i in range(3000)])\n\ + \x20 kept = [n for n in created if int(n[5:11]) % 3] + ['zz_new']\n\ + \x20 print(json.dumps([len(g), list(g) == left, int(g['new2999'][0]),\n\ + \x20 int(g['k155448'][0]), len(l), list(l) == kept, int(l['zz_new'][0])]))" + ); + assert_eq!(h5py(&path, &modify), "[88714, true, 1, 9, 66667, true, 2]"); + h5dump(&["-d", "/compact/new0", &path]); + let f = File::open(&path).unwrap(); + assert_eq!( + f.group("compact").unwrap().datasets().unwrap().len(), + 88_714 + ); + assert_eq!(f.group("long").unwrap().datasets().unwrap().len(), 66_667); +} + +// ---- 70 000 attributes on one object ---- + +#[test] +fn seventy_thousand_attributes_on_one_object() { + skip_if_no_python!(); + const N: i64 = 70_000; + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("attrs.h5").display().to_string(); + let mut b = FileBuilder::new(); + let x = b.create_dataset("x"); + x.with_i32_data(&[1]); + for i in 0..N { + x.set_attr(&format!("attr_{i}"), AttrValue::I64(i * 3)); + } + let mut g = b.create_group("g"); + for i in 0..N { + g.set_attr(&format!("s{i:05}"), AttrValue::String(format!("value {i}"))); + } + b.add_group(g.finish()); + b.write(&path).unwrap(); + + let check = "\ + N = 70000\n\ + with h5py.File(path, 'r') as f:\n\ + \x20 a, s = f['x'].attrs, f['g'].attrs\n\ + \x20 names = list(a)\n\ + \x20 out = [len(a), names == sorted('attr_%d' % i for i in range(N))]\n\ + \x20 out.append(all(int(a['attr_%d' % i]) == 3 * i for i in range(0, N, 331)))\n\ + \x20 out.append(len(s))\n\ + \x20 v = dict(s.items())\n\ + \x20 out.append(all(v['s%05d' % i].decode() == 'value %d' % i for i in range(N)))\n\ + \x20 print(json.dumps(out))"; + assert_eq!(h5py(&path, check), "[70000, true, true, 70000, true]"); + let d = h5dump(&["-a", "/x/attr_69999", &path]); + assert!(d.is_empty() || d.contains("(0): 209997"), "{d}"); + + let f = File::open(&path).unwrap(); + let attrs = f.dataset("x").unwrap().attrs().unwrap(); + assert_eq!(attrs.len(), N as usize); + for i in [0, 1, 35_000, N - 1] { + assert!( + matches!(attrs[&format!("attr_{i}")], AttrValue::I64(v) if v == 3 * i), + "attr_{i}" + ); + } + let attrs = f.group("g").unwrap().attrs().unwrap(); + assert_eq!(attrs.len(), N as usize); + drop(f); + + let modify = "\ + N = 70000\n\ + with h5py.File(path, 'r+') as f:\n\ + \x20 a = f['x'].attrs\n\ + \x20 for i in range(2000):\n\ + \x20 a['new_%d' % i] = i\n\ + \x20 for i in range(0, N, 5):\n\ + \x20 del a['attr_%d' % i]\n\ + \x20 f['g'].attrs['s00000'] = 'changed'\n\ + with h5py.File(path, 'r') as f:\n\ + \x20 a = f['x'].attrs\n\ + \x20 want = sorted(['attr_%d' % i for i in range(N) if i % 5] + ['new_%d' % i for i in range(2000)])\n\ + \x20 print(json.dumps([len(a), list(a) == want, int(a['attr_69999']), int(a['new_1999']),\n\ + \x20 'attr_5' in a, f['g'].attrs['s00000'], len(f['g'].attrs)]))"; + assert_eq!( + h5py(&path, modify), + r#"[58000, true, 209997, 1999, false, "changed", 70000]"# + ); + let f = File::open(&path).unwrap(); + assert_eq!(f.dataset("x").unwrap().attrs().unwrap().len(), 58_000); +} + +// ---- 200 000 chunks with two unlimited dimensions ---- + +#[test] +fn two_hundred_thousand_chunks_with_two_unlimited_dims() { + skip_if_no_python!(); + const U: u64 = u64::MAX; + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("chunks.h5").display().to_string(); + let data: Vec = (0..200_000).collect(); + let small: Vec = (0..80_000).map(|v| v * 2).collect(); + let mut b = FileBuilder::new(); + // Type 10 (unfiltered): 24-byte records, depth 2 in 2048-byte nodes. + b.create_dataset("d") + .with_i32_data(&data) + .with_shape(&[400, 500]) + .with_chunks(&[1, 1]) + .with_maxshape(&[U, U]); + // Type 11 (filtered): each record also holds a size and filter mask. + b.create_dataset("z") + .with_i32_data(&small) + .with_shape(&[200, 400]) + .with_chunks(&[1, 1]) + .with_maxshape(&[U, U]) + .with_deflate(1); + b.write(&path).unwrap(); + + let check = "\ + with h5py.File(path, 'r') as f:\n\ + \x20 d, z = f['d'], f['z']\n\ + \x20 print(json.dumps([d.shape, d.chunks, d.id.get_num_chunks(),\n\ + \x20 bool(np.array_equal(d[()], np.arange(200000).reshape(400, 500))),\n\ + \x20 int(d[399, 499]), z.id.get_num_chunks(),\n\ + \x20 bool(np.array_equal(z[()], 2 * np.arange(80000).reshape(200, 400)))]))"; + assert_eq!( + h5py(&path, check), + "[[400, 500], [1, 1], 200000, true, 199999, 80000, true]" + ); + let d = h5dump(&["-d", "/d", "-s", "399,498", "-c", "1,2", &path]); + assert!(d.is_empty() || d.contains("199998, 199999"), "{d}"); + + let f = File::open(&path).unwrap(); + assert_eq!(f.dataset("d").unwrap().read_i32().unwrap(), data); + assert_eq!(f.dataset("z").unwrap().read_i32().unwrap(), small); + drop(f); + + // libhdf5 adds chunks to both trees. + let modify = "\ + with h5py.File(path, 'r+') as f:\n\ + \x20 d, z = f['d'], f['z']\n\ + \x20 d.resize((401, 510))\n\ + \x20 d[400, :] = -1\n\ + \x20 d[:, 500:] = -2\n\ + \x20 z.resize((201, 400))\n\ + \x20 z[200, :] = 7\n\ + with h5py.File(path, 'r') as f:\n\ + \x20 d, z = f['d'][()], f['z'][()]\n\ + \x20 print(json.dumps([f['d'].id.get_num_chunks(),\n\ + \x20 bool(np.array_equal(d[:400, :500], np.arange(200000).reshape(400, 500))),\n\ + \x20 int(d[400, 3]), int(d[5, 505]), f['z'].id.get_num_chunks(),\n\ + \x20 bool(np.array_equal(z[:200], 2 * np.arange(80000).reshape(200, 400))), int(z[200, 9])]))"; + assert_eq!( + h5py(&path, modify), + "[204510, true, -1, -2, 80400, true, 7]" + ); + let f = File::open(&path).unwrap(); + let d = f.dataset("d").unwrap().read_i32().unwrap(); + assert_eq!(d.len(), 401 * 510); + assert_eq!(d[499], 499); + assert_eq!(d[400 * 510 + 3], -1); +} diff --git a/crates/clawhdf5/tests/writer_groups_interop.rs b/crates/clawhdf5/tests/writer_groups_interop.rs index 46d155b..d16567c 100644 --- a/crates/clawhdf5/tests/writer_groups_interop.rs +++ b/crates/clawhdf5/tests/writer_groups_interop.rs @@ -531,32 +531,6 @@ fn ten_thousand_links_in_one_group() { ); } -#[test] -fn more_links_than_one_index_leaf_holds_is_an_error() { - let mut b = FileBuilder::new(); - for i in 0..70_000 { - b.add_soft_link(&format!("s{i}"), "/x"); - } - let err = b.finish().unwrap_err().to_string(); - assert!( - err.contains("70000 links in one group: at most 65535"), - "{err}" - ); - // Dense attributes have the same one-leaf index. Their count used to - // be written modulo 65 536. - let mut b = FileBuilder::new(); - let x = b.create_dataset("x"); - x.with_i32_data(&[1]); - for i in 0..70_000 { - x.set_attr(&format!("a{i}"), AttrValue::I64(i)); - } - let err = b.finish().unwrap_err().to_string(); - assert!( - err.contains("70000 attributes on one object: at most 65535"), - "{err}" - ); -} - #[test] fn track_order_lists_members_in_creation_order() { skip_if_no_python!(); @@ -643,7 +617,11 @@ fn names_whose_hashes_collide_are_found_by_name() { .chain((0..10).map(|_| "")) .enumerate() { - let name = if n.is_empty() { format!("d{i}") } else { n.into() }; + let name = if n.is_empty() { + format!("d{i}") + } else { + n.into() + }; g.create_dataset(&name).with_i32_data(&[i as i32]); } b.add_group(g.finish()); diff --git a/docs/known-issues.md b/docs/known-issues.md index 2657cdf..865102b 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -264,10 +264,21 @@ fill-value item that did is fixed). external links at any depth and optional creation-order tracking; h5py, h5dump and `h5rs check --data` read them (`crates/clawhdf5/tests/writer_groups_interop.rs`, - `crates/clawhdf5-tools/tests/h5rs_interop.rs`). Still missing: a group - with more than 65 535 links, or an object with more than 65 535 dense - attributes, is an error (the index is one B-tree leaf), and attribute - creation order is not tracked. + `crates/clawhdf5-tools/tests/h5rs_interop.rs`). Still missing: + attribute creation order is not tracked. + - ~~A group with more than 65 535 links, or an object with more than + 65 535 dense attributes, is an error (the index is one B-tree leaf).~~ + **Fixed 2026-09-26:** the dense indexes are v2 B-trees of any depth + (libhdf5's 512-byte nodes once the records outgrow the one-leaf layout, + which smaller indexes keep byte for byte). Tested on tank with 100 000 + links in one group (short names, and 111-byte names with creation order + tracked) and 70 000 attributes on one object: h5py lists them in order + and reads the values, h5dump reads spot checks, `h5rs check` reads every + record, and h5py in "r+" mode adds and deletes thousands of links and + attributes in those trees (`cargo test -p clawhdf5 --test + deep_btree_interop`; `cargo test -p clawhdf5-tools --test h5rs_interop + check_files_with_deep_btrees`). The name indexes are ordered by hash and + then name, as libhdf5 needs for names whose hashes collide. - ~~Dense link or attribute storage past 512 KiB of messages was written unreadable (child indirect blocks of the fractal heap written as direct blocks).~~ **Fixed 2026-09-26** (it affected 2.7.0 too): tested with @@ -280,8 +291,13 @@ fill-value item that did is fixed). an object, or more than 8 links in a group) one attribute or link message over 65 515 bytes is an error. - Output that HDF5 1.8 can read. - - A B-tree v2 chunk index larger than one leaf, so datasets with several - unlimited dimensions are limited to 65 535 chunks. + - ~~A B-tree v2 chunk index larger than one leaf, so datasets with + several unlimited dimensions are limited to 65 535 chunks.~~ **Fixed + 2026-09-26:** more chunks get libhdf5's 2048-byte nodes with internal + nodes above the leaves. Tested on tank with 200 000 chunks (and 80 000 + deflated): h5py and clawhdf5 read every value, and h5py resizes the + dataset and writes 4 510 new chunks into the tree (same commands as + above). --- From 193a5f8a8267337c8534bb1f6d1b55a575fb3bce Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 10:17:37 -0500 Subject: [PATCH 3/3] writer: track attribute creation order with track_order h5py's track_order=True orders attributes as well as links; the writer tracked links only. A tracking object's header now sets the attribute creation order tracked/indexed flags and carries per-message creation orders, an Attribute Info message holds the next order (inline too), and dense storage gets a type-9 creation-order index. The file default applies to datasets, with DatasetBuilder::track_order per dataset; more than 65 535 attributes on a tracking object is an error (libhdf5's counter is 2 bytes). The reader lists such attributes in creation order. h5py lists them in order (inline, dense, 20 000 on one dataset) and keeps numbering in r+ mode, including its inline-to-dense move. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 16 + crates/clawhdf5-format/src/attribute.rs | 35 +- crates/clawhdf5-format/src/file_writer.rs | 360 ++++++++++++++---- .../src/object_header_writer.rs | 65 +++- crates/clawhdf5-format/src/type_builders.rs | 23 +- crates/clawhdf5-tools/tests/h5rs_interop.rs | 6 + crates/clawhdf5/src/writer.rs | 8 +- .../clawhdf5/tests/writer_groups_interop.rs | 91 +++++ docs/known-issues.md | 12 +- 9 files changed, 515 insertions(+), 101 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 448d2a0..dbecdc4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,22 @@ ## Unreleased ### Writer: large dense indexes (2026-09-26) +- **`track_order` orders attributes too, as h5py's `track_order=True` + does.** It tracked link creation order only, so h5py listed a tracked + object's attributes by name. A tracking object's header now has the + attribute creation order tracked and indexed flags, an Attribute Info + message with the next creation order (also for inline attributes), and a + creation order on each inline attribute message; dense attribute storage + gets a creation-order index (B-tree type 9). `FileWriter::track_order` / + `FileBuilder::track_order` now apply to datasets' attributes as well, and + `DatasetBuilder::track_order` sets it per dataset. Groups and datasets + that track order are written differently from before; others are + unchanged. More than 65 535 attributes on a tracking object is an error + (libhdf5's creation order counter is 2 bytes). The reader + (`attribute::extract_attributes*`) lists a tracking object's attributes + in creation order. Test `track_order_lists_attributes_in_creation_order` + (h5py lists, reads and extends them in "r+" mode, including libhdf5's + move from inline to dense storage). - **No more 65 535-record limit on the writer's v2 B-trees.** Dense link storage (name index and creation-order index), dense attribute storage and the chunk index of datasets with more than one unlimited dimension diff --git a/crates/clawhdf5-format/src/attribute.rs b/crates/clawhdf5-format/src/attribute.rs index 11bde3e..6c4e9ae 100644 --- a/crates/clawhdf5-format/src/attribute.rs +++ b/crates/clawhdf5-format/src/attribute.rs @@ -450,6 +450,8 @@ fn extract_attributes_with( on_error: &mut dyn FnMut(FormatError) -> Result<(), FormatError>, ) -> Result, FormatError> { let mut attrs = Vec::new(); + // Each attribute's creation order, where the file records one. + let mut orders: Vec = Vec::new(); // Collect compact attributes (inline in OH) for msg in &header.messages { @@ -479,7 +481,10 @@ fn extract_attributes_with( }; let attr = attr.and_then(|a| check_in_header(a, header)); match attr { - Ok(attr) => attrs.push(attr), + Ok(attr) => { + attrs.push(attr); + orders.push(msg.creation_order.map_or(0, u32::from)); + } Err(e) => on_error(e)?, } } @@ -487,20 +492,30 @@ fn extract_attributes_with( // Check for dense attributes via AttributeInfo message let attr_info = find_attribute_info(header, offset_size)?; - if let Some(info) = attr_info + if let Some(info) = &attr_info && let Some(fh_addr) = info.fractal_heap_address { extract_dense_attributes( file_data, - &info, + info, fh_addr, offset_size, length_size, &mut attrs, + &mut orders, on_error, )?; } + // An object that tracks attribute creation order lists its attributes + // in that order (h5py's `track_order=True`), as libhdf5 does; otherwise + // they come in storage order. + if attr_info.is_some_and(|i| i.max_creation_index.is_some()) { + let mut paired: Vec<(u32, AttributeMessage)> = orders.into_iter().zip(attrs).collect(); + paired.sort_by_key(|(o, _)| *o); + attrs = paired.into_iter().map(|(_, a)| a).collect(); + } + Ok(attrs) } @@ -518,7 +533,9 @@ fn find_attribute_info( Ok(None) } -/// Extract attributes from dense storage (fractal heap + B-tree v2). +/// 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: &[u8], attr_info: &AttributeInfoMessage, @@ -526,6 +543,7 @@ fn extract_dense_attributes( offset_size: u8, length_size: u8, attrs: &mut Vec, + orders: &mut Vec, on_error: &mut dyn FnMut(FormatError) -> Result<(), FormatError>, ) -> Result<(), FormatError> { // Parse fractal heap @@ -561,7 +579,14 @@ fn extract_dense_attributes( AttributeMessage::parse_in_file(&attr_data, file_data, offset_size, length_size) }); match attr { - Ok(attr) => attrs.push(attr), + Ok(attr) => { + attrs.push(attr); + let order = record + .data + .get(id_len + 1..id_len + 5) + .map_or(0, |b| u32::from_le_bytes([b[0], b[1], b[2], b[3]])); + orders.push(order); + } Err(e) => on_error(e)?, } } diff --git a/crates/clawhdf5-format/src/file_writer.rs b/crates/clawhdf5-format/src/file_writer.rs index a9b3af5..2568582 100644 --- a/crates/clawhdf5-format/src/file_writer.rs +++ b/crates/clawhdf5-format/src/file_writer.rs @@ -75,14 +75,57 @@ const DENSE_LINK_THRESHOLD: usize = 8; // ---- OH builders ---- +/// An object's attributes as its header stores them: inline Attribute +/// messages, or (`dense`) the Attribute Info message of dense storage; with +/// `track_order`, their creation order tracked and indexed. +#[derive(Clone, Copy)] +pub(crate) struct AttrStorage<'a> { + pub(crate) attrs: &'a [AttributeMessage], + pub(crate) dense: Option<&'a DenseAttrBlob>, + pub(crate) track_order: bool, +} + +impl AttrStorage<'_> { + /// Add the attribute messages to the header being built. Tracking + /// creation order, as libhdf5 does it: the header's flags say so, an + /// Attribute Info message is written even for inline attributes (it + /// holds the next creation order), and each inline attribute's message + /// carries its creation order. + fn add_to(&self, w: &mut ObjectHeaderWriter) { + if self.track_order { + w.track_attr_order(); + } + if let Some(blob) = self.dense { + w.add_message(MessageType::AttributeInfo, blob.attr_info_message.clone()); + return; + } + if self.track_order { + w.add_message( + MessageType::AttributeInfo, + serialize_attribute_info( + u64::MAX, + u64::MAX, + Some((self.attrs.len() as u16, u64::MAX)), + ), + ); + } + for (i, attr) in self.attrs.iter().enumerate() { + w.add_message_with_order( + MessageType::Attribute, + attr.serialize(LENGTH_SIZE), + i as u16, + ); + } + } +} + #[allow(clippy::too_many_arguments)] pub(crate) fn build_chunked_dataset_oh( dt: &Datatype, ds: &Dataspace, layout_message: &[u8], pipeline_message: Option<&[u8]>, - attrs: &[AttributeMessage], - dense_blob: Option<&DenseAttrBlob>, + attrs: AttrStorage<'_>, fill_message: &[u8], refcount: u32, ) -> Result, FormatError> { @@ -94,13 +137,7 @@ pub(crate) fn build_chunked_dataset_oh( if let Some(pm) = pipeline_message { w.add_message(MessageType::FilterPipeline, pm.to_vec()); } - if let Some(blob) = dense_blob { - w.add_message(MessageType::AttributeInfo, blob.attr_info_message.clone()); - } else { - for attr in attrs { - w.add_message(MessageType::Attribute, attr.serialize(LENGTH_SIZE)); - } - } + attrs.add_to(&mut w); add_refcount(&mut w, refcount); w.serialize() } @@ -111,8 +148,7 @@ pub(crate) fn build_dataset_oh( ds: &Dataspace, data_addr: u64, data_size: u64, - attrs: &[AttributeMessage], - dense_blob: Option<&DenseAttrBlob>, + attrs: AttrStorage<'_>, fill_message: &[u8], refcount: u32, ) -> Result, FormatError> { @@ -132,13 +168,7 @@ pub(crate) fn build_dataset_oh( dl.extend_from_slice(&data_addr.to_le_bytes()); dl.extend_from_slice(&data_size.to_le_bytes()); w.add_message(MessageType::DataLayout, dl); - if let Some(blob) = dense_blob { - w.add_message(MessageType::AttributeInfo, blob.attr_info_message.clone()); - } else { - for attr in attrs { - w.add_message(MessageType::Attribute, attr.serialize(LENGTH_SIZE)); - } - } + attrs.add_to(&mut w); add_refcount(&mut w, refcount); w.serialize() } @@ -148,8 +178,7 @@ pub(crate) fn build_compact_dataset_oh( dt: &Datatype, ds: &Dataspace, data: &[u8], - attrs: &[AttributeMessage], - dense_blob: Option<&DenseAttrBlob>, + attrs: AttrStorage<'_>, fill_message: &[u8], refcount: u32, ) -> Result, FormatError> { @@ -164,13 +193,7 @@ pub(crate) fn build_compact_dataset_oh( dl.extend_from_slice(&(data.len() as u16).to_le_bytes()); dl.extend_from_slice(data); w.add_message(MessageType::DataLayout, dl); - if let Some(blob) = dense_blob { - w.add_message(MessageType::AttributeInfo, blob.attr_info_message.clone()); - } else { - for attr in attrs { - w.add_message(MessageType::Attribute, attr.serialize(LENGTH_SIZE)); - } - } + attrs.add_to(&mut w); add_refcount(&mut w, refcount); w.serialize() } @@ -182,8 +205,7 @@ pub(crate) fn build_group_oh( links: &[LinkMessage], link_info: &[u8], dense_links: bool, - attrs: &[AttributeMessage], - dense_blob: Option<&DenseAttrBlob>, + attrs: AttrStorage<'_>, refcount: u32, ) -> Result, FormatError> { let mut w = ObjectHeaderWriter::new(); @@ -198,13 +220,7 @@ pub(crate) fn build_group_oh( w.add_message(MessageType::Link, link.serialize(OFFSET_SIZE)); } } - if let Some(blob) = dense_blob { - w.add_message(MessageType::AttributeInfo, blob.attr_info_message.clone()); - } else { - for attr in attrs { - w.add_message(MessageType::Attribute, attr.serialize(LENGTH_SIZE)); - } - } + attrs.add_to(&mut w); add_refcount(&mut w, refcount); w.serialize() } @@ -891,11 +907,31 @@ fn write_frhp(p: WriteFrhp) -> Vec { frhp } +/// libhdf5 numbers the attributes of an object that tracks their creation +/// order with a 2-byte counter. +fn check_tracked_attr_count(track_order: bool, n: usize) -> Result<(), FormatError> { + if track_order && n > usize::from(u16::MAX) { + return Err(FormatError::SerializationError(format!( + "{n} attributes on one object with creation order tracked: libhdf5 \ + numbers at most {} (set fewer, or turn off track_order)", + u16::MAX + ))); + } + Ok(()) +} + /// Build dense attribute storage for a set of attributes. +/// +/// With `track_order` the Attribute Info message tracks creation order (an +/// attribute's creation order is its position in `attrs`) and a type-9 +/// creation-order index follows the name index, as libhdf5 writes for h5py's +/// `track_order=True`. libhdf5 numbers at most 65 535 attributes. pub(crate) fn build_dense_attrs( attrs: &[AttributeMessage], base_address: u64, + track_order: bool, ) -> Result { + check_tracked_attr_count(track_order, attrs.len())?; // Dense attrs use v3 attribute messages (adds character set encoding byte). let serialized: Vec> = attrs.iter().map(|a| a.serialize_v3(LENGTH_SIZE)).collect(); @@ -936,7 +972,30 @@ pub(crate) fn build_dense_attrs( let mut blob = heap.blob; blob.extend_from_slice(&dense_v2_btree(8, record_size, &records, bthd_addr)?); - let attr_info = serialize_attribute_info(frhp_addr, bthd_addr); + let order = if track_order { + // Type 9 records: heap ID, message flags, creation order (the key). + let records: Vec> = heap_ids + .iter() + .enumerate() + .map(|(i, heap_id)| { + let mut rec = heap_id.clone(); + rec.push(0); // msg_flags + rec.extend_from_slice(&(i as u32).to_le_bytes()); + rec + }) + .collect(); + let corder_addr = base_address + blob.len() as u64; + blob.extend_from_slice(&dense_v2_btree( + 9, + heap_id_length + 1 + 4, + &records, + corder_addr, + )?); + Some((attrs.len() as u16, corder_addr)) + } else { + None + }; + let attr_info = serialize_attribute_info(frhp_addr, bthd_addr, order); Ok(DenseAttrBlob { attr_info_message: attr_info, @@ -1139,12 +1198,25 @@ fn encode_managed_id(offset: u64, length: u64, max_heap_size: u16, id_length: u1 id } -fn serialize_attribute_info(fh_addr: u64, btree_name_addr: u64) -> Vec { +/// Serialize an Attribute Info message (version 0). `order` — the next +/// creation order to assign and the creation-order index's address — is +/// present when creation order is tracked and indexed. +fn serialize_attribute_info( + fh_addr: u64, + btree_name_addr: u64, + order: Option<(u16, u64)>, +) -> Vec { let mut data = Vec::new(); data.push(0); // version - data.push(0x00); // flags + data.push(if order.is_some() { 0x03 } else { 0x00 }); // flags: tracked, indexed + if let Some((next, _)) = order { + data.extend_from_slice(&next.to_le_bytes()); + } data.extend_from_slice(&fh_addr.to_le_bytes()); data.extend_from_slice(&btree_name_addr.to_le_bytes()); + if let Some((_, corder_addr)) = order { + data.extend_from_slice(&corder_addr.to_le_bytes()); + } data } @@ -1219,8 +1291,7 @@ pub(crate) fn build_vds_dataset_oh( dt: &Datatype, ds: &Dataspace, global_heap_addr: u64, - attrs: &[AttributeMessage], - dense_blob: Option<&DenseAttrBlob>, + attrs: AttrStorage<'_>, fill_message: &[u8], refcount: u32, ) -> Result, FormatError> { @@ -1235,13 +1306,7 @@ pub(crate) fn build_vds_dataset_oh( dl.extend_from_slice(&global_heap_addr.to_le_bytes()); dl.extend_from_slice(&1u32.to_le_bytes()); // object index 1 in the collection w.add_message(MessageType::DataLayout, dl); - if let Some(blob) = dense_blob { - w.add_message(MessageType::AttributeInfo, blob.attr_info_message.clone()); - } else { - for attr in attrs { - w.add_message(MessageType::Attribute, attr.serialize(LENGTH_SIZE)); - } - } + attrs.add_to(&mut w); add_refcount(&mut w, refcount); w.serialize() } @@ -1276,7 +1341,8 @@ fn write_undef_offset(buf: &mut Vec, offset_size: u8) { pub struct FileWriter { /// The root group's contents (its name is unused). root: GroupBuilder, - /// Default for groups that do not call [`GroupBuilder::track_order`]. + /// Default for groups and datasets that do not set their own + /// `track_order`. track_order: bool, /// Global alignment threshold: datasets with raw data >= this many bytes /// will have their data aligned to `alignment_bytes`. @@ -1311,11 +1377,18 @@ struct DsFlat { virtual_sources: Option>, /// Number of hard links to the dataset. refcount: u32, + /// Track (and index) attribute creation order. + track_order: bool, } /// Convert a DatasetBuilder into a DsFlat, handling VDS (which does not /// require a `data` field). -fn flatten_ds(db: DatasetBuilder, refcount: u32) -> Result { +fn flatten_ds( + db: DatasetBuilder, + refcount: u32, + default_track_order: bool, +) -> Result { + let track_order = db.track_order.unwrap_or(default_track_order); let dt = db.datatype.ok_or(FormatError::DatasetMissingData)?; let shape = db.shape.ok_or(FormatError::DatasetMissingShape)?; let is_vds = db.virtual_sources.is_some(); @@ -1365,6 +1438,7 @@ fn flatten_ds(db: DatasetBuilder, refcount: u32) -> Result alignment: db.alignment, virtual_sources: db.virtual_sources, refcount, + track_order, }) } @@ -1421,10 +1495,12 @@ impl FileWriter { self } - /// Track (and index) link creation order in every group that does not - /// set its own [`GroupBuilder::track_order`], the root included — as - /// h5py's `track_order=True`: libhdf5 then lists members in the order - /// they were added. Off by default (members are listed by name). + /// Track (and index) creation order — of links and attributes in every + /// group that does not set its own [`GroupBuilder::track_order`], the + /// root included, and of attributes on every dataset that does not set + /// its own [`DatasetBuilder::track_order`] — as h5py's + /// `track_order=True`: libhdf5 then lists members and attributes in the + /// order they were added. Off by default (they are listed by name). pub fn track_order(&mut self, track: bool) -> &mut Self { self.track_order = track; self @@ -1495,7 +1571,7 @@ impl FileWriter { let all_ds: Vec = tree .datasets .into_iter() - .map(|(db, refcount)| flatten_ds(db, refcount)) + .map(|(db, refcount)| flatten_ds(db, refcount, self.track_order)) .collect::>()?; let groups: Vec = tree .groups @@ -1512,6 +1588,15 @@ impl FileWriter { }) .collect(); + // Refuse up front what dense storage would refuse after the work. + let tracked = groups + .iter() + .map(|g| (g.track_order, g.attrs.len())) + .chain(all_ds.iter().map(|d| (d.track_order, d.attrs.len()))); + for (track, n) in tracked { + check_tracked_attr_count(track, n)?; + } + // Every datatype must have an on-disk encoding before anything is laid // out: `Datatype::serialize` itself cannot report a failure. let group_attrs = groups.iter().flat_map(|g| &g.attrs); @@ -1566,7 +1651,7 @@ impl FileWriter { .map(|(gi, g)| { let dummy_links = g.link_messages(&[], &[]); let attr_blob = group_dense[gi] - .then(|| build_dense_attrs(&g.attrs, 0)) + .then(|| build_dense_attrs(&g.attrs, 0, g.track_order)) .transpose()?; let li = if group_links_dense[gi] { serialize_link_info( @@ -1582,8 +1667,11 @@ impl FileWriter { &dummy_links, &li, group_links_dense[gi], - &g.attrs, - attr_blob.as_ref(), + AttrStorage { + attrs: &g.attrs, + dense: attr_blob.as_ref(), + track_order: g.track_order, + }, g.refcount, ) .map(|oh| oh.len()) @@ -1602,7 +1690,7 @@ impl FileWriter { let mut dummy_cursor = 0u64; for (i, d) in all_ds.iter().enumerate() { let dense_blob = ds_dense[i] - .then(|| build_dense_attrs(&d.attrs, 0)) + .then(|| build_dense_attrs(&d.attrs, 0, d.track_order)) .transpose()?; if is_vds[i] { // VDS: dummy OH with address 0 to get the OH size. The global @@ -1611,8 +1699,11 @@ impl FileWriter { &d.dt, &d.ds, 0, // dummy address - &d.attrs, - dense_blob.as_ref(), + AttrStorage { + attrs: &d.attrs, + dense: dense_blob.as_ref(), + track_order: d.track_order, + }, &d.fill_message, d.refcount, )?; @@ -1651,8 +1742,11 @@ impl FileWriter { &d.ds, &result.layout_message, result.pipeline_message.as_deref(), - &d.attrs, - dense_blob.as_ref(), + AttrStorage { + attrs: &d.attrs, + dense: dense_blob.as_ref(), + track_order: d.track_order, + }, &d.fill_message, d.refcount, )?; @@ -1666,8 +1760,11 @@ impl FileWriter { &d.dt, &d.ds, &d.raw, - &d.attrs, - dense_blob.as_ref(), + AttrStorage { + attrs: &d.attrs, + dense: dense_blob.as_ref(), + track_order: d.track_order, + }, &d.fill_message, d.refcount, )?; @@ -1682,8 +1779,11 @@ impl FileWriter { &d.ds, 0, d.raw.len() as u64, - &d.attrs, - dense_blob.as_ref(), + AttrStorage { + attrs: &d.attrs, + dense: dense_blob.as_ref(), + track_order: d.track_order, + }, &d.fill_message, d.refcount, )?; @@ -1727,7 +1827,7 @@ impl FileWriter { group_link_blob_addrs.push(None); } if group_dense[gi] { - let blob = build_dense_attrs(&g.attrs, cursor2 as u64)?; + let blob = build_dense_attrs(&g.attrs, cursor2 as u64, g.track_order)?; cursor2 += blob.blob.len(); group_dense_blobs.push(Some(blob)); } else { @@ -1744,7 +1844,8 @@ impl FileWriter { let addr = cursor2 as u64; cursor2 += sz; if ds_dense[i] { - let blob = build_dense_attrs(&all_ds[i].attrs, cursor2 as u64)?; + let blob = + build_dense_attrs(&all_ds[i].attrs, cursor2 as u64, all_ds[i].track_order)?; cursor2 += blob.blob.len(); ds_dense_blobs.push(Some(blob)); } else { @@ -1768,8 +1869,11 @@ impl FileWriter { &d.dt, &d.ds, heap_addr, - &d.attrs, - ds_dense_blobs[i].as_ref(), + AttrStorage { + attrs: &d.attrs, + dense: ds_dense_blobs[i].as_ref(), + track_order: d.track_order, + }, &d.fill_message, d.refcount, )?; @@ -1796,8 +1900,11 @@ impl FileWriter { &d.ds, &result.layout_message, result.pipeline_message.as_deref(), - &d.attrs, - ds_dense_blobs[i].as_ref(), + AttrStorage { + attrs: &d.attrs, + dense: ds_dense_blobs[i].as_ref(), + track_order: d.track_order, + }, &d.fill_message, d.refcount, )?; @@ -1812,8 +1919,11 @@ impl FileWriter { &d.dt, &d.ds, &d.raw, - &d.attrs, - ds_dense_blobs[i].as_ref(), + AttrStorage { + attrs: &d.attrs, + dense: ds_dense_blobs[i].as_ref(), + track_order: d.track_order, + }, &d.fill_message, d.refcount, )?; @@ -1838,8 +1948,11 @@ impl FileWriter { &d.ds, cursor2 as u64, d.raw.len() as u64, - &d.attrs, - ds_dense_blobs[i].as_ref(), + AttrStorage { + attrs: &d.attrs, + dense: ds_dense_blobs[i].as_ref(), + track_order: d.track_order, + }, &d.fill_message, d.refcount, )?; @@ -1907,8 +2020,11 @@ impl FileWriter { &links, &li, link_blob.is_some(), - &g.attrs, - group_dense_blobs[gi].as_ref(), + AttrStorage { + attrs: &g.attrs, + dense: group_dense_blobs[gi].as_ref(), + track_order: g.track_order, + }, g.refcount, )?; debug_assert_eq!(oh.len(), group_oh_sizes[gi]); @@ -2139,6 +2255,92 @@ mod tests { assert_eq!(read_dataset_f64(&bytes, "data"), vec![1.0, 2.0, 3.0]); } + /// Attribute names of the object at `path`, in the order the reader + /// lists them. + fn attr_names(bytes: &[u8], path: &str) -> Vec { + let sig = signature::find_signature(bytes).unwrap(); + let sb = Superblock::parse(bytes, sig).unwrap(); + let addr = if path == "/" { + sb.root_group_address + } else { + resolve_path_any(bytes, &sb, path).unwrap() + }; + let hdr = + ObjectHeader::parse(bytes, addr as usize, sb.offset_size, sb.length_size).unwrap(); + crate::attribute::extract_attributes_full(bytes, &hdr, sb.offset_size, sb.length_size) + .unwrap() + .into_iter() + .map(|a| a.name) + .collect() + } + + #[test] + fn tracked_attributes_are_read_in_creation_order() { + let set = |names: &[String]| -> Vec<(String, AttrValue)> { + names + .iter() + .enumerate() + .map(|(i, n)| (n.clone(), AttrValue::I64(i as i64))) + .collect() + }; + let compact: Vec = ["zeta", "alpha", "mid"].map(String::from).to_vec(); + let dense: Vec = (0..30).rev().map(|i| format!("a{i:02}")).collect(); + let mut fw = FileWriter::new(); + fw.track_order(true); + for (n, v) in set(&compact) { + fw.set_root_attr(&n, v); + } + let ds = fw.create_dataset("dense"); + ds.with_i32_data(&[1]); + for (n, v) in set(&dense) { + ds.set_attr(&n, v); + } + let ds = fw.create_dataset("untracked"); + ds.with_i32_data(&[1]).track_order(false); + for (n, v) in set(&dense) { + ds.set_attr(&n, v); + } + let mut g = fw.create_group("g"); + g.track_order(false); + for (n, v) in set(&compact) { + g.set_attr(&n, v); + } + fw.add_group(g.finish()); + let bytes = fw.finish().unwrap(); + assert_eq!(attr_names(&bytes, "/"), compact); + assert_eq!(attr_names(&bytes, "dense"), dense); + // Without tracking: storage order (inline: as added; dense: hash). + assert_eq!(attr_names(&bytes, "g"), compact); + let mut by_hash = dense.clone(); + by_hash.sort_by_key(|n| crate::checksum::jenkins_lookup3(n.as_bytes())); + assert_eq!(attr_names(&bytes, "untracked"), by_hash); + } + + #[test] + fn too_many_tracked_attributes_is_an_error() { + // libhdf5 numbers at most 65 535 attributes on an object that + // tracks their creation order (a 2-byte field). (`set_attr` looks + // for an earlier value, so 65 536 of them through the builder take + // a while; build the messages directly.) + let attrs: Vec = (0..65_536) + .map(|i| build_attr_message(&format!("a{i}"), &AttrValue::I64(i))) + .collect(); + let err = build_dense_attrs(&attrs, 0, true) + .err() + .unwrap() + .to_string(); + assert!(err.contains("65536 attributes on one object"), "{err}"); + assert!(build_dense_attrs(&attrs[1..], 0, true).is_ok()); + assert!(build_dense_attrs(&attrs, 0, false).is_ok()); + let mut fw = FileWriter::new(); + let ds = fw.create_dataset("x"); + ds.with_i32_data(&[1]).track_order(true); + for i in 0..20 { + ds.set_attr(&format!("a{i}"), AttrValue::I64(i)); + } + assert!(fw.finish().is_ok()); + } + #[test] fn dense_attrs_root_group_self_roundtrip() { let mut fw = FileWriter::new(); diff --git a/crates/clawhdf5-format/src/object_header_writer.rs b/crates/clawhdf5-format/src/object_header_writer.rs index 8d52ad4..bbb3c0a 100644 --- a/crates/clawhdf5-format/src/object_header_writer.rs +++ b/crates/clawhdf5-format/src/object_header_writer.rs @@ -12,9 +12,16 @@ use crate::message_type::MessageType; /// its size truncated to 16 bits produced files libhdf5 refuses. pub const MAX_MESSAGE_SIZE: usize = u16::MAX as usize; +/// Object header flags: attribute creation order tracked (each message +/// then carries a 2-byte creation order) and indexed. +const OHDR_ATTR_CRT_ORDER_TRACKED: u8 = 0x04; +const OHDR_ATTR_CRT_ORDER_INDEXED: u8 = 0x08; + /// Writer for v2 object headers with proper checksums. pub struct ObjectHeaderWriter { - messages: Vec<(MessageType, Vec, u8)>, // (type, data, msg_flags) + messages: Vec<(MessageType, Vec, u8, u16)>, // (type, data, msg_flags, creation order) + /// Attribute creation order tracked and indexed. + attr_order: bool, } impl ObjectHeaderWriter { @@ -22,17 +29,33 @@ impl ObjectHeaderWriter { pub fn new() -> Self { Self { messages: Vec::new(), + attr_order: false, } } + /// Track and index attribute creation order, as libhdf5 does for an + /// object created with `H5P_CRT_ORDER_TRACKED | H5P_CRT_ORDER_INDEXED` + /// (h5py's `track_order=True`): the header's flags say so, and every + /// message carries a creation order (an attribute's own; 0 for the + /// others). libhdf5 reads the setting back from these flags. + pub fn track_attr_order(&mut self) { + self.attr_order = true; + } + /// Add a message to the header with default flags (0). pub fn add_message(&mut self, msg_type: MessageType, data: Vec) { - self.messages.push((msg_type, data, 0)); + self.messages.push((msg_type, data, 0, 0)); } /// Add a message with specific flags. pub fn add_message_with_flags(&mut self, msg_type: MessageType, data: Vec, flags: u8) { - self.messages.push((msg_type, data, flags)); + self.messages.push((msg_type, data, flags, 0)); + } + + /// Add a message with its creation order, which is written only when + /// attribute creation order is tracked ([`Self::track_attr_order`]). + pub fn add_message_with_order(&mut self, msg_type: MessageType, data: Vec, order: u16) { + self.messages.push((msg_type, data, 0, order)); } /// Serialize the complete v2 object header (OHDR + messages + checksum). @@ -41,10 +64,10 @@ impl ObjectHeaderWriter { /// than [`MAX_MESSAGE_SIZE`] (e.g. an attribute over ~64 KiB, which would /// need dense attribute storage), rather than writing a corrupt header. pub fn serialize(&self) -> Result, FormatError> { - if let Some((msg_type, data, _)) = self + if let Some((msg_type, data, _, _)) = self .messages .iter() - .find(|(_, data, _)| data.len() > MAX_MESSAGE_SIZE) + .find(|(_, data, _, _)| data.len() > MAX_MESSAGE_SIZE) { return Err(FormatError::SerializationError(format!( "{msg_type:?} message is {} bytes; an object header message holds at most \ @@ -52,11 +75,13 @@ impl ObjectHeaderWriter { data.len() ))); } - // Calculate total message bytes: each message has type(1) + size(2) + flags(1) + data + // Calculate total message bytes: each message has type(1) + size(2) + + // flags(1) [+ creation order(2)] + data + let msg_header = if self.attr_order { 6 } else { 4 }; let msg_bytes_total: usize = self .messages .iter() - .map(|(_, data, _)| 4 + data.len()) + .map(|(_, data, _, _)| msg_header + data.len()) .sum(); // Determine chunk size field width based on msg_bytes_total @@ -68,6 +93,12 @@ impl ObjectHeaderWriter { (0x02u8, 4) }; + let flags = if self.attr_order { + flags | OHDR_ATTR_CRT_ORDER_TRACKED | OHDR_ATTR_CRT_ORDER_INDEXED + } else { + flags + }; + let mut buf = Vec::new(); // OHDR signature @@ -85,7 +116,7 @@ impl ObjectHeaderWriter { } // Messages - for (msg_type, data, msg_flags) in &self.messages { + for (msg_type, data, msg_flags, order) in &self.messages { let type_id = msg_type.to_u16(); assert!( type_id <= 255, @@ -94,6 +125,9 @@ impl ObjectHeaderWriter { buf.push(type_id as u8); // type (1 byte in v2) buf.extend_from_slice(&(data.len() as u16).to_le_bytes()); // size (2 bytes) buf.push(*msg_flags); // flags + if self.attr_order { + buf.extend_from_slice(&order.to_le_bytes()); // creation order + } buf.extend_from_slice(data); } @@ -193,6 +227,21 @@ mod tests { assert_eq!(hdr.messages.len(), 0); } + #[test] + fn tracked_attribute_order_is_in_the_flags_and_every_message() { + let mut writer = ObjectHeaderWriter::new(); + writer.track_attr_order(); + writer.add_message(MessageType::Dataspace, vec![1, 2, 3, 4]); + writer.add_message_with_order(MessageType::Attribute, vec![5, 6], 7); + let bytes = writer.serialize().unwrap(); + assert_eq!(bytes[5] & 0x0C, 0x0C); + let hdr = ObjectHeader::parse(&bytes, 0, 8, 8).unwrap(); + assert_eq!(hdr.messages.len(), 2); + assert_eq!(hdr.messages[0].creation_order, Some(0)); + assert_eq!(hdr.messages[1].creation_order, Some(7)); + assert_eq!(hdr.messages[1].data, vec![5, 6]); + } + #[test] fn two_messages_roundtrip() { let mut writer = ObjectHeaderWriter::new(); diff --git a/crates/clawhdf5-format/src/type_builders.rs b/crates/clawhdf5-format/src/type_builders.rs index ce43f64..3019daa 100644 --- a/crates/clawhdf5-format/src/type_builders.rs +++ b/crates/clawhdf5-format/src/type_builders.rs @@ -503,6 +503,9 @@ pub struct DatasetBuilder { /// `data` field is ignored; instead the global heap blob is built from /// these mappings and a VDS layout message is emitted. pub(crate) virtual_sources: Option>, + /// Track (and index) attribute creation order; `None` follows the + /// file's default (`FileWriter::track_order`). + pub(crate) track_order: Option, #[cfg(feature = "provenance")] pub(crate) provenance: Option, } @@ -522,11 +525,22 @@ impl DatasetBuilder { compact: false, alignment: 0, virtual_sources: None, + track_order: None, #[cfg(feature = "provenance")] provenance: None, } } + /// Track the creation order of this dataset's attributes, and index it, + /// as h5py's `create_dataset(..., track_order=True)` does: libhdf5 (and + /// h5py) then list the attributes in the order they were set rather + /// than by name. libhdf5 numbers at most 65 535 attributes on an object + /// that tracks their order; more is an error when the file is written. + pub fn track_order(&mut self, track: bool) -> &mut Self { + self.track_order = Some(track); + self + } + pub fn with_f64_data(&mut self, data: &[f64]) -> &mut Self { self.datatype = Some(make_f64_type()); let mut b = Vec::with_capacity(data.len() * 8); @@ -986,10 +1000,11 @@ impl GroupBuilder { self.attrs.push((name.to_string(), value)); } - /// Track the creation order of this group's links, and index it, as - /// h5py's `track_order=True` does: libhdf5 (and h5py) then list the - /// group's members in the order they were added rather than by name. - /// Applies to links only, not to attributes. + /// Track the creation order of this group's links and attributes, and + /// index it, as h5py's `track_order=True` does: libhdf5 (and h5py) then + /// list the group's members, and its attributes, in the order they were + /// added rather than by name. libhdf5 numbers at most 65 535 attributes + /// on an object that tracks their order. pub fn track_order(&mut self, track: bool) -> &mut Self { self.track_order = Some(track); self diff --git a/crates/clawhdf5-tools/tests/h5rs_interop.rs b/crates/clawhdf5-tools/tests/h5rs_interop.rs index 32f6fb2..f7b0f74 100644 --- a/crates/clawhdf5-tools/tests/h5rs_interop.rs +++ b/crates/clawhdf5-tools/tests/h5rs_interop.rs @@ -940,11 +940,17 @@ fn write_nested_links(dir: &Path) -> Vec { g.create_dataset(&format!("n{i:02}")).with_i32_data(&[i]); } g.add_hard_link("back", "/a/b/c"); + // Attribute creation order tracked too: dense, with a type-9 index. + for i in (0..12).rev() { + g.set_attr(&format!("attr{i:02}"), AttrValue::I64(i)); + } b.add_group(g.finish()); let mut g = b.create_group("compact_ordered"); g.track_order(true); g.create_dataset("z").with_i32_data(&[1]); g.create_dataset("a").with_i32_data(&[2]); + g.set_attr("zz", AttrValue::I64(1)); + g.set_attr("aa", AttrValue::I64(2)); b.add_group(g.finish()); let nested = dir.join("nested.h5"); b.write(&nested).unwrap(); diff --git a/crates/clawhdf5/src/writer.rs b/crates/clawhdf5/src/writer.rs index bfa9393..ea48bc9 100644 --- a/crates/clawhdf5/src/writer.rs +++ b/crates/clawhdf5/src/writer.rs @@ -88,9 +88,11 @@ impl FileBuilder { self } - /// Track link creation order in every group that does not set its own - /// (`GroupBuilder::track_order`), as h5py's `track_order=True`: libhdf5 - /// then lists members in the order they were added. + /// Track the creation order of links and attributes in every group, and + /// of attributes on every dataset, that does not set its own + /// (`GroupBuilder::track_order`, `DatasetBuilder::track_order`), as + /// h5py's `track_order=True`: libhdf5 then lists members and attributes + /// in the order they were added. pub fn track_order(&mut self, track: bool) -> &mut Self { self.writer.track_order(track); self diff --git a/crates/clawhdf5/tests/writer_groups_interop.rs b/crates/clawhdf5/tests/writer_groups_interop.rs index d16567c..777ec07 100644 --- a/crates/clawhdf5/tests/writer_groups_interop.rs +++ b/crates/clawhdf5/tests/writer_groups_interop.rs @@ -531,6 +531,97 @@ fn ten_thousand_links_in_one_group() { ); } +#[test] +fn track_order_lists_attributes_in_creation_order() { + skip_if_no_python!(); + // h5py's track_order=True orders an object's attributes as well as a + // group's links; the writer tracked links only, so h5py listed the + // attributes by name. Now the object header's flags say attribute + // creation order is tracked and indexed, an Attribute Info message + // holds the next order, inline attributes carry theirs, and dense + // storage gets a creation-order index (B-tree type 9). + let dir = tempfile::tempdir().unwrap(); + let small = ["zeta", "alpha", "mid"]; + let mut b = FileBuilder::new(); + b.track_order(true); // the root, and every group and dataset by default + for (i, n) in small.iter().enumerate() { + b.set_attr(n, AttrValue::I64(i as i64)); + } + let mut g = b.create_group("g"); // dense: 30 attributes + for i in (0..30).rev() { + g.set_attr(&format!("a{i:02}"), AttrValue::I64(i)); + } + b.add_group(g.finish()); + let d = b.create_dataset("d"); + d.with_i32_data(&[1]); + for (i, n) in small.iter().enumerate() { + d.set_attr(n, AttrValue::I64(i as i64)); + } + // 20 000 attributes: a one-leaf creation-order index of 20 000 records. + let big = b.create_dataset("big"); + big.with_i32_data(&[2]); + for i in (0..20_000).rev() { + big.set_attr(&format!("b{i:05}"), AttrValue::I64(i)); + } + let plain = b.create_dataset("plain"); + plain.with_i32_data(&[3]).track_order(false); + for (i, n) in small.iter().enumerate() { + plain.set_attr(n, AttrValue::I64(i as i64)); + } + let path = write(&dir, "attr_order.h5", b); + + let out = h5py( + &path, + "def order(o):\n\ + \x20 return o.id.get_create_plist().get_attr_creation_order()\n\ + with h5py.File(path, 'r') as f:\n\ + \x20 big = list(f['big'].attrs)\n\ + \x20 print(json.dumps([list(f.attrs), [int(v) for v in f.attrs.values()],\n\ + \x20 list(f['g'].attrs)[:3], len(f['g'].attrs), list(f['d'].attrs),\n\ + \x20 big[:2], big == ['b%05d' % i for i in range(19999, -1, -1)],\n\ + \x20 int(f['big'].attrs['b00007']), list(f['plain'].attrs),\n\ + \x20 [order(f['/']), order(f['g']), order(f['d']), order(f['plain'])]]))", + ); + assert_eq!( + out, + r#"[["zeta", "alpha", "mid"], [0, 1, 2], ["a29", "a28", "a27"], 30, ["zeta", "alpha", "mid"], ["b19999", "b19998"], true, 7, ["alpha", "mid", "zeta"], [3, 3, 3, 0]]"# + ); + h5dump_ok(&path); + let f = File::open(&path).unwrap(); + assert_eq!(f.dataset("big").unwrap().attrs().unwrap().len(), 20_000); + assert!(matches!( + f.group("g").unwrap().attrs().unwrap()["a07"], + AttrValue::I64(7) + )); + drop(f); + + // libhdf5 continues the numbering: new attributes come last, also when + // it moves the inline ones of `d` to dense storage. + let out = h5py( + &path, + "with h5py.File(path, 'r+') as f:\n\ + \x20 f.attrs['new'] = 9\n\ + \x20 del f.attrs['alpha']\n\ + \x20 f['g'].attrs['new'] = 9\n\ + \x20 del f['g'].attrs['a15']\n\ + \x20 for i in range(8):\n\ + \x20 f['d'].attrs['x%d' % i] = i\n\ + \x20 f['big'].attrs['new'] = 9\n\ + \x20 for i in range(0, 20000, 2):\n\ + \x20 del f['big'].attrs['b%05d' % i]\n\ + with h5py.File(path, 'r') as f:\n\ + \x20 big = list(f['big'].attrs)\n\ + \x20 print(json.dumps([list(f.attrs), list(f['g'].attrs)[-2:], len(f['g'].attrs),\n\ + \x20 list(f['d'].attrs)[:4], len(f['d'].attrs),\n\ + \x20 big == ['b%05d' % i for i in range(19999, -1, -2)] + ['new']]))", + ); + assert_eq!( + out, + r#"[["zeta", "mid", "new"], ["a00", "new"], 30, ["zeta", "alpha", "mid", "x0"], 11, true]"# + ); + h5dump_ok(&path); +} + #[test] fn track_order_lists_members_in_creation_order() { skip_if_no_python!(); diff --git a/docs/known-issues.md b/docs/known-issues.md index 865102b..fca0df6 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -264,8 +264,16 @@ fill-value item that did is fixed). external links at any depth and optional creation-order tracking; h5py, h5dump and `h5rs check --data` read them (`crates/clawhdf5/tests/writer_groups_interop.rs`, - `crates/clawhdf5-tools/tests/h5rs_interop.rs`). Still missing: - attribute creation order is not tracked. + `crates/clawhdf5-tools/tests/h5rs_interop.rs`). ~~Still missing: + attribute creation order is not tracked.~~ **Fixed 2026-09-26:** + `track_order` (file default, `GroupBuilder`, and the new + `DatasetBuilder::track_order`) tracks and indexes attribute creation + order as h5py's `track_order=True` does; h5py lists the attributes in + the order they were set, inline and dense (20 000 on one dataset), and + keeps numbering them in "r+" mode (tank, + `cargo test -p clawhdf5 --test writer_groups_interop + track_order_lists_attributes_in_creation_order`). libhdf5 numbers at + most 65 535 attributes on such an object, so more is an error. - ~~A group with more than 65 535 links, or an object with more than 65 535 dense attributes, is an error (the index is one B-tree leaf).~~ **Fixed 2026-09-26:** the dense indexes are v2 B-trees of any depth