From fb88cc9cd7d2887fe80865d5895e4867a1bcea32 Mon Sep 17 00:00:00 2001 From: Sang Woo Kim Date: Wed, 12 Aug 2026 23:49:20 +0900 Subject: [PATCH 01/20] Generalize the hyperslab walker to two differently-shaped arrays MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A chunked hyperslab write copies each chunk's share of the selection between two row-major arrays with independent shapes: the caller's counts-shaped buffer and one chunk_dims-shaped chunk buffer. The existing walker only handled the case where the second array *is* the selection, so a chunk scatter would have had to re-derive stride arithmetic — the exact duplication that produced the "one transfer per last-axis row" defect the walker was introduced to close. for_each_dual_run takes both shapes and both origins, and coalesces a run only across dimensions that are full on both sides. for_each_contiguous_run becomes the degenerate case (second array = the selection at its origin), so there is still one owner of the geometry. --- src/io/hyperslab.rs | 191 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 169 insertions(+), 22 deletions(-) diff --git a/src/io/hyperslab.rs b/src/io/hyperslab.rs index 9f4a29b..2b07750 100644 --- a/src/io/hyperslab.rs +++ b/src/io/hyperslab.rs @@ -1,11 +1,13 @@ //! Hyperslab (strideless) selection geometry shared by the reader and writer. //! -//! Both directions of an N-dimensional hyperslab transfer — reading a slice -//! out of a contiguous/chunked dataset and writing a slice into a contiguous -//! dataset — decompose the selection into the same set of maximal contiguous -//! byte-runs. [`for_each_contiguous_run`] is the single owner of that -//! geometry, so the "one transfer per last-axis row" defect cannot reappear in -//! one path while another is fixed. +//! Every N-dimensional hyperslab transfer — reading a slice out of a +//! contiguous/chunked dataset, writing a slice into a contiguous dataset, and +//! scattering a slice across a chunked dataset's chunk buffers — decomposes +//! the selection into the same set of maximal contiguous byte-runs. +//! [`for_each_dual_run`] is the single owner of that geometry, so the "one +//! transfer per last-axis row" defect cannot reappear in one path while +//! another is fixed. [`for_each_contiguous_run`] is the special case where the +//! second array *is* the selection. use crate::io::IoResult; @@ -49,35 +51,80 @@ pub(crate) fn for_each_contiguous_run( element_size: u64, mut f: impl FnMut(u64, usize, usize) -> IoResult<()>, ) -> IoResult<()> { - let ndims = dims.len(); - debug_assert_eq!(starts.len(), ndims); - debug_assert_eq!(counts.len(), ndims); - if ndims == 0 { + // The selection buffer is exactly `counts`-shaped and read from its + // origin, which is what makes this the degenerate case of the dual walk. + let src_starts = vec![0u64; counts.len()]; + for_each_dual_run( + dims, + starts, + counts, + &src_starts, + counts, + element_size, + |dst_off, src_off, len| f(dst_off, src_off as usize, len), + ) +} + +/// Visit the maximal contiguous byte-runs shared by the *same* logical region +/// as it sits in two differently-shaped row-major arrays. +/// +/// The region is `counts` elements wide, located at `dst_starts` within an +/// array of shape `dst_dims` and at `src_starts` within an array of shape +/// `src_dims`. `f(dst_off, src_off, len)` is called once per run with the byte +/// offsets of that run in each array. +/// +/// A run may only coalesce across a dimension that is fully selected in +/// **both** arrays: a trailing dimension that is full on one side but partial +/// on the other is contiguous there and strided here, so the shorter run wins. +/// This is what lets one walker serve a whole-dataset transfer (where the +/// second array is the selection itself, see [`for_each_contiguous_run`]) and a +/// chunk scatter (where the second array is one chunk of the grid) without +/// either path re-deriving stride arithmetic. +pub(crate) fn for_each_dual_run( + dst_dims: &[u64], + dst_starts: &[u64], + src_dims: &[u64], + src_starts: &[u64], + counts: &[u64], + element_size: u64, + mut f: impl FnMut(u64, u64, usize) -> IoResult<()>, +) -> IoResult<()> { + let ndims = counts.len(); + debug_assert_eq!(dst_dims.len(), ndims); + debug_assert_eq!(dst_starts.len(), ndims); + debug_assert_eq!(src_dims.len(), ndims); + debug_assert_eq!(src_starts.len(), ndims); + if ndims == 0 || counts.contains(&0) { return Ok(()); } - let strides = compute_strides(dims, element_size); + let dst_strides = compute_strides(dst_dims, element_size); + let src_strides = compute_strides(src_dims, element_size); // Largest fully-selected trailing block: walk inward while the dimension - // just inside the run boundary is fully selected. `m` ends as the - // outermost dimension folded into a single run; dims `(m, ndims)` are all - // full, dim `m` may be partial and forms the run's outer stride. + // just inside the run boundary is fully selected on both sides. `m` ends as + // the outermost dimension folded into a single run; dims `(m, ndims)` are + // full in both arrays, dim `m` may be partial and forms the run's outer + // stride. let mut m = ndims - 1; - while m > 0 && counts[m] == dims[m] { + while m > 0 && counts[m] == dst_dims[m] && counts[m] == src_dims[m] { m -= 1; } let run_elems: u64 = counts[m..].iter().product(); let run_bytes = (run_elems * element_size) as usize; // Offset of the run's first element within dims `[m, ndims)` is constant - // across outer iterations (dim `m` starts at `starts[m]`, deeper dims at 0 - // since they are full). - let inner_base: u64 = (m..ndims).map(|d| starts[d] * strides[d]).sum(); + // across outer iterations (dim `m` starts at its start coordinate, deeper + // dims at 0 since a fully-selected dimension must start at 0). + let dst_base: u64 = (m..ndims).map(|d| dst_starts[d] * dst_strides[d]).sum(); + let src_base: u64 = (m..ndims).map(|d| src_starts[d] * src_strides[d]).sum(); let n_outer: u64 = counts[..m].iter().product(); // empty product == 1 let mut coords = vec![0u64; m]; - for outer in 0..n_outer { - let mut src_off = inner_base; + for _ in 0..n_outer { + let mut dst_off = dst_base; + let mut src_off = src_base; for d in 0..m { - src_off += (starts[d] + coords[d]) * strides[d]; + dst_off += (dst_starts[d] + coords[d]) * dst_strides[d]; + src_off += (src_starts[d] + coords[d]) * src_strides[d]; } - f(src_off, outer as usize * run_bytes, run_bytes)?; + f(dst_off, src_off, run_bytes)?; for d in (0..m).rev() { coords[d] += 1; if coords[d] < counts[d] { @@ -88,3 +135,103 @@ pub(crate) fn for_each_contiguous_run( } Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + + /// Collect `(dst_off, src_off, len)` for every run of a dual walk. + fn dual( + dst_dims: &[u64], + dst_starts: &[u64], + src_dims: &[u64], + src_starts: &[u64], + counts: &[u64], + es: u64, + ) -> Vec<(u64, u64, usize)> { + let mut runs = Vec::new(); + for_each_dual_run( + dst_dims, + dst_starts, + src_dims, + src_starts, + counts, + es, + |d, s, l| { + runs.push((d, s, l)); + Ok(()) + }, + ) + .unwrap(); + runs + } + + #[test] + fn dual_run_coalesces_when_trailing_dim_is_full_on_both_sides() { + // A 2x6 region occupying whole rows of both a 4x6 array and a 2x6 one: + // both sides are contiguous end to end, so it is a single run. + let runs = dual(&[4, 6], &[2, 0], &[2, 6], &[0, 0], &[2, 6], 4); + assert_eq!(runs, vec![(48, 0, 48)]); + } + + #[test] + fn dual_run_splits_when_trailing_dim_is_partial_on_the_source() { + // Trailing dim is full in the destination (6 of 6) but only part of the + // 12-wide source, so the source stride breaks the run. + let runs = dual(&[4, 6], &[1, 0], &[4, 12], &[1, 3], &[2, 6], 4); + assert_eq!(runs, vec![(24, 60, 24), (48, 108, 24)]); + } + + #[test] + fn dual_run_splits_when_trailing_dim_is_partial_on_the_destination() { + // Mirror of the previous case: full on the source side, partial on the + // destination side. The run must still break. + let runs = dual(&[4, 12], &[1, 3], &[4, 6], &[1, 0], &[2, 6], 4); + assert_eq!(runs, vec![(60, 24, 24), (108, 48, 24)]); + } + + #[test] + fn dual_run_walks_three_dimensions() { + // 3-D selection whose innermost dim is full on both sides but whose + // middle dim is not: one run per (outer, middle) pair. + let runs = dual( + &[2, 4, 3], + &[0, 1, 0], + &[2, 2, 3], + &[0, 0, 0], + &[2, 2, 3], + 2, + ); + assert_eq!(runs, vec![(6, 0, 12), (30, 12, 12)]); + } + + #[test] + fn empty_selection_visits_no_runs() { + assert!(dual(&[4, 6], &[0, 0], &[4, 6], &[0, 0], &[0, 6], 4).is_empty()); + assert!(dual(&[4, 6], &[0, 0], &[4, 6], &[0, 0], &[2, 0], 4).is_empty()); + } + + #[test] + fn contiguous_run_is_the_dual_walk_against_the_selection_itself() { + // The wrapper must agree with the general walker for every case the + // old single-array implementation covered. + for (dims, starts, counts) in [ + (vec![4u64, 6], vec![1u64, 2], vec![2u64, 3]), + (vec![4, 6], vec![2, 0], vec![2, 6]), + (vec![5, 3, 2], vec![1, 0, 0], vec![3, 3, 2]), + (vec![7], vec![2], vec![4]), + ] { + let mut got = Vec::new(); + for_each_contiguous_run(&dims, &starts, &counts, 4, |dst, src, len| { + got.push((dst, src as u64, len)); + Ok(()) + }) + .unwrap(); + let zeros = vec![0u64; counts.len()]; + assert_eq!(got, dual(&dims, &starts, &counts, &zeros, &counts, 4)); + // The selection buffer is filled exactly once, front to back. + let total: usize = got.iter().map(|r| r.2).sum(); + assert_eq!(total as u64, counts.iter().product::() * 4); + } + } +} From 6c57badf405dd745df692bf121ede40f9eb3f5ea Mon Sep 17 00:00:00 2001 From: Sang Woo Kim Date: Wed, 12 Aug 2026 23:52:00 +0900 Subject: [PATCH 02/20] Let the file allocator reclaim released blocks Rewriting a chunk whose stored size changed has to put the new bytes somewhere else, and the old block was previously abandoned: the allocator only ever bumped the end of file. libhdf5 hands that block back with H5MF_xfree (H5Dchunk.c H5D__chunk_file_alloc), so a chunk rewritten many times does not grow the file without bound. FileAllocator::free returns a block for reuse; allocate takes a best-fit block before growing, splitting the remainder at an aligned boundary. Adjacent blocks merge on free so repeated grow/shrink cycles cannot shred the list into unusable fragments. A relaxed counter lets allocate skip the lock entirely while nothing has been freed, so the streaming path keeps its lock-free bump. Like libhdf5's default (non-persistent) free-space strategy, the list is session-only: a block released but not reused before close stays as slack rather than being recorded in an on-disk free-space manager. --- src/io/allocator.rs | 217 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 212 insertions(+), 5 deletions(-) diff --git a/src/io/allocator.rs b/src/io/allocator.rs index ddbac14..8f3f67b 100644 --- a/src/io/allocator.rs +++ b/src/io/allocator.rs @@ -1,6 +1,14 @@ use std::sync::atomic::{AtomicU64, Ordering}; +use std::sync::Mutex; -/// Simple append-only file space allocator. +/// One released, reusable region of the file. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +struct FreeBlock { + addr: u64, + len: u64, +} + +/// File space allocator: bump-the-end-of-file, with reuse of released blocks. /// /// Hands out file offsets by bumping an end-of-file pointer. Every /// allocation is aligned to the configured boundary (default 8 bytes). @@ -9,10 +17,30 @@ use std::sync::atomic::{AtomicU64, Ordering}; /// and is safe to call concurrently: two threads allocating at once each get /// a distinct, non-overlapping, aligned offset. This is the lock-free /// foundation that lets the `threadsafe` writer hand out chunk space without -/// a global lock (see `docs/threadsafe-fine-grained-locking.md`). +/// a global lock (see `docs/threadsafe-fine-grained-locking.md`). A writer +/// that never calls [`free`](Self::free) never touches the free list, and +/// [`allocate`](Self::allocate) skips its lock entirely while the list is +/// empty, so the streaming path keeps that lock-free fast path. +/// +/// [`free`](Self::free) returns a block for reuse — the counterpart of +/// libhdf5's `H5MF_xfree`, called when a rewritten chunk no longer fits its +/// old location. Like libhdf5's default (non-persistent) free-space strategy, +/// the list lives only for the session: a block released but not reused +/// before `close` stays as slack in the file rather than being recorded in an +/// on-disk free-space manager. pub struct FileAllocator { eof: AtomicU64, alignment: u64, + /// Released blocks, sorted by address with adjacent blocks merged. + /// + /// A plain `Mutex` regardless of the `threadsafe` feature: the allocator + /// is shared across threads in both builds (see + /// `concurrent_allocations_are_disjoint`), and this lock is only ever + /// taken on the rare free/reuse path. + free_list: Mutex>, + /// `free_list.len()`, readable without taking the lock so the common + /// never-freed case costs one relaxed load. + free_count: AtomicU64, } impl FileAllocator { @@ -21,15 +49,27 @@ impl FileAllocator { Self { eof: AtomicU64::new(initial_eof), alignment: 8, + free_list: Mutex::new(Vec::new()), + free_count: AtomicU64::new(0), } } + /// Round `size` up to the allocator's alignment. + fn align_up(&self, size: u64) -> u64 { + (size + self.alignment - 1) & !(self.alignment - 1) + } + /// Allocate `size` bytes, returning the aligned starting offset. /// - /// Lock-free: the aligned bump is published with a compare-and-swap loop, - /// so concurrent callers never overlap (alignment makes a plain - /// `fetch_add` insufficient, hence the CAS). + /// A released block large enough to hold `size` is reused before the file + /// grows; otherwise the end-of-file pointer is bumped. The bump is + /// lock-free: it is published with a compare-and-swap loop, so concurrent + /// callers never overlap (alignment makes a plain `fetch_add` + /// insufficient, hence the CAS). pub fn allocate(&self, size: u64) -> u64 { + if let Some(addr) = self.take_free(size) { + return addr; + } let mut cur = self.eof.load(Ordering::Acquire); loop { let aligned = (cur + self.alignment - 1) & !(self.alignment - 1); @@ -44,6 +84,63 @@ impl FileAllocator { } } + /// Release `len` bytes at `addr` for reuse by later allocations. + /// + /// The caller must have already dropped every reference to the block (for + /// a chunk: the index entry must be about to point elsewhere). Adjacent + /// blocks merge, so a repeatedly grown-and-shrunk chunk does not shred the + /// list into unusable fragments. + pub fn free(&self, addr: u64, len: u64) { + if len == 0 { + return; + } + let mut list = self.free_list.lock().unwrap(); + let pos = list.partition_point(|b| b.addr < addr); + list.insert(pos, FreeBlock { addr, len }); + // Merge with the following block, then with the preceding one, so a + // block that fills the gap between two free blocks yields one block. + if pos + 1 < list.len() && list[pos].addr + list[pos].len == list[pos + 1].addr { + list[pos].len += list[pos + 1].len; + list.remove(pos + 1); + } + if pos > 0 && list[pos - 1].addr + list[pos - 1].len == list[pos].addr { + list[pos - 1].len += list[pos].len; + list.remove(pos); + } + self.free_count.store(list.len() as u64, Ordering::Release); + } + + /// Take the smallest released block that fits `size`, splitting off the + /// remainder. Returns `None` when nothing fits (or nothing was freed). + fn take_free(&self, size: u64) -> Option { + if size == 0 || self.free_count.load(Ordering::Acquire) == 0 { + return None; + } + let mut list = self.free_list.lock().unwrap(); + // Best fit: the smallest sufficient block, so a large released region + // stays available for a large chunk. + let pos = list + .iter() + .enumerate() + .filter(|(_, b)| b.len >= size) + .min_by_key(|(_, b)| b.len) + .map(|(i, _)| i)?; + let block = list[pos]; + // `block.addr` is aligned (every allocation is) and `used` is a + // multiple of the alignment, so the remainder stays aligned too. + let used = self.align_up(size); + if block.len > used { + list[pos] = FreeBlock { + addr: block.addr + used, + len: block.len - used, + }; + } else { + list.remove(pos); + } + self.free_count.store(list.len() as u64, Ordering::Release); + Some(block.addr) + } + /// Return the current end-of-file offset. pub fn eof(&self) -> u64 { self.eof.load(Ordering::Acquire) @@ -134,4 +231,114 @@ mod tests { let unique = all.iter().collect::>().len(); assert_eq!(unique, all.len(), "duplicate offsets handed out"); } + + /// Snapshot of the free list for assertions. + fn free_blocks(alloc: &FileAllocator) -> Vec<(u64, u64)> { + alloc + .free_list + .lock() + .unwrap() + .iter() + .map(|b| (b.addr, b.len)) + .collect() + } + + #[test] + fn freed_block_is_reused_before_the_file_grows() { + let alloc = FileAllocator::new(0); + let a = alloc.allocate(64); + alloc.allocate(64); + let eof_before = alloc.eof(); + + alloc.free(a, 64); + assert_eq!(alloc.allocate(64), a, "exact-fit reuse"); + assert_eq!(alloc.eof(), eof_before, "file must not grow on reuse"); + assert!(free_blocks(&alloc).is_empty()); + } + + #[test] + fn reusing_part_of_a_block_leaves_an_aligned_remainder() { + let alloc = FileAllocator::new(0); + let a = alloc.allocate(64); + alloc.allocate(8); + let eof_before = alloc.eof(); + + alloc.free(a, 64); + // 10 bytes round up to 16, so 48 bytes remain at a + 16. + assert_eq!(alloc.allocate(10), a); + assert_eq!(free_blocks(&alloc), vec![(a + 16, 48)]); + assert_eq!(alloc.allocate(48), a + 16); + assert_eq!(alloc.eof(), eof_before); + } + + #[test] + fn a_request_larger_than_every_free_block_grows_the_file() { + let alloc = FileAllocator::new(0); + let a = alloc.allocate(32); + alloc.allocate(32); + let eof_before = alloc.eof(); + + alloc.free(a, 32); + let big = alloc.allocate(33); + assert_eq!(big, eof_before, "must come from the end of the file"); + assert_eq!( + free_blocks(&alloc), + vec![(a, 32)], + "the block that did not fit stays available" + ); + } + + #[test] + fn best_fit_picks_the_smallest_sufficient_block() { + let alloc = FileAllocator::new(0); + // Separators keep the three blocks apart, so freeing them cannot + // merge them into one and the choice between them is a real one. + let small = alloc.allocate(16); + alloc.allocate(8); + let mid = alloc.allocate(32); + alloc.allocate(8); + let big = alloc.allocate(64); + alloc.allocate(8); + alloc.free(big, 64); + alloc.free(small, 16); + alloc.free(mid, 32); + + assert_eq!(alloc.allocate(20), mid, "20 fits 32 more tightly than 64"); + assert_eq!(alloc.allocate(16), small); + assert_eq!(alloc.allocate(64), big); + } + + #[test] + fn adjacent_freed_blocks_merge() { + let alloc = FileAllocator::new(0); + let a = alloc.allocate(32); + let b = alloc.allocate(32); + let c = alloc.allocate(32); + alloc.allocate(8); + + // Free the outer two first: they are not adjacent, so they stay apart. + alloc.free(a, 32); + alloc.free(c, 32); + assert_eq!(free_blocks(&alloc), vec![(a, 32), (c, 32)]); + + // Filling the hole between them collapses all three into one block, + // which is then large enough for a 96-byte request. + alloc.free(b, 32); + assert_eq!(free_blocks(&alloc), vec![(a, 96)]); + let eof_before = alloc.eof(); + assert_eq!(alloc.allocate(96), a); + assert_eq!(alloc.eof(), eof_before); + } + + #[test] + fn freeing_nothing_is_a_no_op() { + let alloc = FileAllocator::new(0); + let a = alloc.allocate(16); + alloc.free(a, 0); + assert!(free_blocks(&alloc).is_empty()); + // A zero-size request never consumes a free block either. + alloc.free(a, 16); + assert_eq!(alloc.allocate(0), alloc.eof()); + assert_eq!(free_blocks(&alloc), vec![(a, 16)]); + } } From b7fd938a5cdec2a65212ce30c3d7f1c32e08a7af Mon Sep 17 00:00:00 2001 From: Sang Woo Kim Date: Wed, 12 Aug 2026 23:59:55 +0900 Subject: [PATCH 03/20] Rewrite a chunk in place instead of abandoning its block MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every chunk write allocated fresh file space and left the previous block orphaned, so rewriting one chunk N times grew the file by N chunks. libhdf5 instead decides placement from the index entry that is already there (H5D__chunk_file_alloc): a chunk whose stored size is unchanged is overwritten where it lives, and only one that no longer fits moves and releases its old block. place_chunk is the single owner of that decision. The three record paths (extensible array, fixed array, v2 B-tree) now read their index slot *before* placing the bytes, which is what makes the decision possible at all — previously the caller had allocated and written before the index was ever consulted. record_ea_chunk therefore takes the final bytes rather than a pre-written address, so write_chunk and write_compressed_chunk no longer place bytes themselves. Under SWMR the old block is kept rather than freed: a reader may still hold an index pointing at it. This mirrors H5D__chunk_file_alloc skipping H5MF_xfree under H5F_ACC_SWMR_WRITE, and makes the in-place path the only thing keeping a SWMR rewrite from growing the file — which is what the new SWMR test pins. --- src/dataset.rs | 135 +++++++++++++++++++++++++++++++++++++++++ src/io/writer.rs | 131 ++++++++++++++++++++++++++++++--------- tests/swmr_full_api.rs | 53 ++++++++++++++++ 3 files changed, 289 insertions(+), 30 deletions(-) diff --git a/src/dataset.rs b/src/dataset.rs index 1c0ecc7..24c7913 100644 --- a/src/dataset.rs +++ b/src/dataset.rs @@ -2783,6 +2783,141 @@ mod tests { std::fs::remove_file(&path).ok(); } + /// One 2x4 i32 chunk whose every element is `v`. + fn chunk_of(v: i32) -> Vec { + (0..8).flat_map(|_| v.to_le_bytes()).collect() + } + + /// Write chunk (0,0) `rewrites` times — each time with a different value, + /// so no write can be skipped — and return the closed file's size along + /// with what the chunk reads back as. + fn rewrite_chunk( + tag: &str, + rewrites: i32, + build: impl Fn(&H5File) -> crate::H5Dataset, + ) -> (u64, i32) { + let path = temp_path(tag); + { + let file = H5File::create(&path).unwrap(); + let ds = build(&file); + for v in 1..=rewrites { + ds.write_chunk_at(&[0, 0], &chunk_of(v)).unwrap(); + } + file.close().unwrap(); + } + let size = std::fs::metadata(&path).unwrap().len(); + let first = { + let file = H5File::open(&path).unwrap(); + file.dataset("d").unwrap().read_raw::().unwrap()[0] + }; + std::fs::remove_file(&path).ok(); + (size, first) + } + + // An unfiltered chunk's stored size is fixed by the chunk shape, so + // rewriting it must overwrite the block it already occupies rather than + // abandoning it and appending a new one (libhdf5 H5D__chunk_flush_entry + // leaves must_alloc false for exactly this case). The file must therefore + // be byte-identical in size no matter how many times the chunk is written. + #[test] + fn rewriting_an_unfiltered_extensible_array_chunk_stays_in_place() { + let build = |f: &H5File| { + f.new_dataset::() + .shape([2, 4]) + .chunk(&[2, 4]) + .max_shape(&[None, Some(4)]) + .create("d") + .unwrap() + }; + let (once, _) = rewrite_chunk("rewrite_ea_1", 1, build); + let (many, last) = rewrite_chunk("rewrite_ea_8", 8, build); + assert_eq!(many, once, "8 rewrites grew the file past a single write"); + assert_eq!(last, 8, "the last write must be the one that survives"); + } + + #[test] + fn rewriting_an_unfiltered_fixed_array_chunk_stays_in_place() { + let build = |f: &H5File| { + f.new_dataset::() + .shape([2, 4]) + .chunk(&[2, 4]) + .create("d") + .unwrap() + }; + let (once, _) = rewrite_chunk("rewrite_fa_1", 1, build); + let (many, last) = rewrite_chunk("rewrite_fa_8", 8, build); + assert_eq!(many, once, "8 rewrites grew the file past a single write"); + assert_eq!(last, 8); + } + + #[test] + fn rewriting_an_unfiltered_btree_v2_chunk_stays_in_place() { + let build = |f: &H5File| { + f.new_dataset::() + .shape([2, 4]) + .chunk(&[2, 4]) + .max_shape(&[None, None]) + .create("d") + .unwrap() + }; + let (once, _) = rewrite_chunk("rewrite_bt2_1", 1, build); + let (many, last) = rewrite_chunk("rewrite_bt2_8", 8, build); + assert_eq!(many, once, "8 rewrites grew the file past a single write"); + assert_eq!(last, 8); + } + + // A filtered chunk whose compressed size changes cannot stay put, so it + // moves and releases its old block (libhdf5 H5D__chunk_file_alloc calls + // H5MF_xfree). Alternating between two payloads of different compressed + // size must therefore keep reusing the same two blocks instead of + // appending a fresh one each time. + #[test] + fn rewriting_a_filtered_chunk_recycles_the_released_block() { + // All-equal elements deflate to far fewer bytes than a varied payload, + // so the two writes below land at different stored sizes. + let flat: Vec = (0..8).flat_map(|_| 7i32.to_le_bytes()).collect(); + let varied: Vec = (0..8i32) + .flat_map(|i| i.wrapping_mul(0x5bd1_e995).to_le_bytes()) + .collect(); + + let sizes: Vec = [1usize, 8] + .iter() + .map(|&rounds| { + let path = temp_path(&format!("rewrite_filtered_{rounds}")); + { + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([2, 4]) + .chunk(&[2, 4]) + .max_shape(&[None, Some(4)]) + .deflate(6) + .create("d") + .unwrap(); + for _ in 0..rounds { + ds.write_chunk_at(&[0, 0], &flat).unwrap(); + ds.write_chunk_at(&[0, 0], &varied).unwrap(); + } + file.close().unwrap(); + } + let size = std::fs::metadata(&path).unwrap().len(); + { + let file = H5File::open(&path).unwrap(); + let got = file.dataset("d").unwrap().read_raw::().unwrap(); + let want: Vec = (0..8i32).map(|i| i.wrapping_mul(0x5bd1_e995)).collect(); + assert_eq!(got, want, "the last write must survive the round trip"); + } + std::fs::remove_file(&path).ok(); + size + }) + .collect(); + + assert_eq!( + sizes[1], sizes[0], + "8 alternating rewrites grew the file past a single pair" + ); + } + #[test] fn write_slice_out_of_bounds_rejected() { let path = temp_path("write_slice_oob"); diff --git a/src/io/writer.rs b/src/io/writer.rs index da5cb6b..b5bcc4f 100644 --- a/src/io/writer.rs +++ b/src/io/writer.rs @@ -414,6 +414,14 @@ pub struct Hdf5Writer { /// write path takes it, so it cannot deadlock with the registry locks. pub(crate) create_lock: Slot<()>, closed: bool, + /// Set once `finalize_for_swmr` has published a readable file. + /// + /// A SWMR reader may hold a chunk index that still points at a block this + /// writer has since replaced, so from that point on a relocated chunk's + /// old block is kept rather than released for reuse — the same rule as + /// libhdf5's `H5D__chunk_file_alloc`, which skips `H5MF_xfree` under + /// `H5F_ACC_SWMR_WRITE`. + swmr_active: bool, /// Address of the root group object header (set after first finalize). root_group_addr: Option, /// Size of the encoded root group object header (for in-place rewrites). @@ -467,6 +475,7 @@ impl Hdf5Writer { root_attributes: Slot::new(Vec::new()), create_lock: Slot::new(()), closed: false, + swmr_active: false, root_group_addr: None, root_group_encoded_size: 0, }) @@ -936,6 +945,7 @@ impl Hdf5Writer { root_attributes: Slot::new(root_attributes), create_lock: Slot::new(()), closed: false, + swmr_active: false, root_group_addr: None, root_group_encoded_size: 0, }) @@ -1654,29 +1664,57 @@ impl Hdf5Writer { } else { data }; - let compressed_size = write_data.len() as u64; - - // Allocate space for the chunk data - let chunk_addr = self.allocator.allocate(compressed_size); - self.handle.write_at(chunk_addr, write_data)?; - // filter_mask = 0: this path runs the whole pipeline, so no filter is // skipped for the chunk. - self.record_ea_chunk(index, chunk_idx, chunk_addr, compressed_size, 0) + self.record_ea_chunk(index, chunk_idx, write_data, 0) + } + + /// Decide where a chunk's bytes belong and put them there, returning the + /// address to record in the index. + /// + /// `old` is the chunk's current `(address, stored length)` if the index + /// already holds an entry for it. This is the single owner of the + /// rewrite-placement rule, mirroring libhdf5's `H5D__chunk_file_alloc` + /// (`H5Dchunk.c`): a chunk whose stored size is unchanged is overwritten + /// where it already lives, and only a chunk that no longer fits moves, + /// releasing its old block. Without this every rewrite would abandon the + /// old block and grow the file. + fn place_chunk(&self, old: Option<(u64, u64)>, new_len: u64) -> u64 { + match old { + // Same stored size: overwrite in place. This is every unfiltered + // rewrite (the stored size is fixed by the chunk shape) and every + // filtered rewrite that compressed to the same length. + Some((addr, len)) if addr != UNDEF_ADDR && len == new_len => addr, + Some((addr, len)) if addr != UNDEF_ADDR => { + // The chunk has to move. Under SWMR a reader may still hold an + // index that points at the old block, so libhdf5 keeps it + // (H5D__chunk_file_alloc skips H5MF_xfree when the file is + // open for SWMR writing); do the same. + if !self.swmr_active { + self.allocator.free(addr, len); + } + self.allocator.allocate(new_len) + } + _ => self.allocator.allocate(new_len), + } } - /// Record a written chunk in the extensible-array index, placing - /// its address (and compressed size, for filtered datasets) into - /// the index block, a data block, or a super block per the EA - /// geometry. Shared by write_chunk and write_compressed_chunk. + /// Place a chunk's already-final bytes (filtered if the dataset is + /// filtered) in the file and record them in the extensible-array index — + /// in the index block, a data block, or a super block per the EA geometry. + /// Shared by write_chunk and write_compressed_chunk. + /// + /// The index lookup happens *before* the bytes are placed, because the + /// entry it finds is what tells [`place_chunk`](Self::place_chunk) whether + /// this is a rewrite that can stay put. fn record_ea_chunk( &self, index: usize, chunk_idx: u64, - chunk_addr: u64, - compressed_size: u64, + final_bytes: &[u8], filter_mask: u32, ) -> IoResult<()> { + let compressed_size = final_bytes.len() as u64; let ds = self.ds(index); // Hold one slot guard for the whole method: every dataset-state access // below goes through `m`, while `self.handle`/`self.allocator`/`self.ctx` @@ -1709,6 +1747,10 @@ impl Hdf5Writer { let chunked = m.chunked.as_mut().unwrap(); if is_filtered { if let Some(ref mut fiblk) = chunked.filt_iblk { + let old = fiblk.elements[chunk_idx as usize]; + let chunk_addr = + self.place_chunk(Some((old.addr, old.nbytes)), compressed_size); + self.handle.write_at(chunk_addr, final_bytes)?; fiblk.elements[chunk_idx as usize] = FilteredChunkEntry { addr: chunk_addr, nbytes: compressed_size, @@ -1716,6 +1758,11 @@ impl Hdf5Writer { }; } } else { + // An unfiltered chunk's stored size is fixed by the chunk + // shape, so a rewrite always fits where it already is. + let old = chunked.ea_iblk.elements[chunk_idx as usize]; + let chunk_addr = self.place_chunk(Some((old, compressed_size)), compressed_size); + self.handle.write_at(chunk_addr, final_bytes)?; chunked.ea_iblk.elements[chunk_idx as usize] = chunk_addr; } chunked.chunks_written += 1; @@ -1831,11 +1878,6 @@ impl Hdf5Writer { // Create or update the data block holding this chunk's entry. let created = dblk_addr == UNDEF_ADDR; if is_filtered { - let entry = FilteredChunkEntry { - addr: chunk_addr, - nbytes: compressed_size, - filter_mask, - }; let mut dblk = if created { FilteredDataBlock::new(ea_header_addr, loc.dblk_block_offset, dblk_nelmts) } else { @@ -1848,6 +1890,16 @@ impl Hdf5Writer { chunk_size_len, )? }; + // A freshly created data block holds only undefined addresses, + // so this reads as "no previous chunk" without a special case. + let old = dblk.elements[loc.offset_in_dblk as usize]; + let chunk_addr = self.place_chunk(Some((old.addr, old.nbytes)), compressed_size); + self.handle.write_at(chunk_addr, final_bytes)?; + let entry = FilteredChunkEntry { + addr: chunk_addr, + nbytes: compressed_size, + filter_mask, + }; dblk.elements[loc.offset_in_dblk as usize] = entry; let enc = dblk.encode(&self.ctx, max_nelmts_bits, chunk_size_len); if created { @@ -1870,6 +1922,13 @@ impl Hdf5Writer { let buf = self.handle.read_at_most(dblk_addr, 65536)?; ExtensibleArrayDataBlock::decode(&buf, &self.ctx, max_nelmts_bits, dblk_nelmts)? }; + // Unfiltered: the stored size is fixed by the chunk shape, so + // a rewrite always fits its old block. A freshly created data + // block holds undefined addresses and falls through to a new + // allocation. + let old = dblk.elements[loc.offset_in_dblk as usize]; + let chunk_addr = self.place_chunk(Some((old, compressed_size)), compressed_size); + self.handle.write_at(chunk_addr, final_bytes)?; dblk.elements[loc.offset_in_dblk as usize] = chunk_addr; let enc = dblk.encode(&self.ctx, max_nelmts_bits); if created { @@ -3415,11 +3474,8 @@ impl Hdf5Writer { stride *= n_chunks_in_dim; } - // Allocate space for the chunk data - let chunk_addr = self.allocator.allocate(final_bytes.len() as u64); - self.handle.write_at(chunk_addr, final_bytes)?; - - // Update the fixed array data block. + // Update the fixed array data block. The slot is read before the bytes + // are placed so a rewrite can stay where it is (see `place_chunk`). let fa = m.fixed_array.as_mut().unwrap(); let lidx = linear_idx as usize; if is_filtered { @@ -3449,6 +3505,12 @@ impl Hdf5Writer { ))); } if lidx < fa.fa_dblk.filtered_elements.len() { + let old = &fa.fa_dblk.filtered_elements[lidx]; + let chunk_addr = self.place_chunk( + Some((old.address, old.chunk_size as u64)), + stored_size as u64, + ); + self.handle.write_at(chunk_addr, final_bytes)?; fa.fa_dblk.filtered_elements[lidx] = FixedArrayFilteredChunkElement { address: chunk_addr, chunk_size: stored_size as u32, @@ -3471,6 +3533,12 @@ impl Hdf5Writer { )); } if lidx < fa.fa_dblk.elements.len() { + // Unfiltered: the stored size is fixed by the chunk shape, so + // a rewrite always fits its old block. + let old = fa.fa_dblk.elements[lidx]; + let len = final_bytes.len() as u64; + let chunk_addr = self.place_chunk(Some((old, len)), len); + self.handle.write_at(chunk_addr, final_bytes)?; fa.fa_dblk.elements[lidx] = chunk_addr; fa.chunks_written += 1; } else { @@ -3513,8 +3581,11 @@ impl Hdf5Writer { ))); } - // Allocate space for the chunk data - let chunk_addr = self.allocator.allocate(chunk_bytes); + // Place the bytes: a rewrite of an already-recorded chunk stays where + // it is, since an unfiltered chunk's stored size is fixed by the chunk + // shape (see `place_chunk`). + let old = bt2.index.lookup(chunk_coords).map(|r| r.chunk_address); + let chunk_addr = self.place_chunk(old.map(|a| (a, chunk_bytes)), chunk_bytes); self.handle.write_at(chunk_addr, data)?; // Insert into the in-memory BT2 index @@ -3620,11 +3691,7 @@ impl Hdf5Writer { .into(), )); } - let compressed_size = compressed_data.len() as u64; - let chunk_addr = self.allocator.allocate(compressed_size); - self.handle.write_at(chunk_addr, compressed_data)?; - - self.record_ea_chunk(index, chunk_idx, chunk_addr, compressed_size, filter_mask) + self.record_ea_chunk(index, chunk_idx, compressed_data, filter_mask) } /// Extend the dimensions of a chunked dataset. @@ -3960,6 +4027,10 @@ impl Hdf5Writer { self.write_superblock(FLAG_WRITE_ACCESS | FLAG_SWMR_WRITE)?; self.handle.sync_all()?; + // Readers can now be following this file, so a chunk that moves must + // leave its old block intact for whoever is still holding the previous + // index (see `swmr_active`). + self.swmr_active = true; Ok(()) } diff --git a/tests/swmr_full_api.rs b/tests/swmr_full_api.rs index 6f5042e..ba164e6 100644 --- a/tests/swmr_full_api.rs +++ b/tests/swmr_full_api.rs @@ -400,3 +400,56 @@ fn grid_dataset_positioned_write_rejects_misuse() { w.close().unwrap(); cleanup(&path); } + +/// Under SWMR a relocated chunk's old block must stay intact for readers still +/// holding the previous index, so the allocator cannot recycle it. An +/// unfiltered chunk therefore has only one way not to grow the file on +/// rewrite: overwrite the block it already occupies (libhdf5 does the same — +/// `H5D__chunk_flush_entry` leaves `must_alloc` false when the address is +/// already defined and no filter changes the stored size). +#[test] +fn swmr_chunk_rewrite_does_not_grow_the_file() { + const H: u64 = 4; + const W: u64 = 4; + let chunk = |v: u16| -> Vec { + (0..(H * W) as u16) + .flat_map(|p| (v + p).to_le_bytes()) + .collect() + }; + + // Same file, written once vs rewritten eight times over. + let write = |label: &str, rewrites: u16| -> (PathBuf, u64) { + let path = unique_tmp(label); + { + let mut w = SwmrFileWriter::create_with_locking(&path, NO_LOCK).unwrap(); + let ds = w + .create_grid_dataset::("grid", &[1, 1, H, W], &[1, 1, H, W]) + .unwrap(); + w.start_swmr().unwrap(); + for v in 1..=rewrites { + w.write_chunk_at(ds, &[0, 0, 0, 0], &chunk(v * 100)) + .unwrap(); + w.flush().unwrap(); + } + w.close().unwrap(); + } + let size = std::fs::metadata(&path).unwrap().len(); + (path, size) + }; + + let (once_path, once) = write("swmr_rewrite_1", 1); + let (many_path, many) = write("swmr_rewrite_8", 8); + assert_eq!( + many, once, + "8 SWMR rewrites of one chunk grew the file past a single write" + ); + + // The last write is the one that survives. + let mut r = SwmrFileReader::open_with_locking(&many_path, NO_LOCK).unwrap(); + let data: Vec = r.read_dataset("grid").unwrap(); + assert_eq!(data[0], 800); + drop(r); + + cleanup(&once_path); + cleanup(&many_path); +} From e8e707e2c8f5e45ac682fa7ce885eb9378720879 Mon Sep 17 00:00:00 2001 From: Sang Woo Kim Date: Thu, 13 Aug 2026 00:14:27 +0900 Subject: [PATCH 04/20] Gate the filtered-chunk rewrite test on the deflate feature The test added with the in-place chunk rewrite builds a `.deflate(6)` dataset, so it fails under `--no-default-features` with "deflate filter requires the 'deflate' feature" rather than skipping. The other filter tests in the suite already carry this gate. --- src/dataset.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/dataset.rs b/src/dataset.rs index 24c7913..8fec66f 100644 --- a/src/dataset.rs +++ b/src/dataset.rs @@ -2871,6 +2871,7 @@ mod tests { // H5MF_xfree). Alternating between two payloads of different compressed // size must therefore keep reusing the same two blocks instead of // appending a fresh one each time. + #[cfg(feature = "deflate")] #[test] fn rewriting_a_filtered_chunk_recycles_the_released_block() { // All-equal elements deflate to far fewer bytes than a varied payload, From a8e13ba3933527b2cd449882d1da60237959fab0 Mon Sep 17 00:00:00 2001 From: Sang Woo Kim Date: Thu, 13 Aug 2026 00:14:56 +0900 Subject: [PATCH 05/20] Support write_slice on chunked datasets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit write_slice rejected any chunked dataset with "write_slice is only for contiguous datasets", so updating one row of an appendable dataset meant reading it whole, editing the copy, and writing it back: O(dataset) memory and I/O for an O(row) change (issue #2). There is no format-level reason for the restriction — libhdf5 treats a chunked hyperslab write as its ordinary path, not a special case. write_slice now validates the selection once and dispatches on the layout. The chunked writer decomposes the selection onto the chunk grid and touches only the chunks it intersects. A chunk the selection covers completely is built from the caller's buffer alone; a partially covered chunk is read back, patched and rewritten, so its other elements survive and a chunk that did not exist yet starts from the fill value. libhdf5 draws the same line with the `relax` flag of H5D__chunk_lock. Three pieces make the walk index-agnostic: - ChunkGeometry owns the grid arithmetic. Its linear_index multiplies the extents the chunks were actually indexed under (current dims) while bounding each coordinate by max_dims, so an unlimited dimension stays unbounded and a fixed one still rejects a coordinate that would alias another chunk's slot. - read_chunk_at_coords / write_chunk_at_coords are the read and write halves of the read-modify-write, covering the extensible array, fixed array and v2 B-tree indexes behind one entry point. - read_chunk_block is the shared read-and-reverse-filters tail, the one place that decides a chunk has no block yet. Tests cover each boundary of the decomposition rule — whole-chunk vs partial coverage, a selection straddling all four chunks, an edge chunk hanging past the extent, untouched elements reading as the fill value, three dimensions, and repeated patches — across all three index types and, where the filter is built in, deflate. Two h5py cross-checks confirm libhdf5 reads the result, including a filtered chunk that grew under recompression and had to be relocated. --- src/dataset.rs | 19 +- src/io/writer.rs | 392 +++++++++++++++++++++++++++++++-- tests/chunked_write_slice.rs | 346 +++++++++++++++++++++++++++++ tests/h5py_cross_validation.rs | 87 ++++++++ 4 files changed, 813 insertions(+), 31 deletions(-) create mode 100644 tests/chunked_write_slice.rs diff --git a/src/dataset.rs b/src/dataset.rs index 8fec66f..2f127f4 100644 --- a/src/dataset.rs +++ b/src/dataset.rs @@ -1713,9 +1713,18 @@ impl H5Dataset { } } - /// Write a typed slice to a sub-region of a contiguous dataset. + /// Write a typed slice to a sub-region of the dataset. /// - /// `starts` and `counts` define the N-dimensional selection. + /// `starts` and `counts` define the N-dimensional selection, which must lie + /// inside the dataset's current extent. + /// + /// Works for both contiguous and chunked datasets. For a chunked dataset + /// only the chunks the selection touches are rewritten — a partially + /// covered chunk is read back, patched, and written again, so updating one + /// row of an appendable dataset costs the chunks that row crosses rather + /// than the whole dataset. Elements of a touched chunk that the selection + /// does not cover keep their stored value, or the dataset's fill value if + /// the chunk did not exist yet. pub fn write_slice( &self, starts: &[usize], @@ -1726,14 +1735,8 @@ impl H5Dataset { DatasetInfo::Writer { index, element_size, - chunked, .. } => { - if *chunked { - return Err(Hdf5Error::InvalidState( - "write_slice is only for contiguous datasets".into(), - )); - } if T::element_size() != *element_size { return Err(Hdf5Error::TypeMismatch(format!( "write type has element size {} but dataset expects {}", diff --git a/src/io/writer.rs b/src/io/writer.rs index b5bcc4f..898fe3d 100644 --- a/src/io/writer.rs +++ b/src/io/writer.rs @@ -32,7 +32,7 @@ use crate::format::{FormatContext, UNDEF_ADDR}; use crate::io::allocator::FileAllocator; use crate::io::file_handle::FileHandle; -use crate::io::hyperslab::for_each_contiguous_run; +use crate::io::hyperslab::{for_each_contiguous_run, for_each_dual_run}; use crate::io::IoResult; /// On-disk size in bytes of a fixed-array data block, for the layout (paged or @@ -291,6 +291,100 @@ enum DblkParent { }, } +/// Which chunk index a dataset uses. libhdf5 picks it from the dataspace: a +/// v2 B-tree for two or more unlimited dimensions, an extensible array for +/// exactly one, a fixed array for none. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +enum ChunkIndexKind { + ExtensibleArray, + FixedArray, + BtreeV2, +} + +/// A chunked dataset's grid geometry, snapshotted out of its slot. +/// +/// The single owner of chunk-grid arithmetic: how many chunks span each +/// dimension, where a coordinate sits in the row-major order the array +/// indices record, and how many bytes one chunk holds. +struct ChunkGeometry { + kind: ChunkIndexKind, + dims: Vec, + max_dims: Option>, + chunk_dims: Vec, + element_size: u64, +} + +impl ChunkGeometry { + /// Unfiltered byte size of one whole chunk. + fn chunk_bytes(&self) -> u64 { + self.chunk_dims.iter().product::() * self.element_size + } + + /// Number of chunks spanning each dimension at the current extent. + fn grid(&self) -> Vec { + self.dims + .iter() + .zip(&self.chunk_dims) + .map(|(&d, &c)| if c > 0 { d.div_ceil(c) } else { 0 }) + .collect() + } + + /// Row-major position of `coords` in the chunk grid — the linear index an + /// extensible or fixed array records the chunk under. + /// + /// The grid extents multiplied here come from the *current* dims, which is + /// how every chunk written so far was indexed. The bound a coordinate is + /// checked against comes from `max_dims`, so an unlimited dimension — the + /// one an extensible array exists to grow — is unbounded, while a fixed + /// dimension still rejects an out-of-grid coordinate that would otherwise + /// silently alias another chunk's slot. + fn linear_index(&self, coords: &[u64]) -> IoResult { + let ndims = self.dims.len(); + if coords.len() != ndims { + return Err(crate::io::IoError::InvalidState(format!( + "chunk_coords has {} entries but the dataset has {} dimensions", + coords.len(), + ndims + ))); + } + if self.chunk_dims.len() != ndims { + return Err(crate::io::IoError::InvalidState(format!( + "dataset chunk shape has {} dimensions but the dataspace has {}", + self.chunk_dims.len(), + ndims + ))); + } + let grid = self.grid(); + let mut linear = 0u64; + for d in 0..ndims { + if self.chunk_dims[d] == 0 { + return Err(crate::io::IoError::InvalidState(format!( + "chunk dimension {d} is zero" + ))); + } + let extent = self.max_dims.as_ref().map_or(self.dims[d], |m| m[d]); + if extent != u64::MAX { + let bound = extent.div_ceil(self.chunk_dims[d]); + if coords[d] >= bound { + return Err(crate::io::IoError::InvalidState(format!( + "chunk coordinate {} in dimension {} is outside the chunk grid (0..{})", + coords[d], d, bound + ))); + } + } + linear = linear + .checked_mul(grid[d]) + .and_then(|l| l.checked_add(coords[d])) + .ok_or_else(|| { + crate::io::IoError::InvalidState( + "chunk coordinates overflow the array index".into(), + ) + })?; + } + Ok(linear) + } +} + /// Runtime metadata for a fixed-array-indexed chunked dataset. pub struct FixedArrayDatasetInfo { /// Chunk dimension sizes. @@ -1986,10 +2080,14 @@ impl Hdf5Writer { Ok(()) } - /// Write a slice (hyperslab) of data to a contiguous dataset. + /// Write a slice (hyperslab) of data to a dataset, contiguous or chunked. /// /// `starts` and `counts` define the N-dimensional selection. /// `data` must be exactly `product(counts) * element_size` bytes. + /// + /// The selection is validated once here and then handed to the layout's + /// own writer, so a caller never has to know which storage the dataset + /// uses. pub fn write_slice( &self, index: usize, @@ -1999,16 +2097,7 @@ impl Hdf5Writer { ) -> IoResult<()> { let ds_ref = self.ds(index); let ds = ds_ref.lock(); - if ds.chunked.is_some() || ds.fixed_array.is_some() || ds.btree_v2.is_some() { - return Err(crate::io::IoError::InvalidState( - "write_slice is only for contiguous datasets".into(), - )); - } - if ds.data_addr == UNDEF_ADDR { - return Err(crate::io::IoError::InvalidState( - "dataset has no data allocated".into(), - )); - } + let is_chunked = ds.chunked.is_some() || ds.fixed_array.is_some() || ds.btree_v2.is_some(); let dims = &ds.dataspace.dims; let element_size = ds.datatype.element_size() as u64; @@ -2048,10 +2137,20 @@ impl Hdf5Writer { ))); } - let base_addr = ds.data_addr; - // `dims` borrows self.datasets; collect what the run iterator needs so - // the closure can borrow self.handle (a disjoint field) for the write. + // `dims` borrows the dataset slot; collect what the writers below need + // so the guard can be dropped before they re-lock it. let dims = dims.clone(); + let base_addr = ds.data_addr; + drop(ds); + + if is_chunked { + return self.write_slice_chunked(index, starts, counts, data); + } + if base_addr == UNDEF_ADDR { + return Err(crate::io::IoError::InvalidState( + "dataset has no data allocated".into(), + )); + } // Write each maximal contiguous run in one `write_at`. Trailing // full-selected dimensions coalesce, mirroring the read path: a slice @@ -2072,6 +2171,125 @@ impl Hdf5Writer { Ok(()) } + /// Write a hyperslab into a chunked dataset, one chunk at a time. + /// + /// The selection is already validated by [`write_slice`](Self::write_slice). + /// For each chunk the selection touches, the chunk's share of `data` is + /// scattered into a whole-chunk buffer and the chunk is rewritten: + /// + /// - a chunk the selection covers completely is built from `data` alone — + /// nothing needs reading back (libhdf5 takes the same shortcut with the + /// `relax` flag of `H5D__chunk_lock`); + /// - a chunk covered only in part starts from what is already stored, or + /// from a fill-value buffer when the chunk has never been written, so + /// neighbouring elements survive and untouched ones read as fill. + /// + /// An edge chunk that hangs past the dataset extent is always the partial + /// case, so the region beyond the extent keeps its fill value. + fn write_slice_chunked( + &self, + index: usize, + starts: &[u64], + counts: &[u64], + data: &[u8], + ) -> IoResult<()> { + if counts.contains(&0) { + return Ok(()); + } + let geo = self.chunk_geometry(index)?; + let ndims = geo.dims.len(); + if geo.chunk_dims.len() != ndims { + return Err(crate::io::IoError::InvalidState(format!( + "dataset chunk shape has {} dimensions but the dataspace has {}", + geo.chunk_dims.len(), + ndims + ))); + } + if geo.chunk_dims.contains(&0) { + return Err(crate::io::IoError::InvalidState( + "chunk shape has a zero-length dimension".into(), + )); + } + let chunk_bytes = geo.chunk_bytes() as usize; + + // Grid range the selection touches, inclusive on both ends. + let first: Vec = (0..ndims).map(|d| starts[d] / geo.chunk_dims[d]).collect(); + let last: Vec = (0..ndims) + .map(|d| (starts[d] + counts[d] - 1) / geo.chunk_dims[d]) + .collect(); + + let mut coords = first.clone(); + loop { + // Intersect the selection with this chunk. `in_chunk` is the + // region's origin inside the chunk, `in_data` its origin inside + // the caller's counts-shaped buffer, `extent` its size. + let mut in_chunk = vec![0u64; ndims]; + let mut in_data = vec![0u64; ndims]; + let mut extent = vec![0u64; ndims]; + let mut covers_whole_chunk = true; + for d in 0..ndims { + let chunk_origin = coords[d] * geo.chunk_dims[d]; + let lo = starts[d].max(chunk_origin); + let hi = (starts[d] + counts[d]).min(chunk_origin + geo.chunk_dims[d]); + in_chunk[d] = lo - chunk_origin; + in_data[d] = lo - starts[d]; + extent[d] = hi - lo; + if in_chunk[d] != 0 || extent[d] != geo.chunk_dims[d] { + covers_whole_chunk = false; + } + } + + let mut buf = if covers_whole_chunk { + // Every byte is overwritten below. + vec![0u8; chunk_bytes] + } else { + match self.read_chunk_at_coords(index, &coords)? { + Some(existing) => { + if existing.len() != chunk_bytes { + return Err(crate::io::IoError::InvalidState(format!( + "stored chunk at {coords:?} is {} bytes but the chunk shape \ + needs {chunk_bytes}", + existing.len() + ))); + } + existing + } + None => self.new_chunk_buffer(index, chunk_bytes), + } + }; + + for_each_dual_run( + &geo.chunk_dims, + &in_chunk, + counts, + &in_data, + &extent, + geo.element_size, + |dst_off, src_off, len| { + let dst = dst_off as usize; + let src = src_off as usize; + buf[dst..dst + len].copy_from_slice(&data[src..src + len]); + Ok(()) + }, + )?; + self.write_chunk_at_coords(index, &coords, &buf)?; + + // Odometer over the touched grid range. + let mut d = ndims; + loop { + if d == 0 { + return Ok(()); + } + d -= 1; + if coords[d] < last[d] { + coords[d] += 1; + break; + } + coords[d] = first[d]; + } + } + } + /// Add an attribute to the root group (file-level attribute). pub fn add_root_attribute(&self, attr: crate::format::messages::attribute::AttributeMessage) { // Replace existing attribute with the same name, or append new one. @@ -2868,21 +3086,149 @@ impl Hdf5Writer { } } }; + self.read_chunk_block(pipeline.as_ref(), addr, nbytes, mask) + } + + /// Read one stored chunk block and undo its filters. + /// + /// `nbytes` is the *stored* length and `mask` the chunk's filter mask, so + /// a chunk written by a direct chunk write with a skipped filter is + /// reversed correctly. `Ok(None)` means the chunk has no block yet — the + /// single place that judgement is made, shared by every chunk index. + fn read_chunk_block( + &self, + pipeline: Option<&FilterPipeline>, + addr: u64, + nbytes: u64, + mask: u32, + ) -> IoResult>> { if addr == UNDEF_ADDR || nbytes == 0 { return Ok(None); } - let raw = self.handle.read_at(addr, nbytes as usize)?; - if is_filtered { - let Some(pl) = pipeline.as_ref() else { - return Ok(None); - }; - Ok(Some(filter::reverse_filters_masked(pl, &raw, mask)?)) - } else { - Ok(Some(raw)) + match pipeline { + Some(pl) => Ok(Some(filter::reverse_filters_masked(pl, &raw, mask)?)), + None => Ok(Some(raw)), + } + } + + /// Read the *decompressed* bytes of the chunk at `chunk_coords`, whichever + /// chunk index the dataset uses, or `Ok(None)` when that chunk has never + /// been written. + /// + /// This is the read half of a partial-chunk read-modify-write: a hyperslab + /// write that covers only part of a chunk must start from what is already + /// there. Keeping one entry point for all three index types is what lets + /// [`write_slice`](Self::write_slice) stay index-agnostic. + pub(crate) fn read_chunk_at_coords( + &self, + ds_index: usize, + chunk_coords: &[u64], + ) -> IoResult>> { + let geo = self.chunk_geometry(ds_index)?; + let linear = geo.linear_index(chunk_coords)?; + match geo.kind { + ChunkIndexKind::ExtensibleArray => self.read_chunk_if_present(ds_index, linear), + ChunkIndexKind::FixedArray => { + let ds = self.ds(ds_index); + let m = ds.lock(); + let pipeline = m.filter_pipeline.clone(); + let fa = m.fixed_array.as_ref().unwrap(); + let lidx = linear as usize; + let (addr, nbytes, mask) = if pipeline.is_some() { + match fa.fa_dblk.filtered_elements.get(lidx) { + Some(e) => (e.address, e.chunk_size as u64, e.filter_mask), + None => return Ok(None), + } + } else { + match fa.fa_dblk.elements.get(lidx) { + Some(&a) => (a, geo.chunk_bytes(), 0), + None => return Ok(None), + } + }; + drop(m); + self.read_chunk_block(pipeline.as_ref(), addr, nbytes, mask) + } + ChunkIndexKind::BtreeV2 => { + let ds = self.ds(ds_index); + let m = ds.lock(); + let pipeline = m.filter_pipeline.clone(); + let bt2 = m.btree_v2.as_ref().unwrap(); + let found = if bt2.index.filtered { + bt2.index + .lookup_filtered(chunk_coords) + .map(|r| (r.chunk_address, r.chunk_size as u64, r.filter_mask)) + } else { + bt2.index + .lookup(chunk_coords) + .map(|r| (r.chunk_address, geo.chunk_bytes(), 0)) + }; + drop(m); + match found { + Some((addr, nbytes, mask)) => { + self.read_chunk_block(pipeline.as_ref(), addr, nbytes, mask) + } + None => Ok(None), + } + } + } + } + + /// Write one whole chunk addressed by its grid coordinates, whichever + /// chunk index the dataset uses. `data` is the chunk's unfiltered bytes; + /// the dataset's filter pipeline (if any) runs here. + /// + /// The write half of the pair with + /// [`read_chunk_at_coords`](Self::read_chunk_at_coords). Unlike the + /// dataset-level `write_chunk_at`, this never grows the dataspace — a + /// hyperslab write is bounded by the current extent by definition. + pub(crate) fn write_chunk_at_coords( + &self, + ds_index: usize, + chunk_coords: &[u64], + data: &[u8], + ) -> IoResult<()> { + let geo = self.chunk_geometry(ds_index)?; + match geo.kind { + ChunkIndexKind::ExtensibleArray => { + let linear = geo.linear_index(chunk_coords)?; + self.write_chunk(ds_index, linear, data) + } + ChunkIndexKind::FixedArray => { + self.write_chunk_fixed_array(ds_index, chunk_coords, data) + } + ChunkIndexKind::BtreeV2 => self.write_chunk_btree_v2(ds_index, chunk_coords, data), } } + /// Snapshot the geometry needed to address a chunked dataset's grid. + /// + /// Taken under one brief slot guard so the callers below — which re-lock + /// the slot through `write_chunk`/`read_chunk_*` — never hold it across + /// compression or I/O. + fn chunk_geometry(&self, ds_index: usize) -> IoResult { + let ds = self.ds(ds_index); + let m = ds.lock(); + let (kind, chunk_dims) = if let Some(ref c) = m.chunked { + (ChunkIndexKind::ExtensibleArray, c.chunk_dims.clone()) + } else if let Some(ref f) = m.fixed_array { + (ChunkIndexKind::FixedArray, f.chunk_dims.clone()) + } else if let Some(ref b) = m.btree_v2 { + (ChunkIndexKind::BtreeV2, b.chunk_dims.clone()) + } else { + return Err(crate::io::IoError::InvalidState( + "not a chunked dataset".into(), + )); + }; + Ok(ChunkGeometry { + kind, + dims: m.dataspace.dims.clone(), + max_dims: m.dataspace.max_dims.clone(), + chunk_dims, + element_size: m.datatype.element_size() as u64, + }) + } + /// Define a chunked dataset indexed by a fixed array (no unlimited dimensions). /// /// `dims` and `max_dims` should be the same (all fixed). `chunk_dims` defines the diff --git a/tests/chunked_write_slice.rs b/tests/chunked_write_slice.rs new file mode 100644 index 0000000..c37d7d8 --- /dev/null +++ b/tests/chunked_write_slice.rs @@ -0,0 +1,346 @@ +//! Hyperslab writes into chunked datasets. +//! +//! One case per boundary of the chunk-decomposition rule: whole-chunk vs +//! partial coverage, first/last chunk of a span, edge chunks that hang past +//! the extent, never-written chunks, each of the three chunk index types, and +//! filtered storage where a partial write has to decompress and recompress. + +use rust_hdf5::{H5Dataset, H5File}; + +fn tmp(name: &str) -> std::path::PathBuf { + use std::sync::atomic::{AtomicU64, Ordering}; + static COUNTER: AtomicU64 = AtomicU64::new(0); + let n = COUNTER.fetch_add(1, Ordering::Relaxed); + std::env::temp_dir().join(format!( + "rust_hdf5_chunk_slice_{}_{}_{}.h5", + name, + std::process::id(), + n + )) +} + +/// 4x6 i32 dataset in 2x3 chunks (a 2x2 chunk grid), created with `build`, +/// zero-filled by writing every chunk, then patched by `patch`. Returns what +/// the whole dataset reads back as. +fn round_trip( + name: &str, + build: impl Fn(&H5File) -> H5Dataset, + patch: impl Fn(&H5Dataset), +) -> Vec { + let path = tmp(name); + { + let file = H5File::create(&path).unwrap(); + let ds = build(&file); + // Seed every element with its own index so any clobbering is visible. + let seed: Vec = (0..24).collect(); + ds.write_slice(&[0, 0], &[4, 6], &seed).unwrap(); + patch(&ds); + file.close().unwrap(); + } + let out = { + let file = H5File::open(&path).unwrap(); + file.dataset("d").unwrap().read_raw::().unwrap() + }; + std::fs::remove_file(&path).ok(); + out +} + +fn fixed_array(file: &H5File) -> H5Dataset { + file.new_dataset::() + .shape([4, 6]) + .chunk(&[2, 3]) + .create("d") + .unwrap() +} + +fn extensible_array(file: &H5File) -> H5Dataset { + file.new_dataset::() + .shape([4, 6]) + .chunk(&[2, 3]) + .max_shape(&[None, Some(6)]) + .create("d") + .unwrap() +} + +fn btree_v2(file: &H5File) -> H5Dataset { + file.new_dataset::() + .shape([4, 6]) + .chunk(&[2, 3]) + .max_shape(&[None, None]) + .create("d") + .unwrap() +} + +#[cfg(feature = "deflate")] +fn deflated(file: &H5File) -> H5Dataset { + file.new_dataset::() + .shape([4, 6]) + .chunk(&[2, 3]) + .max_shape(&[None, Some(6)]) + .deflate(6) + .create("d") + .unwrap() +} + +/// A storage layout to run a case against: a label for assertion messages and +/// the constructor that produces a dataset with that layout. +type Layout = (&'static str, fn(&H5File) -> H5Dataset); + +/// Every storage layout a 4x6/2x3 dataset can have, paired with a label for +/// assertion messages. Filtered storage only appears when the filter is built +/// in, so the suite still passes under `--no-default-features`. +fn layouts() -> Vec { + let mut cases: Vec = vec![ + ("fa", fixed_array), + ("ea", extensible_array), + ("bt2", btree_v2), + ]; + cases.extend(filtered_layout()); + cases +} + +#[cfg(feature = "deflate")] +fn filtered_layout() -> Option { + Some(("deflate", deflated)) +} + +#[cfg(not(feature = "deflate"))] +fn filtered_layout() -> Option { + None +} + +/// Expected contents of the seeded 4x6 grid after writing `values` into the +/// region at `starts` of size `counts`. +fn expected(starts: [usize; 2], counts: [usize; 2], values: &[i32]) -> Vec { + let mut want: Vec = (0..24).collect(); + for r in 0..counts[0] { + for c in 0..counts[1] { + want[(starts[0] + r) * 6 + starts[1] + c] = values[r * counts[1] + c]; + } + } + want +} + +/// A selection covering exactly one whole chunk needs no read-back at all. +#[test] +fn whole_chunk_selection_replaces_that_chunk_only() { + let values: Vec = vec![-1, -2, -3, -4, -5, -6]; + for (label, build) in layouts() { + // Chunk (1,1) is rows 2..4, cols 3..6. + let got = round_trip(&format!("whole_{label}"), build, |ds| { + ds.write_slice(&[2, 3], &[2, 3], &values).unwrap() + }); + assert_eq!(got, expected([2, 3], [2, 3], &values), "index {label}"); + } +} + +/// A selection inside one chunk must leave the rest of that chunk alone — +/// the read-modify-write path. +#[test] +fn partial_chunk_selection_preserves_the_rest_of_the_chunk() { + let values: Vec = vec![-7, -8]; + for (label, build) in layouts() { + // Row 1, cols 1..3: inside chunk (0,0), touching neither its first + // row nor its first column. + let got = round_trip(&format!("partial_{label}"), build, |ds| { + ds.write_slice(&[1, 1], &[1, 2], &values).unwrap() + }); + assert_eq!(got, expected([1, 1], [1, 2], &values), "index {label}"); + } +} + +/// One row of the dataset crosses the chunk grid horizontally: partial in +/// every chunk it touches, untouched in the chunks it does not. +#[test] +fn row_update_spans_the_chunk_row_and_leaves_the_others() { + let values: Vec = vec![100, 101, 102, 103, 104, 105]; + for (label, build) in layouts() { + let got = round_trip(&format!("row_{label}"), build, |ds| { + ds.write_slice(&[2, 0], &[1, 6], &values).unwrap() + }); + assert_eq!(got, expected([2, 0], [1, 6], &values), "index {label}"); + } +} + +/// A selection straddling all four chunks, partial in each of them. +#[test] +fn selection_straddling_every_chunk_boundary() { + let values: Vec = vec![70, 71, 72, 73, 74, 75, 76, 77]; + for (label, build) in layouts() { + // Rows 1..3, cols 2..6: crosses the row boundary at 2 and the column + // boundary at 3, so all four chunks are partially covered. + let got = round_trip(&format!("straddle_{label}"), build, |ds| { + ds.write_slice(&[1, 2], &[2, 4], &values).unwrap() + }); + assert_eq!(got, expected([1, 2], [2, 4], &values), "index {label}"); + } +} + +/// Elements of a chunk that no write has ever touched read back as the +/// dataset's fill value, not as zeros left over from the buffer. +#[test] +fn untouched_elements_of_a_new_chunk_hold_the_fill_value() { + let path = tmp("fill"); + { + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([4, 6]) + .chunk(&[2, 3]) + .fill_value(-99) + .create("d") + .unwrap(); + // Only one element, in chunk (0,0). Nothing else is ever written. + ds.write_slice(&[1, 1], &[1, 1], &[42i32]).unwrap(); + file.close().unwrap(); + } + { + let file = H5File::open(&path).unwrap(); + let got = file.dataset("d").unwrap().read_raw::().unwrap(); + let mut want = vec![-99i32; 24]; + want[6 + 1] = 42; // row 1, column 1 + assert_eq!(got, want); + } + std::fs::remove_file(&path).ok(); +} + +/// The chunk grid does not divide the extent evenly: the last chunk of each +/// axis hangs past the end of the dataset. Writing into it must not disturb +/// its in-extent neighbours, and the reader must still see the right shape. +#[test] +fn edge_chunks_that_hang_past_the_extent() { + let path = tmp("edge"); + { + let file = H5File::create(&path).unwrap(); + // 5x5 in 2x2 chunks: a 3x3 grid whose last row/column of chunks is + // only half inside the dataset. + let ds = file + .new_dataset::() + .shape([5, 5]) + .chunk(&[2, 2]) + .fill_value(0) + .create("d") + .unwrap(); + let seed: Vec = (0..25).collect(); + ds.write_slice(&[0, 0], &[5, 5], &seed).unwrap(); + // Bottom-right 1x1 corner: the only in-extent element of chunk (2,2). + ds.write_slice(&[4, 4], &[1, 1], &[999i32]).unwrap(); + // Last row, all columns: crosses every chunk of the bottom grid row. + ds.write_slice(&[4, 0], &[1, 4], &[900i32, 901, 902, 903]) + .unwrap(); + file.close().unwrap(); + } + { + let file = H5File::open(&path).unwrap(); + let ds = file.dataset("d").unwrap(); + assert_eq!(ds.shape(), vec![5, 5]); + let mut want: Vec = (0..25).collect(); + want[24] = 999; + want[20..24].copy_from_slice(&[900, 901, 902, 903]); + assert_eq!(ds.read_raw::().unwrap(), want); + } + std::fs::remove_file(&path).ok(); +} + +/// Three dimensions, with the middle axis crossing a chunk boundary. +#[test] +fn three_dimensional_selection() { + let path = tmp("3d"); + { + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([2, 4, 3]) + .chunk(&[1, 2, 3]) + .create("d") + .unwrap(); + let seed: Vec = (0..24).collect(); + ds.write_slice(&[0, 0, 0], &[2, 4, 3], &seed).unwrap(); + // Plane 1, rows 1..3 (crossing the chunk boundary at 2), cols 0..2. + ds.write_slice(&[1, 1, 0], &[1, 2, 2], &[-1i32, -2, -3, -4]) + .unwrap(); + file.close().unwrap(); + } + { + let file = H5File::open(&path).unwrap(); + let got = file.dataset("d").unwrap().read_raw::().unwrap(); + let mut want: Vec = (0..24).collect(); + // (1,1,0)=15 (1,1,1)=16 (1,2,0)=18 (1,2,1)=19 + want[15] = -1; + want[16] = -2; + want[18] = -3; + want[19] = -4; + assert_eq!(got, want); + } + std::fs::remove_file(&path).ok(); +} + +/// Repeating the same partial write must converge, not accumulate: the reread +/// of an already-patched chunk has to give back exactly what was stored. +#[test] +fn repeated_partial_writes_are_idempotent() { + for (label, build) in layouts() { + let got = round_trip(&format!("idem_{label}"), build, |ds| { + for _ in 0..5 { + ds.write_slice(&[1, 2], &[2, 2], &[-1i32, -2, -3, -4]) + .unwrap(); + } + }); + assert_eq!( + got, + expected([1, 2], [2, 2], &[-1, -2, -3, -4]), + "index {label}" + ); + } +} + +/// Updating one row of a large chunked dataset must touch only the chunks +/// that row crosses — the whole point of the feature. With 1x16 chunks, a +/// one-row write to a 64x16 dataset should leave the other 63 chunks +/// unallocated, so the file stays far smaller than the full extent. +#[test] +fn a_row_update_allocates_only_the_chunks_it_touches() { + let path = tmp("cost"); + let full_extent_bytes = 64 * 16 * 4; + { + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([64, 16]) + .chunk(&[1, 16]) + .max_shape(&[None, Some(16)]) + .create("d") + .unwrap(); + ds.write_slice(&[7, 0], &[1, 16], &[5i32; 16]).unwrap(); + file.close().unwrap(); + } + let size = std::fs::metadata(&path).unwrap().len(); + assert!( + size < full_extent_bytes / 4, + "one-row update wrote {size} bytes for a {full_extent_bytes}-byte extent" + ); + { + let file = H5File::open(&path).unwrap(); + let got = file.dataset("d").unwrap().read_raw::().unwrap(); + assert_eq!(&got[7 * 16..8 * 16], &[5i32; 16]); + assert!(got[..7 * 16].iter().all(|&v| v == 0)); + assert!(got[8 * 16..].iter().all(|&v| v == 0)); + } + std::fs::remove_file(&path).ok(); +} + +/// A selection outside the current extent is still rejected, for chunked +/// storage just as for contiguous. +#[test] +fn out_of_bounds_selection_is_rejected() { + let path = tmp("oob"); + let file = H5File::create(&path).unwrap(); + let ds = fixed_array(&file); + assert!(ds.write_slice(&[3, 0], &[2, 6], &[0i32; 12]).is_err()); + assert!(ds.write_slice(&[0, 4], &[4, 4], &[0i32; 16]).is_err()); + // Wrong data length for the selection. + assert!(ds.write_slice(&[0, 0], &[2, 3], &[0i32; 5]).is_err()); + // An in-bounds selection still works. + assert!(ds.write_slice(&[2, 3], &[2, 3], &[1i32; 6]).is_ok()); + std::fs::remove_file(&path).ok(); +} diff --git a/tests/h5py_cross_validation.rs b/tests/h5py_cross_validation.rs index 73c1f1e..321dc91 100644 --- a/tests/h5py_cross_validation.rs +++ b/tests/h5py_cross_validation.rs @@ -460,3 +460,90 @@ fn b_multichunk_deflate_readable_by_h5py() { ); std::fs::remove_file(&path).ok(); } + +/// CS1: a hyperslab written into a chunked dataset must be readable by +/// libhdf5 — including the chunks the selection only partially covers and the +/// chunks it never touches, which must come back as the fill value. +#[test] +fn cs1_chunked_write_slice_readable_by_h5py() { + let Some(py) = python() else { return }; + let path = tmp("cs1_chunk_slice"); + { + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([6, 8]) + .chunk(&[2, 3]) + .fill_value(-5) + .create("grid") + .unwrap(); + // Rows 1..4 x cols 2..7: crosses both chunk-row boundaries and all + // three chunk columns, partial in every chunk it touches. + let patch: Vec = (100..115).collect(); + ds.write_slice(&[1, 2], &[3, 5], &patch).unwrap(); + // One element in the far corner chunk, which nothing else touches. + ds.write_slice(&[5, 7], &[1, 1], &[777i32]).unwrap(); + file.close().unwrap(); + } + read_back_with_h5py( + py, + &path, + "ds = f['grid']\n\ + assert ds.chunks == (2, 3), ds.chunks\n\ + assert ds.shape == (6, 8), ds.shape\n\ + want = np.full((6, 8), -5, dtype='i4')\n\ + want[1:4, 2:7] = np.arange(100, 115).reshape(3, 5)\n\ + want[5, 7] = 777\n\ + assert np.array_equal(ds[...], want), ds[...]\n", + ); + std::fs::remove_file(&path).ok(); +} + +/// CS2: patching part of a *filtered* chunk means decompressing it, editing +/// it, and recompressing. The rewritten chunk is deliberately made harder to +/// compress than the original, so it no longer fits its old file block and +/// has to be relocated — libhdf5 must still find and decode it. +#[cfg(feature = "deflate")] +#[test] +fn cs2_filtered_chunked_write_slice_readable_by_h5py() { + let Some(py) = python() else { return }; + let path = tmp("cs2_filtered_slice"); + // Incompressible patch: a scrambled sequence, not a run of small ints. + let noise: Vec = (0..4u32) + .map(|i| (i.wrapping_mul(0x9e37_79b9) ^ 0x5bd1_e995) as i32) + .collect(); + { + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([4, 6]) + .chunk(&[2, 3]) + .deflate(6) + .create("grid") + .unwrap(); + // Highly compressible seed: every chunk is a short deflate stream. + ds.write_slice(&[0, 0], &[4, 6], &[7i32; 24]).unwrap(); + // 2x2 patch straddling the column boundary at 3, so both chunks of + // the top chunk-row are read, modified and recompressed. + ds.write_slice(&[0, 2], &[2, 2], &noise).unwrap(); + file.close().unwrap(); + } + let noise_py = noise + .iter() + .map(|v| v.to_string()) + .collect::>() + .join(", "); + read_back_with_h5py( + py, + &path, + &format!( + "ds = f['grid']\n\ + assert ds.compression == 'gzip', ds.compression\n\ + assert ds.chunks == (2, 3), ds.chunks\n\ + want = np.full((4, 6), 7, dtype='i4')\n\ + want[0:2, 2:4] = np.array([{noise_py}], dtype='i4').reshape(2, 2)\n\ + assert np.array_equal(ds[...], want), ds[...]\n" + ), + ); + std::fs::remove_file(&path).ok(); +} From 84473cda2a784c50a0b4adbccac4df7ef2a23d65 Mon Sep 17 00:00:00 2001 From: Sang Woo Kim Date: Thu, 13 Aug 2026 00:15:01 +0900 Subject: [PATCH 06/20] Document chunked write_slice and the chunk-block reuse fixes CHANGELOG gains an Unreleased section covering the three changes since 0.3.2: the allocator free list, in-place chunk rewriting, and write_slice on chunked datasets. README drops the contiguous-only implication from the hyperslab feature bullet and the slice example. --- CHANGELOG.md | 35 +++++++++++++++++++++++++++++++++++ README.md | 9 ++++++++- 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 222dd69..dc61763 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,40 @@ # Changelog +## Unreleased + +### Added + +- `write_slice` now works on chunked datasets, including compressed ones + (issue #2). It was previously rejected with "write_slice is only for + contiguous datasets", so updating one row of an appendable dataset meant + rebuilding and rewriting the whole thing — O(dataset) memory and I/O for an + O(row) change. + + The selection is decomposed onto the chunk grid and only the intersecting + chunks are touched. A chunk the selection covers entirely is written + straight from the caller's buffer; a partially covered chunk is read, + patched and written back (libhdf5 makes the same distinction in + `H5D__chunk_lock`'s `relax` flag). Regions of a chunk that no write has + reached hold the dataset's fill value. All three chunk index types + (extensible array, fixed array, v2 B-tree) are supported. + +### Fixed + +- Rewriting a chunk no longer leaks its old file block. Every chunk write + previously allocated fresh space and left the previous block stranded, so + repeatedly rewriting the same chunk grew the file without bound — including + through `append`. A chunk is now placed by consulting its index entry first: + an unfiltered chunk (whose size never changes) is rewritten in place, and a + filtered chunk that no longer fits its old block is relocated with the old + block released to the allocator for reuse. This mirrors libhdf5's + `H5D__chunk_file_alloc` / `H5MF_xfree`. Under SWMR the release is suppressed, + since a reader may still be following the old address. + +- `FileAllocator` gained a free list, so released blocks are reused before the + file grows. Best fit with merging of adjacent blocks; like libhdf5's default + strategy the list is not persisted to disk, so a block released but unused at + close remains slack in the file. + ## 0.3.2 ### Fixed diff --git a/README.md b/README.md index 7cda2a8..baea546 100644 --- a/README.md +++ b/README.md @@ -22,7 +22,7 @@ Read and write HDF5 files with contiguous, chunked, and compressed datasets, hie - **Attributes** — string and numeric attributes on datasets and root - **SWMR** — Single Writer / Multiple Reader streaming protocol - **File locking** — OS-level advisory locks (`flock` / `LockFileEx`) honoring `HDF5_USE_FILE_LOCKING` -- **Hyperslab I/O** — `read_slice` / `write_slice` for partial N-dimensional access +- **Hyperslab I/O** — `read_slice` / `write_slice` for partial N-dimensional access, on contiguous and chunked (including compressed) datasets - **Buffered I/O** — BufWriter/BufReader with automatic mode switching - **Memory-mapped I/O** — optional zero-copy read-only access via `mmap` feature - **Thread safety** — optional `threadsafe` feature (`Arc` instead of `Rc`) @@ -150,6 +150,13 @@ let region = ds.read_slice::(&[2, 3], &[2, 3])?; assert_eq!(region, vec![1, 2, 3, 4, 5, 6]); ``` +`write_slice` also works on chunked datasets, including compressed ones: it +touches only the chunks the selection intersects, reading and rewriting a +chunk in place when the selection covers it only partially. Updating one row +of a large chunked dataset therefore costs one chunk's worth of I/O, not the +whole dataset's. Chunk regions no write has reached read back as the fill +value. + ### Attributes ```rust From 721d8b927e4a14266f19a1f481438ce3b657a603 Mon Sep 17 00:00:00 2001 From: Sang Woo Kim Date: Thu, 13 Aug 2026 00:20:00 +0900 Subject: [PATCH 07/20] Correct four docs that claim a contiguous-only restriction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `write_raw` and `write_raw_bytes` both dispatch to `write_full_image_chunked`, and the reader's `read_slice` matches on Contiguous, Compact and ChunkedV3, yet all three were documented as contiguous-only — as was the `layout` field that carries the chunked variants. The docs predate chunked support and now misdirect callers to rebuild a dataset they could write directly. `Hdf5Writer::write_dataset_raw` keeps its wording: it really does reject a chunked dataset with "use write_chunk for chunked datasets". --- src/dataset.rs | 12 +++++++++--- src/io/reader.rs | 8 ++++++-- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/dataset.rs b/src/dataset.rs index 2f127f4..a78144d 100644 --- a/src/dataset.rs +++ b/src/dataset.rs @@ -704,11 +704,14 @@ impl H5Dataset { AttrBuilder::new(&self.file_inner, ds_index) } - /// Write a typed slice to the dataset (contiguous datasets only). + /// Write a typed slice holding the dataset's whole image. /// /// The slice length must match the total number of elements declared by /// the dataset shape. The data is reinterpreted as raw bytes and written - /// to the file. + /// to the file: to the contiguous data block, or — for a chunked dataset — + /// scattered across its chunk grid, through the filter pipeline if one is + /// set. To write only part of a dataset, use + /// [`write_slice`](Self::write_slice). /// /// # Errors /// @@ -780,7 +783,10 @@ impl H5Dataset { } } - /// Write the raw byte image of a contiguous dataset directly. + /// Write the raw byte image of the whole dataset directly. + /// + /// Takes the same layouts as [`write_raw`](Self::write_raw): a contiguous + /// data block, or a chunk grid the image is scattered across. /// /// Unlike [`write_raw`](Self::write_raw), this is not generic over an /// `H5Type` carrier, so it works for element types that have no matching diff --git a/src/io/reader.rs b/src/io/reader.rs index 074c0e8..61a5ec2 100644 --- a/src/io/reader.rs +++ b/src/io/reader.rs @@ -105,7 +105,7 @@ pub struct DatasetReadInfo { pub datatype: DatatypeMessage, /// Dataspace (dimensionality). pub dataspace: DataspaceMessage, - /// Data layout (contiguous or compact). + /// Data layout (contiguous, compact, or chunked). pub layout: DataLayoutMessage, /// Filter pipeline for compressed chunks (None = uncompressed). pub filter_pipeline: Option, @@ -2853,7 +2853,11 @@ impl Hdf5Reader { Ok(entries) } - /// Read a slice (hyperslab) of a contiguous dataset. + /// Read a slice (hyperslab) of a dataset, whatever its layout. + /// + /// Contiguous and compact datasets are read run by run; a chunked one + /// reads only the chunks the selection overlaps, and any gap the writer + /// never filled comes back as the fill value. /// /// `starts` and `counts` define the N-dimensional selection: /// starts[d] is the first index along dim d, counts[d] is how many. From 3853c2bf49aaad3c7b45c9a3d546639a42dc221e Mon Sep 17 00:00:00 2001 From: Sang Woo Kim Date: Thu, 13 Aug 2026 00:21:33 +0900 Subject: [PATCH 08/20] Drop the unreachable filtered branch from the BT2 chunk read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `read_chunk_at_coords` handled a filtered v2-B-tree index, but nothing can build one: `write_chunk_btree_v2` stores raw bytes and records a type-10 (unfiltered) record, and the builder rejects a filter on a multi-unlimited-dimension dataset. The branch was therefore untestable code claiming a capability the write half does not have. The read now states the same invariant the writer enforces: a filtered index is an explicit error, and the unfiltered path decodes with no pipeline rather than with whatever `filter_pipeline` happens to hold — the bytes on disk are raw either way. A test pins the builder rejection that makes this hold. --- src/dataset.rs | 26 ++++++++++++++++++++++++++ src/io/writer.rs | 25 ++++++++++++------------- 2 files changed, 38 insertions(+), 13 deletions(-) diff --git a/src/dataset.rs b/src/dataset.rs index a78144d..9ea4a77 100644 --- a/src/dataset.rs +++ b/src/dataset.rs @@ -3530,6 +3530,32 @@ mod tests { std::fs::remove_file(&path).ok(); } + // A v2-B-tree index carries filtered chunks in type-11 records, but this + // writer never produces one: the builder refuses the combination up + // front. The read and write paths for that index rely on it, so pin the + // rejection rather than leaving it to the reader of the code. + #[cfg(feature = "deflate")] + #[test] + fn compression_of_a_multi_unlimited_dataset_is_rejected() { + let path = temp_path("bt2_filtered_reject"); + let file = H5File::create(&path).unwrap(); + let result = file + .new_dataset::() + .shape([4, 4]) + .chunk(&[2, 2]) + .max_shape(&[None, None]) + .deflate(6) + .create("d"); + match result { + Ok(_) => panic!("a compressed multi-unlimited dataset must not be creatable"), + Err(e) => assert!( + e.to_string().contains("not yet supported"), + "unexpected error: {e}" + ), + } + std::fs::remove_file(&path).ok(); + } + #[test] fn btree_v2_multi_unlimited_roundtrip() { // A dataset with two unlimited dimensions uses the v2 B-tree chunk diff --git a/src/io/writer.rs b/src/io/writer.rs index 898fe3d..11bbe98 100644 --- a/src/io/writer.rs +++ b/src/io/writer.rs @@ -3152,22 +3152,21 @@ impl Hdf5Writer { ChunkIndexKind::BtreeV2 => { let ds = self.ds(ds_index); let m = ds.lock(); - let pipeline = m.filter_pipeline.clone(); let bt2 = m.btree_v2.as_ref().unwrap(); - let found = if bt2.index.filtered { - bt2.index - .lookup_filtered(chunk_coords) - .map(|r| (r.chunk_address, r.chunk_size as u64, r.filter_mask)) - } else { - bt2.index - .lookup(chunk_coords) - .map(|r| (r.chunk_address, geo.chunk_bytes(), 0)) - }; + // The write half (`write_chunk_btree_v2`) stores raw bytes and + // records a type-10 record, and the builder refuses a filter on + // a v2-B-tree dataset, so this index is unfiltered by + // construction. Read it back the same way — no pipeline — + // rather than carrying a filtered branch no writer can produce. + if bt2.index.filtered { + return Err(crate::io::IoError::InvalidState( + "filtered v2 B-tree chunk index is not supported".into(), + )); + } + let found = bt2.index.lookup(chunk_coords).map(|r| r.chunk_address); drop(m); match found { - Some((addr, nbytes, mask)) => { - self.read_chunk_block(pipeline.as_ref(), addr, nbytes, mask) - } + Some(addr) => self.read_chunk_block(None, addr, geo.chunk_bytes(), 0), None => Ok(None), } } From 51cbb7860d50bfb6883a802d075f302cf47f0526 Mon Sep 17 00:00:00 2001 From: Sang Woo Kim Date: Thu, 13 Aug 2026 00:32:28 +0900 Subject: [PATCH 09/20] Keep v2 B-tree chunk records ordered by scaled offsets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit libhdf5 searches a B-tree node by bisection: H5D__bt2_compare (H5Dbtree2.c) orders records with H5VM_vector_cmp_u, a lexicographic compare of the scaled-offset vectors. Our index appended records in insertion order, so a leaf was only correctly ordered when the caller happened to write the chunk grid in ascending order. `write_chunk_at` imposes no such order. Writing a 4x6 / 2x3 grid in reverse chunk order produced a file our own reader (which scans records linearly) read back perfectly while h5py saw three of the four chunks as fill — a silent wrong-data interop bug, not an error. Records are now inserted in sorted position, so the encoded leaf is ordered by construction rather than by luck of the write order, and lookups bisect instead of scanning. A cross-check writes the grid in reverse and requires h5py to return the whole array. --- src/format/chunk_index/btree_v2.rs | 111 +++++++++++++++++++++-------- tests/h5py_cross_validation.rs | 40 +++++++++++ 2 files changed, 120 insertions(+), 31 deletions(-) diff --git a/src/format/chunk_index/btree_v2.rs b/src/format/chunk_index/btree_v2.rs index a8c628b..a697d68 100644 --- a/src/format/chunk_index/btree_v2.rs +++ b/src/format/chunk_index/btree_v2.rs @@ -644,15 +644,25 @@ impl Bt2Geometry { /// Keeps all records in memory. Serializes as a header + leaf node(s) for /// small trees. For larger trees, internal nodes would be needed but we use /// the flat approach for simplicity. +/// +/// Records are held sorted by scaled offsets and are inserted in place, so the +/// encoded leaf is always ordered. A B-tree node is searched by bisection — +/// libhdf5 compares records with `H5VM_vector_cmp_u` (`H5Dbtree2.c` +/// `H5D__bt2_compare`), which orders the scaled-offset vectors +/// lexicographically — so an unordered leaf makes libhdf5 miss chunks that are +/// present, reading them back as fill. Insertion order is *not* a safe order: +/// `write_chunk_at` lets a caller address the grid in any sequence. #[derive(Debug, Clone)] pub struct Bt2ChunkIndex { /// Number of dataset dimensions. pub ndims: usize, /// Whether chunks are filtered. pub filtered: bool, - /// Unfiltered chunk records (used when filtered == false). + /// Unfiltered chunk records (used when filtered == false), sorted by + /// scaled offsets. pub records: Vec, - /// Filtered chunk records (used when filtered == true). + /// Filtered chunk records (used when filtered == true), sorted by scaled + /// offsets. pub filtered_records: Vec, } @@ -677,24 +687,24 @@ impl Bt2ChunkIndex { } } - /// Insert an unfiltered chunk record. + /// Insert an unfiltered chunk record, keeping the records sorted. pub fn insert(&mut self, scaled_offsets: Vec, chunk_address: u64) { - // Check if a record with the same coordinates already exists - if let Some(existing) = self + match self .records - .iter_mut() - .find(|r| r.scaled_offsets == scaled_offsets) + .binary_search_by(|r| r.scaled_offsets.as_slice().cmp(&scaled_offsets)) { - existing.chunk_address = chunk_address; - } else { - self.records.push(Bt2ChunkRecord { - scaled_offsets, - chunk_address, - }); + Ok(i) => self.records[i].chunk_address = chunk_address, + Err(i) => self.records.insert( + i, + Bt2ChunkRecord { + scaled_offsets, + chunk_address, + }, + ), } } - /// Insert a filtered chunk record. + /// Insert a filtered chunk record, keeping the records sorted. pub fn insert_filtered( &mut self, scaled_offsets: Vec, @@ -702,36 +712,42 @@ impl Bt2ChunkIndex { chunk_size: u32, filter_mask: u32, ) { - if let Some(existing) = self + match self .filtered_records - .iter_mut() - .find(|r| r.scaled_offsets == scaled_offsets) + .binary_search_by(|r| r.scaled_offsets.as_slice().cmp(&scaled_offsets)) { - existing.chunk_address = chunk_address; - existing.chunk_size = chunk_size; - existing.filter_mask = filter_mask; - } else { - self.filtered_records.push(Bt2FilteredChunkRecord { - scaled_offsets, - chunk_address, - chunk_size, - filter_mask, - }); + Ok(i) => { + let rec = &mut self.filtered_records[i]; + rec.chunk_address = chunk_address; + rec.chunk_size = chunk_size; + rec.filter_mask = filter_mask; + } + Err(i) => self.filtered_records.insert( + i, + Bt2FilteredChunkRecord { + scaled_offsets, + chunk_address, + chunk_size, + filter_mask, + }, + ), } } /// Look up a chunk by its scaled coordinates. Returns the record if found. pub fn lookup(&self, scaled_offsets: &[u64]) -> Option<&Bt2ChunkRecord> { self.records - .iter() - .find(|r| r.scaled_offsets == scaled_offsets) + .binary_search_by(|r| r.scaled_offsets.as_slice().cmp(scaled_offsets)) + .ok() + .map(|i| &self.records[i]) } /// Look up a filtered chunk by its scaled coordinates. pub fn lookup_filtered(&self, scaled_offsets: &[u64]) -> Option<&Bt2FilteredChunkRecord> { self.filtered_records - .iter() - .find(|r| r.scaled_offsets == scaled_offsets) + .binary_search_by(|r| r.scaled_offsets.as_slice().cmp(scaled_offsets)) + .ok() + .map(|i| &self.filtered_records[i]) } /// Iterate all unfiltered records. @@ -1188,6 +1204,39 @@ mod tests { assert!(idx.lookup(&[2, 2]).is_none()); } + // libhdf5 bisects a B-tree node, so the encoded leaf must be ordered by + // scaled offsets no matter what order the caller writes chunks in. + #[test] + fn records_are_ordered_however_they_are_inserted() { + let mut idx = Bt2ChunkIndex::new_unfiltered(2); + for coords in [[1, 1], [0, 2], [1, 0], [0, 0], [0, 1]] { + idx.insert(coords.to_vec(), 0x1000); + } + let order: Vec> = idx.iter().map(|r| r.scaled_offsets.clone()).collect(); + assert_eq!( + order, + vec![vec![0, 0], vec![0, 1], vec![0, 2], vec![1, 0], vec![1, 1]] + ); + // Every record is still reachable after the reordering. + for coords in [[1, 1], [0, 2], [1, 0], [0, 0], [0, 1]] { + assert!(idx.lookup(&coords).is_some(), "lost {coords:?}"); + } + } + + #[test] + fn filtered_records_are_ordered_however_they_are_inserted() { + let mut idx = Bt2ChunkIndex::new_filtered(2); + for (i, coords) in [[2, 0], [0, 1], [1, 3], [0, 0]].iter().enumerate() { + idx.insert_filtered(coords.to_vec(), 0x1000 + i as u64, 7, 0); + } + let order: Vec> = idx + .iter_filtered() + .map(|r| r.scaled_offsets.clone()) + .collect(); + assert_eq!(order, vec![vec![0, 0], vec![0, 1], vec![1, 3], vec![2, 0]]); + assert_eq!(idx.lookup_filtered(&[1, 3]).unwrap().chunk_address, 0x1002); + } + #[test] fn chunk_index_insert_replaces() { let mut idx = Bt2ChunkIndex::new_unfiltered(2); diff --git a/tests/h5py_cross_validation.rs b/tests/h5py_cross_validation.rs index 321dc91..e1a0a84 100644 --- a/tests/h5py_cross_validation.rs +++ b/tests/h5py_cross_validation.rs @@ -547,3 +547,43 @@ fn cs2_filtered_chunked_write_slice_readable_by_h5py() { ); std::fs::remove_file(&path).ok(); } + +/// BT2-1: a v2-B-tree index is searched by bisection, so its leaf records must +/// be ordered by scaled offsets. Write the chunk grid in reverse order — the +/// order a caller is free to use — and require libhdf5 to still find every +/// chunk. With records left in insertion order, h5py reads back mostly fill. +#[test] +fn bt2_1_out_of_order_chunk_writes_readable_by_h5py() { + let Some(py) = python() else { return }; + let path = tmp("bt2_order"); + let chunk = |vals: [i32; 6]| -> Vec { vals.iter().flat_map(|v| v.to_le_bytes()).collect() }; + { + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([4, 6]) + .chunk(&[2, 3]) + .max_shape(&[None, None]) + .create("grid") + .unwrap(); + // Reverse grid order: (1,1), (1,0), (0,1), (0,0). + ds.write_chunk_at(&[1, 1], &chunk([15, 16, 17, 21, 22, 23])) + .unwrap(); + ds.write_chunk_at(&[1, 0], &chunk([12, 13, 14, 18, 19, 20])) + .unwrap(); + ds.write_chunk_at(&[0, 1], &chunk([3, 4, 5, 9, 10, 11])) + .unwrap(); + ds.write_chunk_at(&[0, 0], &chunk([0, 1, 2, 6, 7, 8])) + .unwrap(); + file.close().unwrap(); + } + read_back_with_h5py( + py, + &path, + "ds = f['grid']\n\ + assert ds.maxshape == (None, None), ds.maxshape\n\ + assert ds.chunks == (2, 3), ds.chunks\n\ + assert np.array_equal(ds[...], np.arange(24).reshape(4, 6)), ds[...]\n", + ); + std::fs::remove_file(&path).ok(); +} From d86f0edbf9d7b7ec427332a95672f595c1d8c1fc Mon Sep 17 00:00:00 2001 From: Sang Woo Kim Date: Thu, 13 Aug 2026 00:41:28 +0900 Subject: [PATCH 10/20] Support compression on multi-unlimited-dimension datasets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A dataset with two or more unlimited dimensions uses a v2 B-tree chunk index, and the builder refused to attach a filter to one: "compression of v2 B-tree (multi-unlimited-dimension) datasets is not yet supported". libhdf5 has no such restriction — H5Dchunk.c handles a filter pipeline in the >1-unlimited branch, and H5Dbtree2.c switches the index to H5D_BT2_FILT, whose type-11 records carry the stored size and filter mask. h5py creates one with maxshape=(None, None) plus compression. The format layer already read and wrote type-11 records; what was missing was the writer: - create_btree_v2_dataset_with_pipeline builds a filtered index and a type-11 header/leaf, and stores the pipeline on the dataset. - write_chunk_btree_v2 now runs the pipeline (outside the slot lock), places the compressed bytes through place_chunk keyed on the record's stored size, and records the new size via insert_filtered. - read_chunk_at_coords reads a filtered record's size and mask back, so a partial write_slice can decompress, patch and recompress. - The builder passes the pipeline instead of erroring; the pipeline it hands to the fixed-array and B-tree paths is now built in one place. The record's compressed-size field width is not stored in the file: libhdf5 recomputes it (H5D_BT2_COMPUTE_CHUNK_SIZE_LEN) as sizeof_size for layout version 5 and otherwise from the chunk's magnitude. We emit version-4 layout messages, so the index sizes that field with compute_chunk_size_len, the same helper the extensible and fixed arrays use — for a 2x4 i32 chunk that is 2 bytes, not the 8 the header constructor previously assumed while the index assumed 8 in a different place (32 vs 36 byte records). Both now derive it from one value. Decoding is unchanged and stays width-agnostic, deriving the width from the header's record_size, so libhdf5's version-5 files still read. --- src/dataset.rs | 114 ++++++++++++------- src/format/chunk_index/btree_v2.rs | 66 ++++++++--- src/io/writer.rs | 177 ++++++++++++++++++++++------- tests/chunked_write_slice.rs | 24 +++- tests/h5py_cross_validation.rs | 44 +++++++ 5 files changed, 323 insertions(+), 102 deletions(-) diff --git a/src/dataset.rs b/src/dataset.rs index 9ea4a77..3692be5 100644 --- a/src/dataset.rs +++ b/src/dataset.rs @@ -277,17 +277,43 @@ impl DatasetBuilder { let inner = borrow_inner(&self.file_inner); match &*inner { H5FileInner::Writer(writer) => { + // The requested filter pipeline, if any. Both index + // types that take one explicitly (fixed array and v2 + // B-tree) build it the same way, so resolve it once. + let explicit_pipeline = || { + if let Some(p) = self.custom_pipeline.clone() { + p + } else if let Some(level) = self.shuffle_deflate_level { + crate::format::messages::filter::FilterPipeline::shuffle_deflate( + T::element_size() as u32, + level, + ) + } else { + // deflate_level (checked by wants_filter). + crate::format::messages::filter::FilterPipeline::deflate( + self.deflate_level.unwrap(), + ) + } + }; let idx = if is_btree2 { + // Two or more unlimited dimensions: a v2 B-tree, + // whose records carry the stored size and filter + // mask when the dataset is compressed (libhdf5 + // H5D_BT2_FILT). if wants_filter { - return Err(Hdf5Error::InvalidState( - "compression of v2 B-tree (multi-unlimited-dimension) \ - datasets is not yet supported" - .into(), - )); + writer.create_btree_v2_dataset_with_pipeline( + &full_name, + datatype, + &dims_u64, + &max_u64, + &chunk_u64, + explicit_pipeline(), + )? + } else { + writer.create_btree_v2_dataset( + &full_name, datatype, &dims_u64, &max_u64, &chunk_u64, + )? } - writer.create_btree_v2_dataset( - &full_name, datatype, &dims_u64, &max_u64, &chunk_u64, - )? } else if is_fixed_array { // A chunked dataset with no unlimited dimension // must use the fixed-array index — libhdf5 @@ -295,21 +321,12 @@ impl DatasetBuilder { // compressed fixed-shape dataset uses a *filtered* // fixed array (FA client id 1). if wants_filter { - let pipeline = if let Some(p) = self.custom_pipeline { - p - } else if let Some(level) = self.shuffle_deflate_level { - crate::format::messages::filter::FilterPipeline::shuffle_deflate( - T::element_size() as u32, - level, - ) - } else { - // deflate_level (checked by wants_filter). - crate::format::messages::filter::FilterPipeline::deflate( - self.deflate_level.unwrap(), - ) - }; writer.create_fixed_array_dataset_with_pipeline( - &full_name, datatype, &dims_u64, &chunk_u64, pipeline, + &full_name, + datatype, + &dims_u64, + &chunk_u64, + explicit_pipeline(), )? } else { writer.create_fixed_array_dataset( @@ -3530,28 +3547,41 @@ mod tests { std::fs::remove_file(&path).ok(); } - // A v2-B-tree index carries filtered chunks in type-11 records, but this - // writer never produces one: the builder refuses the combination up - // front. The read and write paths for that index rely on it, so pin the - // rejection rather than leaving it to the reader of the code. + // Two or more unlimited dimensions select the v2 B-tree index; with a + // filter its records become type 11, carrying each chunk's stored size and + // mask. The payload is highly compressible, so the chunks really are + // stored smaller than the extent — the file would be at least + // 6*8*4 = 192 bytes of raw chunk data otherwise. #[cfg(feature = "deflate")] #[test] - fn compression_of_a_multi_unlimited_dataset_is_rejected() { - let path = temp_path("bt2_filtered_reject"); - let file = H5File::create(&path).unwrap(); - let result = file - .new_dataset::() - .shape([4, 4]) - .chunk(&[2, 2]) - .max_shape(&[None, None]) - .deflate(6) - .create("d"); - match result { - Ok(_) => panic!("a compressed multi-unlimited dataset must not be creatable"), - Err(e) => assert!( - e.to_string().contains("not yet supported"), - "unexpected error: {e}" - ), + fn compressed_multi_unlimited_dataset_roundtrips() { + let path = temp_path("bt2_filtered"); + { + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([6, 8]) + .chunk(&[2, 4]) + .max_shape(&[None, None]) + .deflate(6) + .create("d") + .unwrap(); + ds.write_slice(&[0, 0], &[6, 8], &[7i32; 48]).unwrap(); + // A partial write forces a decompress-patch-recompress of one + // chunk, whose new compressed size may not fit its old block. + ds.write_slice(&[1, 1], &[2, 2], &[1i32, 2, 3, 4]).unwrap(); + file.close().unwrap(); + } + { + let file = H5File::open(&path).unwrap(); + let ds = file.dataset("d").unwrap(); + assert_eq!(ds.shape(), vec![6, 8]); + let mut want = vec![7i32; 48]; + want[9] = 1; + want[10] = 2; + want[17] = 3; + want[18] = 4; + assert_eq!(ds.read_raw::().unwrap(), want); } std::fs::remove_file(&path).ok(); } diff --git a/src/format/chunk_index/btree_v2.rs b/src/format/chunk_index/btree_v2.rs index a697d68..ef18440 100644 --- a/src/format/chunk_index/btree_v2.rs +++ b/src/format/chunk_index/btree_v2.rs @@ -32,8 +32,19 @@ pub const BT2_TYPE_CHUNK_UNFILT: u8 = 10; pub const BT2_TYPE_CHUNK_FILT: u8 = 11; /// Bytes used to encode a filtered chunk's compressed size in a v2 B-tree -/// record (libhdf5 layout version 5 uses `sizeof_size`). -pub const BT2_FILT_CHUNK_SIZE_LEN: usize = 8; +/// record. +/// +/// libhdf5 does not read this width off the record; it recomputes it +/// (`H5D_BT2_COMPUTE_CHUNK_SIZE_LEN`, `H5Dbtree2.c`) from the layout message +/// version: `sizeof_size` for version 5, and otherwise the same +/// magnitude-derived width the extensible and fixed arrays use. Our writer +/// emits version-4 layout messages, so a file it writes must use the latter — +/// hence [`compute_chunk_size_len`], shared with those two indexes. +/// +/// Decoding stays width-agnostic: [`Bt2ChunkIndex::decode_filtered_records`] +/// derives the width from the header's `record_size`, so a version-5 file +/// written by libhdf5 reads back just as well. +pub use super::extensible_array::compute_chunk_size_len; /// A chunk record for BT2 type 10 (unfiltered). /// @@ -116,10 +127,12 @@ impl Bt2Header { /// Create a new B-tree v2 header for filtered chunk indexing. /// - /// `ndims` is the number of dataset dimensions. - pub fn new_for_filtered_chunks(ctx: &FormatContext, ndims: usize) -> Self { - // record_size = ndims * 8 + sizeof_addr + 4 (chunk_size) + 4 (filter_mask) - let record_size = (ndims * 8 + ctx.sizeof_addr as usize + 4 + 4) as u16; + /// `ndims` is the number of dataset dimensions and `chunk_size_len` the + /// width of the compressed-size field (see [`compute_chunk_size_len`]). + pub fn new_for_filtered_chunks(ctx: &FormatContext, ndims: usize, chunk_size_len: u8) -> Self { + // record_size = address + chunk_size + filter_mask(4) + scaled offsets + let record_size = + (ctx.sizeof_addr as usize + chunk_size_len as usize + 4 + ndims * 8) as u16; Self { record_type: BT2_TYPE_CHUNK_FILT, node_size: 4096, @@ -664,6 +677,9 @@ pub struct Bt2ChunkIndex { /// Filtered chunk records (used when filtered == true), sorted by scaled /// offsets. pub filtered_records: Vec, + /// Width in bytes of a filtered record's compressed-size field. Meaningful + /// only when `filtered`; see [`compute_chunk_size_len`]. + pub chunk_size_len: u8, } impl Bt2ChunkIndex { @@ -674,16 +690,21 @@ impl Bt2ChunkIndex { filtered: false, records: Vec::new(), filtered_records: Vec::new(), + chunk_size_len: 0, } } /// Create a new empty B-tree v2 chunk index for filtered chunks. - pub fn new_filtered(ndims: usize) -> Self { + /// + /// `chunk_size_len` must be the width libhdf5 will recompute for this + /// dataset — [`compute_chunk_size_len`] of the uncompressed chunk size. + pub fn new_filtered(ndims: usize, chunk_size_len: u8) -> Self { Self { ndims, filtered: true, records: Vec::new(), filtered_records: Vec::new(), + chunk_size_len, } } @@ -771,13 +792,13 @@ impl Bt2ChunkIndex { /// Compute the record size in bytes. /// - /// Filtered records encode the compressed size in a fixed - /// [`BT2_FILT_CHUNK_SIZE_LEN`]-byte field, matching libhdf5's layout - /// version 5. + /// A filtered record adds the compressed size — in the + /// [`chunk_size_len`](Self::chunk_size_len)-byte field libhdf5 will + /// recompute — and a 4-byte filter mask. pub fn record_size(&self, ctx: &FormatContext) -> u16 { let sa = ctx.sizeof_addr as usize; if self.filtered { - (sa + BT2_FILT_CHUNK_SIZE_LEN + 4 + self.ndims * 8) as u16 + (sa + self.chunk_size_len as usize + 4 + self.ndims * 8) as u16 } else { (self.ndims * 8 + sa) as u16 } @@ -796,7 +817,7 @@ impl Bt2ChunkIndex { // filter mask, then the scaled offsets. buf.extend_from_slice(&rec.chunk_address.to_le_bytes()[..sa]); buf.extend_from_slice( - &(rec.chunk_size as u64).to_le_bytes()[..BT2_FILT_CHUNK_SIZE_LEN], + &(rec.chunk_size as u64).to_le_bytes()[..self.chunk_size_len as usize], ); buf.extend_from_slice(&rec.filter_mask.to_le_bytes()); for &offset in &rec.scaled_offsets { @@ -1072,10 +1093,21 @@ mod tests { #[test] fn header_filtered_roundtrip() { - let hdr = Bt2Header::new_for_filtered_chunks(&ctx8(), 2); + let hdr = Bt2Header::new_for_filtered_chunks(&ctx8(), 2, 8); assert_eq!(hdr.record_type, BT2_TYPE_CHUNK_FILT); - // record_size = 2*8 + 8 + 4 + 4 = 32 - assert_eq!(hdr.record_size, 32); + // address(8) + chunk_size(8) + filter_mask(4) + 2 offsets(16) = 36, + // the same rule Bt2ChunkIndex::record_size applies. + assert_eq!(hdr.record_size, 36); + assert_eq!( + hdr.record_size, + Bt2ChunkIndex::new_filtered(2, 8).record_size(&ctx8()), + "header and index must agree on the record size" + ); + // A narrower size field shrinks the record by exactly that much. + assert_eq!( + Bt2Header::new_for_filtered_chunks(&ctx8(), 2, 2).record_size, + 30 + ); let encoded = hdr.encode(&ctx8()); let decoded = Bt2Header::decode(&encoded, &ctx8()).unwrap(); @@ -1225,7 +1257,7 @@ mod tests { #[test] fn filtered_records_are_ordered_however_they_are_inserted() { - let mut idx = Bt2ChunkIndex::new_filtered(2); + let mut idx = Bt2ChunkIndex::new_filtered(2, 8); for (i, coords) in [[2, 0], [0, 1], [1, 3], [0, 0]].iter().enumerate() { idx.insert_filtered(coords.to_vec(), 0x1000 + i as u64, 7, 0); } @@ -1306,7 +1338,7 @@ mod tests { #[test] fn filtered_chunk_index_encode_decode_roundtrip() { let ctx = ctx8(); - let mut idx = Bt2ChunkIndex::new_filtered(2); + let mut idx = Bt2ChunkIndex::new_filtered(2, 8); idx.insert_filtered(vec![0, 0], 0x1000, 512, 0); idx.insert_filtered(vec![1, 0], 0x2000, 300, 1); diff --git a/src/io/writer.rs b/src/io/writer.rs index 11bbe98..0902e18 100644 --- a/src/io/writer.rs +++ b/src/io/writer.rs @@ -3152,21 +3152,25 @@ impl Hdf5Writer { ChunkIndexKind::BtreeV2 => { let ds = self.ds(ds_index); let m = ds.lock(); + let pipeline = m.filter_pipeline.clone(); let bt2 = m.btree_v2.as_ref().unwrap(); - // The write half (`write_chunk_btree_v2`) stores raw bytes and - // records a type-10 record, and the builder refuses a filter on - // a v2-B-tree dataset, so this index is unfiltered by - // construction. Read it back the same way — no pipeline — - // rather than carrying a filtered branch no writer can produce. - if bt2.index.filtered { - return Err(crate::io::IoError::InvalidState( - "filtered v2 B-tree chunk index is not supported".into(), - )); - } - let found = bt2.index.lookup(chunk_coords).map(|r| r.chunk_address); + // A filtered index records the stored size and mask per chunk; + // an unfiltered one stores whole chunks, so their size is the + // chunk shape and no filter ran. + let found = if bt2.index.filtered { + bt2.index + .lookup_filtered(chunk_coords) + .map(|r| (r.chunk_address, r.chunk_size as u64, r.filter_mask)) + } else { + bt2.index + .lookup(chunk_coords) + .map(|r| (r.chunk_address, geo.chunk_bytes(), 0)) + }; drop(m); match found { - Some(addr) => self.read_chunk_block(None, addr, geo.chunk_bytes(), 0), + Some((addr, nbytes, mask)) => { + self.read_chunk_block(pipeline.as_ref(), addr, nbytes, mask) + } None => Ok(None), } } @@ -3437,25 +3441,89 @@ impl Hdf5Writer { max_dims: &[u64], chunk_dims: &[u64], ) -> IoResult { + self.create_btree_v2_dataset_inner(name, datatype, dims, max_dims, chunk_dims, None) + } + + /// Define a *filtered* chunked dataset indexed by a B-tree v2. + /// + /// The v2 B-tree counterpart of + /// [`create_chunked_dataset_with_pipeline`](Self::create_chunked_dataset_with_pipeline): + /// chunks are compressed on write and the index records each chunk's + /// stored size and filter mask (record type 11), the same shape libhdf5 + /// builds when a multi-unlimited-dimension dataset has a filter pipeline + /// (`H5Dbtree2.c`, `H5D_BT2_FILT`). + pub fn create_btree_v2_dataset_with_pipeline( + &self, + name: &str, + datatype: DatatypeMessage, + dims: &[u64], + max_dims: &[u64], + chunk_dims: &[u64], + pipeline: FilterPipeline, + ) -> IoResult { + self.create_btree_v2_dataset_inner( + name, + datatype, + dims, + max_dims, + chunk_dims, + Some(pipeline), + ) + } + + fn create_btree_v2_dataset_inner( + &self, + name: &str, + datatype: DatatypeMessage, + dims: &[u64], + max_dims: &[u64], + chunk_dims: &[u64], + pipeline: Option, + ) -> IoResult { + use crate::format::chunk_index::btree_v2::{ + compute_chunk_size_len, Bt2Header, Bt2LeafNode, BT2_TYPE_CHUNK_FILT, + BT2_TYPE_CHUNK_UNFILT, + }; + // Hold the create gate across the uniqueness check and the registry // push so the two are atomic (see `create_lock`). let _create = self.create_lock.lock(); self.ensure_unique_dataset_name(name)?; let ndims = dims.len(); - let bt2_index = Bt2ChunkIndex::new_unfiltered(ndims); + if chunk_dims.len() != ndims { + return Err(crate::io::IoError::InvalidState(format!( + "chunk shape has {} dimensions but the dataspace has {}", + chunk_dims.len(), + ndims + ))); + } + + // The filtered record's size field is as wide as libhdf5 will + // recompute it from the uncompressed chunk size, exactly as the + // extensible- and fixed-array filtered paths size theirs. + let (bt2_index, record_type) = match pipeline { + Some(_) => { + let chunk_bytes: u64 = + chunk_dims.iter().product::() * datatype.element_size() as u64; + let len = compute_chunk_size_len(chunk_bytes); + (Bt2ChunkIndex::new_filtered(ndims, len), BT2_TYPE_CHUNK_FILT) + } + None => (Bt2ChunkIndex::new_unfiltered(ndims), BT2_TYPE_CHUNK_UNFILT), + }; - // We'll allocate space for header and leaf node; they'll be written - // during flush_dataset_bt2. - let hdr = crate::format::chunk_index::btree_v2::Bt2Header::new_for_chunks(&self.ctx, ndims); + // We'll allocate space for header and leaf node; they'll be rewritten + // from the in-memory index during flush_dataset. + let hdr = if bt2_index.filtered { + Bt2Header::new_for_filtered_chunks(&self.ctx, ndims, bt2_index.chunk_size_len) + } else { + Bt2Header::new_for_chunks(&self.ctx, ndims) + }; let hdr_encoded = hdr.encode(&self.ctx); let bt2_header_addr = self.allocator.allocate(hdr_encoded.len() as u64); self.handle.write_at(bt2_header_addr, &hdr_encoded)?; // Allocate a placeholder leaf node (empty for now) - let leaf = crate::format::chunk_index::btree_v2::Bt2LeafNode::new( - crate::format::chunk_index::btree_v2::BT2_TYPE_CHUNK_UNFILT, - bt2_index.record_size(&self.ctx), - ); + let leaf = Bt2LeafNode::new(record_type, bt2_index.record_size(&self.ctx)); let leaf_encoded = leaf.encode(); let bt2_leaf_addr = self.allocator.allocate(leaf_encoded.len() as u64); self.handle.write_at(bt2_leaf_addr, &leaf_encoded)?; @@ -3475,7 +3543,7 @@ impl Hdf5Writer { attributes: Vec::new(), obj_header_written_addr: None, obj_header_encoded_size: 0, - filter_pipeline: None, + filter_pipeline: pipeline, deleted: false, fill_value: None, chunked: None, @@ -3901,22 +3969,28 @@ impl Hdf5Writer { /// Write a chunk to a B-tree v2 indexed dataset. /// /// `chunk_coords` is the scaled chunk coordinates (one per dimension). + /// `data` is the chunk's unfiltered bytes; if the dataset has a filter + /// pipeline it runs here and the index records the stored size and mask. pub fn write_chunk_btree_v2( &self, index: usize, chunk_coords: &[u64], data: &[u8], ) -> IoResult<()> { - // Hold one slot guard for the whole method; `self.allocator`/`self.handle` - // below touch disjoint fields safe to use with the guard held. + // Read what the write needs under a brief guard, then compress OUTSIDE + // the lock — filtering a chunk must not hold the dataset slot. let ds = self.ds(index); - let mut m = ds.lock(); - let element_size = m.datatype.element_size() as u64; - let bt2 = m - .btree_v2 - .as_ref() - .ok_or_else(|| crate::io::IoError::InvalidState("not a B-tree v2 dataset".into()))?; - let chunk_bytes: u64 = bt2.chunk_dims.iter().product::() * element_size; + let (chunk_bytes, pipeline) = { + let m = ds.lock(); + let element_size = m.datatype.element_size() as u64; + let bt2 = m.btree_v2.as_ref().ok_or_else(|| { + crate::io::IoError::InvalidState("not a B-tree v2 dataset".into()) + })?; + ( + bt2.chunk_dims.iter().product::() * element_size, + m.filter_pipeline.clone(), + ) + }; if data.len() as u64 != chunk_bytes { return Err(crate::io::IoError::InvalidState(format!( @@ -3926,16 +4000,43 @@ impl Hdf5Writer { ))); } - // Place the bytes: a rewrite of an already-recorded chunk stays where - // it is, since an unfiltered chunk's stored size is fixed by the chunk - // shape (see `place_chunk`). - let old = bt2.index.lookup(chunk_coords).map(|r| r.chunk_address); - let chunk_addr = self.place_chunk(old.map(|a| (a, chunk_bytes)), chunk_bytes); - self.handle.write_at(chunk_addr, data)?; + let filtered; + let stored = match pipeline { + Some(ref pl) => { + filtered = filter::apply_filters(pl, data)?; + &filtered[..] + } + None => data, + }; + let stored_len = stored.len() as u64; + + let mut m = ds.lock(); + let bt2 = m.btree_v2.as_ref().unwrap(); + // Place the bytes: a rewrite whose stored size is unchanged stays + // where it is (always so when unfiltered — the size is fixed by the + // chunk shape), and one that no longer fits moves, releasing its old + // block. See `place_chunk`. + let old = if bt2.index.filtered { + bt2.index + .lookup_filtered(chunk_coords) + .map(|r| (r.chunk_address, r.chunk_size as u64)) + } else { + bt2.index + .lookup(chunk_coords) + .map(|r| (r.chunk_address, chunk_bytes)) + }; + let chunk_addr = self.place_chunk(old, stored_len); + self.handle.write_at(chunk_addr, stored)?; - // Insert into the in-memory BT2 index + // Insert into the in-memory BT2 index. filter_mask = 0: the whole + // pipeline ran (or the dataset is unfiltered), so no filter is skipped. let bt2 = m.btree_v2.as_mut().unwrap(); - bt2.index.insert(chunk_coords.to_vec(), chunk_addr); + if bt2.index.filtered { + bt2.index + .insert_filtered(chunk_coords.to_vec(), chunk_addr, stored_len as u32, 0); + } else { + bt2.index.insert(chunk_coords.to_vec(), chunk_addr); + } bt2.chunks_written += 1; Ok(()) diff --git a/tests/chunked_write_slice.rs b/tests/chunked_write_slice.rs index c37d7d8..709e843 100644 --- a/tests/chunked_write_slice.rs +++ b/tests/chunked_write_slice.rs @@ -82,6 +82,20 @@ fn deflated(file: &H5File) -> H5Dataset { .unwrap() } +/// Compressed *and* v2-B-tree indexed: the filtered records carry each chunk's +/// stored size, so a partial write that recompresses to a different size has +/// to update the index, not just the bytes. +#[cfg(feature = "deflate")] +fn deflated_btree_v2(file: &H5File) -> H5Dataset { + file.new_dataset::() + .shape([4, 6]) + .chunk(&[2, 3]) + .max_shape(&[None, None]) + .deflate(6) + .create("d") + .unwrap() +} + /// A storage layout to run a case against: a label for assertion messages and /// the constructor that produces a dataset with that layout. type Layout = (&'static str, fn(&H5File) -> H5Dataset); @@ -95,18 +109,18 @@ fn layouts() -> Vec { ("ea", extensible_array), ("bt2", btree_v2), ]; - cases.extend(filtered_layout()); + cases.extend(filtered_layouts()); cases } #[cfg(feature = "deflate")] -fn filtered_layout() -> Option { - Some(("deflate", deflated)) +fn filtered_layouts() -> Vec { + vec![("deflate", deflated), ("deflate-bt2", deflated_btree_v2)] } #[cfg(not(feature = "deflate"))] -fn filtered_layout() -> Option { - None +fn filtered_layouts() -> Vec { + Vec::new() } /// Expected contents of the seeded 4x6 grid after writing `values` into the diff --git a/tests/h5py_cross_validation.rs b/tests/h5py_cross_validation.rs index e1a0a84..4b437cc 100644 --- a/tests/h5py_cross_validation.rs +++ b/tests/h5py_cross_validation.rs @@ -587,3 +587,47 @@ fn bt2_1_out_of_order_chunk_writes_readable_by_h5py() { ); std::fs::remove_file(&path).ok(); } + +/// BT2-2: a compressed dataset with two unlimited dimensions — a v2 B-tree +/// index whose records are type 11, carrying each chunk's stored size and +/// filter mask. libhdf5 recomputes that size field's width from the layout +/// version rather than reading it off the record, so this also pins that our +/// version-4 layout and our record width agree with what it expects. +#[cfg(feature = "deflate")] +#[test] +fn bt2_2_compressed_multi_unlimited_readable_by_h5py() { + let Some(py) = python() else { return }; + let path = tmp("bt2_filtered"); + { + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([6, 8]) + .chunk(&[2, 4]) + .max_shape(&[None, None]) + .deflate(6) + .create("grid") + .unwrap(); + ds.write_slice(&[0, 0], &[6, 8], &[7i32; 48]).unwrap(); + // Patching part of one chunk recompresses it to a different size, so + // the chunk moves and the index must record the new size and address. + ds.write_slice(&[1, 1], &[2, 2], &[1i32, 2, 3, 4]).unwrap(); + file.close().unwrap(); + } + read_back_with_h5py( + py, + &path, + "ds = f['grid']\n\ + assert ds.compression == 'gzip', ds.compression\n\ + assert ds.maxshape == (None, None), ds.maxshape\n\ + assert ds.chunks == (2, 4), ds.chunks\n\ + want = np.full((6, 8), 7, dtype='i4')\n\ + want[1, 1] = 1; want[1, 2] = 2; want[2, 1] = 3; want[2, 2] = 4\n\ + assert np.array_equal(ds[...], want), ds[...]\n\ + # Every chunk must be stored compressed, i.e. smaller than 2*4*4 = 32 B.\n\ + sizes = [ds.id.get_chunk_info(i).size for i in range(ds.id.get_num_chunks())]\n\ + assert len(sizes) == 6, sizes\n\ + assert all(s < 32 for s in sizes), sizes\n", + ); + std::fs::remove_file(&path).ok(); +} From c5a3134e2c1282b15831720f8933c9c20dac423b Mon Sep 17 00:00:00 2001 From: Sang Woo Kim Date: Thu, 13 Aug 2026 00:42:02 +0900 Subject: [PATCH 11/20] Document filtered v2-B-tree support and the record-ordering fix CHANGELOG gains the compression-on-multi-unlimited-dimension entry plus the two interop fixes it uncovered (record ordering, size-field width). README notes that filters apply to every chunk index, not just the two the builder previously accepted them for. --- CHANGELOG.md | 25 +++++++++++++++++++++++++ README.md | 4 ++++ 2 files changed, 29 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index dc61763..24ac633 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,8 +18,33 @@ reached hold the dataset's fill value. All three chunk index types (extensible array, fixed array, v2 B-tree) are supported. +- Compression now works on datasets with two or more unlimited dimensions, + which use a v2 B-tree chunk index. The combination was previously rejected + with "compression of v2 B-tree (multi-unlimited-dimension) datasets is not + yet supported", although libhdf5 supports it (`H5D_BT2_FILT` in + `H5Dbtree2.c`) and h5py produces one from `maxshape=(None, None)` plus + `compression=`. The index now writes type-11 records carrying each chunk's + stored size and filter mask, so a partial `write_slice` can decompress, + patch and recompress a chunk and relocate it when its size changes. + ### Fixed +- A v2 B-tree chunk index is now written with its records ordered by scaled + offsets. libhdf5 searches a B-tree node by bisection (`H5D__bt2_compare` + orders records with `H5VM_vector_cmp_u`), and records were appended in + insertion order, so a file was only correct when the caller happened to + write the chunk grid in ascending order. Writing it in any other order — + which `write_chunk_at` permits — produced a file this library read back + perfectly while libhdf5 and h5py saw the out-of-order chunks as fill: wrong + data, with no error. Records are now inserted in sorted position and + lookups bisect. + +- The width of a filtered v2-B-tree record's compressed-size field is now + derived from the chunk size, matching what libhdf5 recomputes for a + version-4 layout message (`H5D_BT2_COMPUTE_CHUNK_SIZE_LEN`). The header + constructor and the index previously disagreed about it (a 32- versus + 36-byte record); both now take it from one value. + - Rewriting a chunk no longer leaks its old file block. Every chunk write previously allocated fresh space and left the previous block stranded, so repeatedly rewriting the same chunk grew the file without bound — including diff --git a/README.md b/README.md index baea546..5b9a0ee 100644 --- a/README.md +++ b/README.md @@ -271,6 +271,10 @@ for ensuring no second writer attaches during streaming. rust-hdf5 = { version = "0.2", features = ["lz4", "zstd"] } ``` +Filters apply to every chunked layout, whichever chunk index the dataspace +selects: a fixed array (no unlimited dimension), an extensible array (exactly +one), or a v2 B-tree (two or more). + ## Feature flags | Feature | Description | From 418baa34d4625a2f1879c8656a02bd68ebe02ca2 Mon Sep 17 00:00:00 2001 From: Sang Woo Kim Date: Thu, 13 Aug 2026 01:00:12 +0900 Subject: [PATCH 12/20] Give the v2 B-tree fixed-size nodes and a real tree shape MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Bt2ChunkIndex::encode` derived `node_size` from the current record count and always emitted one depth-0 leaf. That single decision caused two defects: - the leaf grew with every chunk, so each flush had to allocate a bigger block and left the previous one unreachable — the same unconditional-allocate leak fixed for chunk data in b7fd938, this time for index blocks; - one node had to hold every record, but a node's record count is a u16, so a dataset past 65535 chunks truncated it silently. Both close by giving nodes the geometry libhdf5 uses. `BT2_NODE_SIZE` (2048, `H5D_BT2_NODE_SIZE`) now fixes every node's size, matching what the layout message already declared, and `Bt2Tree::build` bulk-loads the ordered records into as many levels as they need: a level's records spread evenly over its nodes with one record promoted between adjacent siblings, which is the shape `H5B2__locate_record` descends. Node capacities come from `Bt2Geometry`, so no node exceeds what a reader deserializes, and only the header's u64 total counts every record. The writer keeps the node blocks in an append-only pool per dataset (`Bt2DatasetInfo::node_addrs`), the single owner of those addresses. A flush re-serializes the whole tree over the pool and allocates only the shortfall, so a block is never orphaned and the addresses a reader holds stay valid. An empty index gets no nodes at all and an undefined root, the state libhdf5 leaves a freshly created B-tree in. Creating a BT2 index now rejects a rank whose record is too wide for a node to hold three records; HDF5's rank limit of 32 leaves room for seven. --- src/dataset.rs | 41 ++ src/format/chunk_index/btree_v2.rs | 583 +++++++++++++++++++++++++---- src/format/messages/data_layout.rs | 15 +- src/io/writer.rs | 82 ++-- tests/h5py_cross_validation.rs | 86 +++++ 5 files changed, 698 insertions(+), 109 deletions(-) diff --git a/src/dataset.rs b/src/dataset.rs index 3692be5..c490144 100644 --- a/src/dataset.rs +++ b/src/dataset.rs @@ -2892,6 +2892,47 @@ mod tests { assert_eq!(last, 8); } + // A flush re-serializes the whole v2 B-tree over the dataset's node-block + // pool. Every node is the same size, so the blocks already on disk are + // reused and repeated flushes cost nothing; sizing the root to its record + // count instead would relocate it each time and orphan the block it left. + #[test] + fn repeated_flushes_do_not_grow_a_btree_v2_index() { + let flush_n = |label: &str, flushes: usize| -> u64 { + let path = temp_path(label); + { + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([2, 4]) + .chunk(&[2, 4]) + .max_shape(&[None, None]) + .create("d") + .unwrap(); + let bytes: Vec = (0..8i32).flat_map(|v| v.to_le_bytes()).collect(); + ds.write_chunk_at(&[0, 0], &bytes).unwrap(); + for _ in 0..flushes { + ds.flush().unwrap(); + } + file.close().unwrap(); + } + let size = std::fs::metadata(&path).unwrap().len(); + // The data must survive every rewrite of the index. + { + let file = H5File::open(&path).unwrap(); + let ds = file.dataset("d").unwrap(); + assert_eq!(ds.read_raw::().unwrap(), (0..8).collect::>()); + } + std::fs::remove_file(&path).ok(); + size + }; + assert_eq!( + flush_n("bt2_flush_8", 8), + flush_n("bt2_flush_1", 1), + "8 index flushes grew the file past a single one" + ); + } + // A filtered chunk whose compressed size changes cannot stay put, so it // moves and releases its old block (libhdf5 H5D__chunk_file_alloc calls // H5MF_xfree). Alternating between two payloads of different compressed diff --git a/src/format/chunk_index/btree_v2.rs b/src/format/chunk_index/btree_v2.rs index ef18440..7f59206 100644 --- a/src/format/chunk_index/btree_v2.rs +++ b/src/format/chunk_index/btree_v2.rs @@ -26,6 +26,23 @@ pub const BTLF_SIGNATURE: [u8; 4] = *b"BTLF"; /// B-tree v2 version. pub const BT2_VERSION: u8 = 0; +/// Node size for a chunk-index v2 B-tree, matching libhdf5's +/// `H5D_BT2_NODE_SIZE` (`H5Dpkg.h`). +/// +/// Every node — leaf or internal — occupies exactly this many bytes on disk. +/// That is what makes a node block reusable: re-serializing a tree overwrites +/// its nodes in place rather than relocating them, so a flush cannot orphan the +/// block it replaced. libhdf5 reads a whole node-size block and checksums only +/// the used prefix, so the tail is padding that must nevertheless exist in the +/// file. +pub const BT2_NODE_SIZE: u32 = 2048; + +/// Percentage full a node must be before it splits (`H5D_BT2_SPLIT_PERC`). +pub const BT2_SPLIT_PERCENT: u8 = 100; + +/// Percentage below which a node is merged (`H5D_BT2_MERGE_PERC`). +pub const BT2_MERGE_PERCENT: u8 = 40; + /// Record type: unfiltered chunks (non-filtered chunked datasets). pub const BT2_TYPE_CHUNK_UNFILT: u8 = 10; /// Record type: filtered chunks (filtered chunked datasets). @@ -114,11 +131,11 @@ impl Bt2Header { let record_size = (ndims * 8 + ctx.sizeof_addr as usize) as u16; Self { record_type: BT2_TYPE_CHUNK_UNFILT, - node_size: 4096, + node_size: BT2_NODE_SIZE, record_size, depth: 0, - split_percent: 100, - merge_percent: 40, + split_percent: BT2_SPLIT_PERCENT, + merge_percent: BT2_MERGE_PERCENT, root_node_addr: UNDEF_ADDR, num_records_in_root: 0, total_num_records: 0, @@ -135,11 +152,11 @@ impl Bt2Header { (ctx.sizeof_addr as usize + chunk_size_len as usize + 4 + ndims * 8) as u16; Self { record_type: BT2_TYPE_CHUNK_FILT, - node_size: 4096, + node_size: BT2_NODE_SIZE, record_size, depth: 0, - split_percent: 100, - merge_percent: 40, + split_percent: BT2_SPLIT_PERCENT, + merge_percent: BT2_MERGE_PERCENT, root_node_addr: UNDEF_ADDR, num_records_in_root: 0, total_num_records: 0, @@ -648,15 +665,241 @@ impl Bt2Geometry { } } +// ========================================================================== +// Bulk-loaded tree +// ========================================================================== + +/// One node of a bulk-loaded v2 B-tree. +#[derive(Debug, Clone, PartialEq)] +pub struct Bt2TreeNode { + /// Height above the leaves; 0 for a leaf. + pub depth: u16, + /// The records this node holds directly, already encoded. + pub record_data: Vec, + /// How many records that is. + pub num_records: u16, + /// Indices into [`Bt2Tree::nodes`] of this node's children. A node holding + /// *m* records has *m + 1* children; a leaf has none. + pub children: Vec, + /// Records in this node and every node beneath it. + pub total_records: u64, +} + +/// A v2 B-tree bulk-loaded from an ordered record list. +/// +/// Nodes are laid out children-before-parents, so [`nodes`](Self::nodes)'s last +/// entry is the root and every node's children have smaller indices — which is +/// what lets [`encode`](Self::encode) resolve child addresses in one pass. +/// +/// Every node is exactly [`node_size`](Self::node_size) bytes, so re-loading a +/// grown index overwrites the nodes already on disk and only ever *appends* +/// blocks. Sizing the root to its contents instead (one leaf that grows with +/// the record count) would force a relocation on every flush, orphaning the +/// block it replaced, and would cap the index at the 65535 records a node's +/// record count can express. +#[derive(Debug, Clone)] +pub struct Bt2Tree { + /// Every node, children before parents; the last entry is the root. + pub nodes: Vec, + /// Record type (10 = unfiltered chunks, 11 = filtered chunks). + pub record_type: u8, + /// Size of one record in bytes. + pub record_size: u16, + /// Size of every node in bytes. + pub node_size: u32, + /// Node geometry for depths `0..=depth()`. + pub geometry: Bt2Geometry, +} + +impl Bt2Tree { + /// Bulk-load a tree from `records` — the encoded records in key order. + /// + /// Each level's records are spread evenly across that level's nodes, with + /// one record promoted to the parent between adjacent siblings. That is the + /// shape libhdf5 searches: `H5B2__locate_record` bisects a node's records + /// and, on a miss, descends into the child the record would fall between. + /// + /// Node capacities come from [`Bt2Geometry`], so no node exceeds what a + /// reader computing the same geometry will deserialize. + pub fn build( + record_type: u8, + record_size: u16, + node_size: u32, + sizeof_addr: u8, + records: &[u8], + ) -> Self { + let rec = record_size.max(1) as usize; + let total = records.len() / rec; + let mut nodes: Vec = Vec::new(); + // Records still to be placed at the current level: the chunk records + // at depth 0, and the separators promoted from below at each depth + // above it. + let mut pending: Vec = records[..total * rec].to_vec(); + // Nodes of the level below, in key order. + let mut children: Vec = Vec::new(); + let mut depth: u16 = 0; + + // An empty index has no nodes at all: the header's root address stays + // undefined, the state libhdf5 leaves a freshly created B-tree in. + if total > 0 { + loop { + let geo = Bt2Geometry::new(node_size, record_size, depth, sizeof_addr); + let cap = geo.node_info[depth as usize].max_nrec.max(1) as usize; + let n = pending.len() / rec; + // Fewest nodes that hold n records with one separator between + // each adjacent pair: k * cap + (k - 1) >= n. + let k = (n + 1).div_ceil(cap + 1); + let own = n - (k - 1); + let (base, extra) = (own / k, own % k); + debug_assert!(base >= 1, "a bulk-loaded node must hold a record"); + debug_assert!(base + usize::from(extra > 0) <= cap); + + let mut next_pending: Vec = Vec::new(); + let mut next_children: Vec = Vec::with_capacity(k); + let (mut r, mut c) = (0usize, 0usize); + for i in 0..k { + let m = base + usize::from(i < extra); + let record_data = pending[r * rec..(r + m) * rec].to_vec(); + r += m; + let kids: Vec = if depth == 0 { + Vec::new() + } else { + let kids = children[c..c + m + 1].to_vec(); + c += m + 1; + kids + }; + let total_records = + m as u64 + kids.iter().map(|&j| nodes[j].total_records).sum::(); + nodes.push(Bt2TreeNode { + depth, + record_data, + num_records: m as u16, + children: kids, + total_records, + }); + next_children.push(nodes.len() - 1); + // The record between this node and the next is the + // separator their parent holds. + if i + 1 < k { + next_pending.extend_from_slice(&pending[r * rec..(r + 1) * rec]); + r += 1; + } + } + if k == 1 { + break; + } + pending = next_pending; + children = next_children; + depth += 1; + } + } + + Self { + geometry: Bt2Geometry::new(node_size, record_size, depth, sizeof_addr), + nodes, + record_type, + record_size, + node_size, + } + } + + /// Depth of the root (0 = the root is a leaf). + pub fn depth(&self) -> u16 { + self.nodes.last().map_or(0, |n| n.depth) + } + + /// Records held directly by the root. + pub fn root_num_records(&self) -> u16 { + self.nodes.last().map_or(0, |n| n.num_records) + } + + /// Records in the whole tree. + pub fn total_records(&self) -> u64 { + self.nodes.last().map_or(0, |n| n.total_records) + } + + /// Serialize every node to a [`node_size`](Self::node_size)-byte image, in + /// [`nodes`](Self::nodes) order. + /// + /// `addrs[i]` is the file address assigned to `nodes[i]`; entries past the + /// node count are ignored, so a caller may pass a longer block pool. + pub fn encode(&self, ctx: &FormatContext, addrs: &[u64]) -> Vec> { + self.nodes + .iter() + .map(|n| { + let mut image = if n.depth == 0 { + Bt2LeafNode { + record_type: self.record_type, + record_data: n.record_data.clone(), + num_records: n.num_records, + record_size: self.record_size, + } + .encode() + } else { + Bt2InternalNode { + record_type: self.record_type, + record_data: n.record_data.clone(), + num_records: n.num_records, + record_size: self.record_size, + child_addrs: n.children.iter().map(|&c| addrs[c]).collect(), + child_nrecords: n + .children + .iter() + .map(|&c| self.nodes[c].num_records) + .collect(), + child_total_nrecords: n + .children + .iter() + .map(|&c| self.nodes[c].total_records) + .collect(), + } + .encode( + ctx, + n.depth, + self.geometry.max_nrec_size, + self.geometry.child_total_size(n.depth), + ) + }; + debug_assert!(image.len() <= self.node_size as usize); + // A reader reads the whole block, so the padding has to be + // there even though only the prefix is checksummed. + image.resize(self.node_size as usize, 0); + image + }) + .collect() + } + + /// The header describing this tree. `root_addr` is the address given to the + /// last node, and is ignored for an empty tree (whose root is undefined, + /// the state libhdf5 leaves a freshly created B-tree in). + pub fn header(&self, root_addr: u64) -> Bt2Header { + Bt2Header { + record_type: self.record_type, + node_size: self.node_size, + record_size: self.record_size, + depth: self.depth(), + split_percent: BT2_SPLIT_PERCENT, + merge_percent: BT2_MERGE_PERCENT, + root_node_addr: if self.nodes.is_empty() { + UNDEF_ADDR + } else { + root_addr + }, + num_records_in_root: self.root_num_records(), + total_num_records: self.total_records(), + } + } +} + // ========================================================================== // In-memory BT2 chunk index (flat approach) // ========================================================================== /// In-memory B-tree v2 chunk index. /// -/// Keeps all records in memory. Serializes as a header + leaf node(s) for -/// small trees. For larger trees, internal nodes would be needed but we use -/// the flat approach for simplicity. +/// Keeps every record in memory as one ordered list and bulk-loads it into a +/// tree of fixed-size nodes on demand — see [`build_tree`](Self::build_tree) +/// and [`Bt2Tree`]. /// /// Records are held sorted by scaled offsets and are inserted in place, so the /// encoded leaf is always ordered. A B-tree node is searched by bisection — @@ -837,47 +1080,28 @@ impl Bt2ChunkIndex { buf } - /// Encode the B-tree as a header + leaf node. - /// - /// Returns `(header_bytes, leaf_bytes)`. - pub fn encode(&self, ctx: &FormatContext) -> (Vec, Vec) { - let rec_size = self.record_size(ctx); - let num = self.num_records() as u16; - - let record_data = self.encode_records(ctx); - - let leaf = Bt2LeafNode { - record_type: if self.filtered { - BT2_TYPE_CHUNK_FILT - } else { - BT2_TYPE_CHUNK_UNFILT - }, - record_data, - num_records: num, - record_size: rec_size, - }; - let leaf_encoded = leaf.encode(); - - // We'll set root_node_addr to UNDEF_ADDR; the caller sets it to the - // actual leaf address after allocating. - let header = Bt2Header { - record_type: if self.filtered { - BT2_TYPE_CHUNK_FILT - } else { - BT2_TYPE_CHUNK_UNFILT - }, - node_size: leaf_encoded.len() as u32, - record_size: rec_size, - depth: 0, - split_percent: 100, - merge_percent: 40, - root_node_addr: UNDEF_ADDR, - num_records_in_root: num, - total_num_records: num as u64, - }; - let header_encoded = header.encode(ctx); + /// The record type these records serialize as. + pub fn record_type(&self) -> u8 { + if self.filtered { + BT2_TYPE_CHUNK_FILT + } else { + BT2_TYPE_CHUNK_UNFILT + } + } - (header_encoded, leaf_encoded) + /// Bulk-load these records into a v2 B-tree of [`BT2_NODE_SIZE`]-byte + /// nodes. + /// + /// The records are already in key order (see the type docs), which is + /// exactly what [`Bt2Tree::build`] needs. + pub fn build_tree(&self, ctx: &FormatContext) -> Bt2Tree { + Bt2Tree::build( + self.record_type(), + self.record_size(ctx), + BT2_NODE_SIZE, + ctx.sizeof_addr, + &self.encode_records(ctx), + ) } /// Decode unfiltered records from a leaf node's raw record data. @@ -1296,10 +1520,7 @@ mod tests { idx.insert(vec![0, 1], 0x2000); idx.insert(vec![1, 0], 0x3000); - let (hdr_bytes, leaf_bytes) = idx.encode(&ctx); - - // Decode header - let hdr = Bt2Header::decode(&hdr_bytes, &ctx).unwrap(); + let (hdr, record_bytes) = serialize_and_walk(&idx, &ctx); assert_eq!(hdr.record_type, BT2_TYPE_CHUNK_UNFILT); assert_eq!(hdr.depth, 0); assert_eq!(hdr.total_num_records, 3); @@ -1307,10 +1528,7 @@ mod tests { // record_size = 2*8 + 8 = 24 assert_eq!(hdr.record_size, 24); - // Decode leaf - let leaf = Bt2LeafNode::decode(&leaf_bytes, 3, hdr.record_size).unwrap(); - let records = - Bt2ChunkIndex::decode_unfiltered_records(&leaf.record_data, 3, 2, &ctx).unwrap(); + let records = Bt2ChunkIndex::decode_unfiltered_records(&record_bytes, 3, 2, &ctx).unwrap(); assert_eq!(records.len(), 3); assert_eq!(records[0].scaled_offsets, vec![0, 0]); @@ -1342,18 +1560,15 @@ mod tests { idx.insert_filtered(vec![0, 0], 0x1000, 512, 0); idx.insert_filtered(vec![1, 0], 0x2000, 300, 1); - let (hdr_bytes, leaf_bytes) = idx.encode(&ctx); - - let hdr = Bt2Header::decode(&hdr_bytes, &ctx).unwrap(); + let (hdr, record_bytes) = serialize_and_walk(&idx, &ctx); assert_eq!(hdr.record_type, BT2_TYPE_CHUNK_FILT); assert_eq!(hdr.total_num_records, 2); // record_size = sizeof_addr(8) + chunk_size_len(8) + filter_mask(4) // + ndims*8(16) = 36 assert_eq!(hdr.record_size, 36); - let leaf = Bt2LeafNode::decode(&leaf_bytes, 2, hdr.record_size).unwrap(); let records = - Bt2ChunkIndex::decode_filtered_records(&leaf.record_data, 2, 2, hdr.record_size, &ctx) + Bt2ChunkIndex::decode_filtered_records(&record_bytes, 2, 2, hdr.record_size, &ctx) .unwrap(); assert_eq!(records.len(), 2); @@ -1372,14 +1587,11 @@ mod tests { idx.insert(vec![0], 0x100); idx.insert(vec![1], 0x200); - let (hdr_bytes, leaf_bytes) = idx.encode(&ctx); - let hdr = Bt2Header::decode(&hdr_bytes, &ctx).unwrap(); + let (hdr, record_bytes) = serialize_and_walk(&idx, &ctx); // record_size = 1*8 + 4 = 12 assert_eq!(hdr.record_size, 12); - let leaf = Bt2LeafNode::decode(&leaf_bytes, 2, hdr.record_size).unwrap(); - let records = - Bt2ChunkIndex::decode_unfiltered_records(&leaf.record_data, 2, 1, &ctx).unwrap(); + let records = Bt2ChunkIndex::decode_unfiltered_records(&record_bytes, 2, 1, &ctx).unwrap(); assert_eq!(records[0].chunk_address, 0x100); assert_eq!(records[1].chunk_address, 0x200); } @@ -1390,11 +1602,240 @@ mod tests { let idx = Bt2ChunkIndex::new_unfiltered(3); assert_eq!(idx.num_records(), 0); - let (hdr_bytes, leaf_bytes) = idx.encode(&ctx); - let hdr = Bt2Header::decode(&hdr_bytes, &ctx).unwrap(); + // No records means no nodes at all: the header names an undefined root, + // the state libhdf5 leaves a freshly created B-tree in. + let tree = idx.build_tree(&ctx); + assert!(tree.nodes.is_empty()); + let (hdr, record_bytes) = serialize_and_walk(&idx, &ctx); assert_eq!(hdr.total_num_records, 0); + assert_eq!(hdr.root_node_addr, UNDEF_ADDR); + assert!(record_bytes.is_empty()); + } + + // ---- Bulk-loaded tree tests ---- + + /// Serialize an index the way the writer's flush does — a distinct block + /// per node — then walk the result the way a reader does, in key order: + /// child 0, record 0, child 1, record 1, ..., child m. Returns the header + /// and the records the walk recovered, so a caller can compare them + /// against the flat ordered list the index holds. + fn serialize_and_walk(idx: &Bt2ChunkIndex, ctx: &FormatContext) -> (Bt2Header, Vec) { + let tree = idx.build_tree(ctx); + let addrs: Vec = (0..tree.nodes.len()) + .map(|i| 0x1000 + i as u64 * tree.node_size as u64) + .collect(); + let blocks: Vec<(u64, Vec)> = addrs + .iter() + .copied() + .zip(tree.encode(ctx, &addrs)) + .collect(); + for (_, image) in &blocks { + assert_eq!( + image.len(), + tree.node_size as usize, + "every node occupies a full block" + ); + } + let hdr = tree.header(addrs.last().copied().unwrap_or(UNDEF_ADDR)); + + let mut out = Vec::new(); + if hdr.root_node_addr != UNDEF_ADDR { + let geo = Bt2Geometry::new(hdr.node_size, hdr.record_size, hdr.depth, ctx.sizeof_addr); + walk_node( + &blocks, + hdr.root_node_addr, + hdr.depth, + hdr.num_records_in_root, + &hdr, + &geo, + ctx, + &mut out, + ); + } + (hdr, out) + } + + #[allow(clippy::too_many_arguments)] + fn walk_node( + blocks: &[(u64, Vec)], + addr: u64, + depth: u16, + nrec: u16, + hdr: &Bt2Header, + geo: &Bt2Geometry, + ctx: &FormatContext, + out: &mut Vec, + ) { + let buf = &blocks + .iter() + .find(|(a, _)| *a == addr) + .unwrap_or_else(|| panic!("no node at {addr:#x}")) + .1; + let rec = hdr.record_size as usize; + assert!( + 10 + nrec as usize * rec <= hdr.node_size as usize, + "node at depth {depth} holds {nrec} records, more than its block fits" + ); + if depth == 0 { + let leaf = Bt2LeafNode::decode(buf, nrec, hdr.record_size).unwrap(); + out.extend_from_slice(&leaf.record_data); + return; + } + let node = Bt2InternalNode::decode( + buf, + ctx, + depth, + nrec, + hdr.record_size, + geo.max_nrec_size, + geo.child_total_size(depth), + ) + .unwrap(); + for i in 0..=nrec as usize { + walk_node( + blocks, + node.child_addrs[i], + depth - 1, + node.child_nrecords[i], + hdr, + geo, + ctx, + out, + ); + if i < nrec as usize { + out.extend_from_slice(&node.record_data[i * rec..(i + 1) * rec]); + } + } + } + + /// Build an unfiltered 2-D index with `n` records at (i, 0). + fn index_with(n: u64) -> Bt2ChunkIndex { + let mut idx = Bt2ChunkIndex::new_unfiltered(2); + for i in 0..n { + idx.insert(vec![i, 0], 0x10_000 + i * 0x100); + } + idx + } - let leaf = Bt2LeafNode::decode(&leaf_bytes, 0, hdr.record_size).unwrap(); - assert!(leaf.record_data.is_empty()); + #[test] + fn a_tree_that_fits_one_node_stays_a_single_leaf() { + let ctx = ctx8(); + // record_size 24, node 2048: a leaf holds (2048 - 10) / 24 = 84. + let idx = index_with(84); + let tree = idx.build_tree(&ctx); + assert_eq!(tree.nodes.len(), 1); + assert_eq!(tree.depth(), 0); + assert_eq!(tree.total_records(), 84); + + let (hdr, walked) = serialize_and_walk(&idx, &ctx); + assert_eq!(hdr.depth, 0); + assert_eq!(walked, idx.encode_records(&ctx)); + } + + #[test] + fn one_record_past_a_leaf_grows_the_tree_a_level() { + let ctx = ctx8(); + let idx = index_with(85); + let tree = idx.build_tree(&ctx); + assert_eq!(tree.depth(), 1, "85 records no longer fit one leaf"); + assert_eq!(tree.total_records(), 85); + // Root separator plus two leaves. + assert_eq!(tree.nodes.len(), 3); + + let (hdr, walked) = serialize_and_walk(&idx, &ctx); + assert_eq!(hdr.depth, 1); + assert_eq!(hdr.total_num_records, 85); + assert_eq!( + walked, + idx.encode_records(&ctx), + "an in-order walk must recover every record in key order" + ); + } + + #[test] + fn a_tree_grows_a_second_level() { + let ctx = ctx8(); + // Depth 1 tops out at 61 root records over 62 leaves of 84: + // 61 + 62 * 84 = 5269. + let idx = index_with(5270); + let tree = idx.build_tree(&ctx); + assert_eq!(tree.depth(), 2); + assert_eq!(tree.total_records(), 5270); + + let (hdr, walked) = serialize_and_walk(&idx, &ctx); + assert_eq!(hdr.depth, 2); + assert_eq!(hdr.total_num_records, 5270); + assert_eq!(walked, idx.encode_records(&ctx)); + } + + /// The record count a node reports is a u16; a tree that kept every record + /// in one node would silently truncate past 65535. Splitting keeps every + /// node small, so the only count that has to be wide is the header's + /// `total_num_records`. + #[test] + fn a_tree_far_past_the_node_record_count_limit_stays_intact() { + let ctx = ctx8(); + let idx = index_with(70_000); + let tree = idx.build_tree(&ctx); + assert_eq!(tree.total_records(), 70_000); + assert!(tree.depth() >= 2); + for node in &tree.nodes { + let cap = tree.geometry.node_info[node.depth as usize].max_nrec; + assert!( + u64::from(node.num_records) <= cap && node.num_records > 0, + "node at depth {} holds {} records (cap {cap})", + node.depth, + node.num_records + ); + assert_eq!( + node.children.len(), + if node.depth == 0 { + 0 + } else { + node.num_records as usize + 1 + } + ); + } + let (hdr, walked) = serialize_and_walk(&idx, &ctx); + assert_eq!(hdr.total_num_records, 70_000); + assert_eq!(walked, idx.encode_records(&ctx)); + } + + #[test] + fn filtered_records_survive_a_split_tree() { + let ctx = ctx8(); + // record_size = 8 + 2 + 4 + 16 = 30, so a leaf holds (2048-10)/30 = 67. + let mut idx = Bt2ChunkIndex::new_filtered(2, 2); + for i in 0..500u64 { + idx.insert_filtered(vec![i, 0], 0x10_000 + i * 0x100, (i % 400) as u32 + 1, 0); + } + let tree = idx.build_tree(&ctx); + assert!(tree.depth() >= 1); + + let (hdr, walked) = serialize_and_walk(&idx, &ctx); + assert_eq!(hdr.record_size, 30); + assert_eq!(hdr.total_num_records, 500); + let records = + Bt2ChunkIndex::decode_filtered_records(&walked, 500, 2, hdr.record_size, &ctx).unwrap(); + for (i, r) in records.iter().enumerate() { + assert_eq!(r.scaled_offsets, vec![i as u64, 0]); + assert_eq!(r.chunk_address, 0x10_000 + i as u64 * 0x100); + assert_eq!(r.chunk_size, (i as u32 % 400) + 1); + } + } + + /// Growing the index must not change the addresses already handed out — + /// that is what lets the writer overwrite node blocks instead of + /// relocating (and orphaning) them. + #[test] + fn every_node_occupies_the_same_size_block_at_any_depth() { + let ctx = ctx8(); + for n in [1u64, 84, 85, 1000, 5270] { + let tree = index_with(n).build_tree(&ctx); + let addrs: Vec = (0..tree.nodes.len() as u64).collect(); + for image in tree.encode(&ctx, &addrs) { + assert_eq!(image.len(), BT2_NODE_SIZE as usize, "n = {n}"); + } + } } } diff --git a/src/format/messages/data_layout.rs b/src/format/messages/data_layout.rs index f6e44ca..643fb97 100644 --- a/src/format/messages/data_layout.rs +++ b/src/format/messages/data_layout.rs @@ -355,13 +355,14 @@ impl DataLayoutMessage { } } ChunkIndexType::BTreeV2 => { - // node_size(4) + split_percent(1) + merge_percent(1). - // The v2 B-tree header carries the authoritative - // copies; readers consult those, so a valid default - // here suffices. - buf.extend_from_slice(&2048u32.to_le_bytes()); - buf.push(100); - buf.push(40); + // node_size(4) + split_percent(1) + merge_percent(1), + // the same geometry the B-tree header carries. + use crate::format::chunk_index::btree_v2::{ + BT2_MERGE_PERCENT, BT2_NODE_SIZE, BT2_SPLIT_PERCENT, + }; + buf.extend_from_slice(&BT2_NODE_SIZE.to_le_bytes()); + buf.push(BT2_SPLIT_PERCENT); + buf.push(BT2_MERGE_PERCENT); } // A filtered single chunk carries its on-disk size // (sizeof_size bytes) and 4-byte filter mask inline, before diff --git a/src/io/writer.rs b/src/io/writer.rs index 0902e18..fbe22c5 100644 --- a/src/io/writer.rs +++ b/src/io/writer.rs @@ -409,8 +409,14 @@ pub struct Bt2DatasetInfo { pub max_dims: Vec, /// File offset of the BT2 header. pub bt2_header_addr: u64, - /// File offset of the BT2 leaf node. - pub bt2_leaf_addr: u64, + /// Pool of `BT2_NODE_SIZE`-byte blocks holding the tree's nodes, in the + /// order [`Bt2Tree::encode`] emits them. + /// + /// Append-only and the single owner of the tree's node addresses: a flush + /// re-serializes the whole tree over these blocks and allocates only the + /// shortfall, so no flush can orphan a block it replaced. Every node is the + /// same size, so a block stays usable however the tree reshapes. + pub node_addrs: Vec, /// In-memory chunk index. pub index: Bt2ChunkIndex, /// Number of chunks written so far. @@ -3481,8 +3487,7 @@ impl Hdf5Writer { pipeline: Option, ) -> IoResult { use crate::format::chunk_index::btree_v2::{ - compute_chunk_size_len, Bt2Header, Bt2LeafNode, BT2_TYPE_CHUNK_FILT, - BT2_TYPE_CHUNK_UNFILT, + compute_chunk_size_len, Bt2Header, BT2_NODE_SIZE, }; // Hold the create gate across the uniqueness check and the registry @@ -3501,18 +3506,31 @@ impl Hdf5Writer { // The filtered record's size field is as wide as libhdf5 will // recompute it from the uncompressed chunk size, exactly as the // extensible- and fixed-array filtered paths size theirs. - let (bt2_index, record_type) = match pipeline { + let bt2_index = match pipeline { Some(_) => { let chunk_bytes: u64 = chunk_dims.iter().product::() * datatype.element_size() as u64; let len = compute_chunk_size_len(chunk_bytes); - (Bt2ChunkIndex::new_filtered(ndims, len), BT2_TYPE_CHUNK_FILT) + Bt2ChunkIndex::new_filtered(ndims, len) } - None => (Bt2ChunkIndex::new_unfiltered(ndims), BT2_TYPE_CHUNK_UNFILT), + None => Bt2ChunkIndex::new_unfiltered(ndims), }; - // We'll allocate space for header and leaf node; they'll be rewritten - // from the in-memory index during flush_dataset. + // The bulk loader spreads a level's records evenly over its nodes, one + // separator between adjacent siblings, which needs room for a few + // records per node. HDF5's rank limit of 32 leaves room for seven; a + // wider rank than that has no valid geometry, so reject it here rather + // than emit a tree no reader can walk. + let record_size = bt2_index.record_size(&self.ctx) as usize; + if (BT2_NODE_SIZE as usize) < 10 + 3 * record_size { + return Err(crate::io::IoError::InvalidState(format!( + "a {ndims}-dimension v2 B-tree record is {record_size} bytes, too wide \ + for a {BT2_NODE_SIZE}-byte node" + ))); + } + + // Only the header gets a home now: it names an empty tree, whose root + // is undefined until the first flush bulk-loads the index into nodes. let hdr = if bt2_index.filtered { Bt2Header::new_for_filtered_chunks(&self.ctx, ndims, bt2_index.chunk_size_len) } else { @@ -3522,12 +3540,6 @@ impl Hdf5Writer { let bt2_header_addr = self.allocator.allocate(hdr_encoded.len() as u64); self.handle.write_at(bt2_header_addr, &hdr_encoded)?; - // Allocate a placeholder leaf node (empty for now) - let leaf = Bt2LeafNode::new(record_type, bt2_index.record_size(&self.ctx)); - let leaf_encoded = leaf.encode(); - let bt2_leaf_addr = self.allocator.allocate(leaf_encoded.len() as u64); - self.handle.write_at(bt2_leaf_addr, &leaf_encoded)?; - let dataspace = DataspaceMessage { dims: dims.to_vec(), max_dims: Some(max_dims.to_vec()), @@ -3552,7 +3564,7 @@ impl Hdf5Writer { chunk_dims: chunk_dims.to_vec(), max_dims: max_dims.to_vec(), bt2_header_addr, - bt2_leaf_addr, + node_addrs: Vec::new(), index: bt2_index, chunks_written: 0, }), @@ -4279,23 +4291,31 @@ impl Hdf5Writer { // BT2-indexed dataset if let Some(ref bt2) = m.btree_v2 { - // Re-encode the leaf node and header - let (hdr_bytes, leaf_bytes) = bt2.index.encode(&self.ctx); - - // The leaf may have grown -- reallocate if needed - let leaf_addr = self.allocator.allocate(leaf_bytes.len() as u64); - self.handle.write_at(leaf_addr, &leaf_bytes)?; - - // Update header with new root node address - let mut hdr = - crate::format::chunk_index::btree_v2::Bt2Header::decode(&hdr_bytes, &self.ctx)?; - hdr.root_node_addr = leaf_addr; - let hdr_encoded = hdr.encode(&self.ctx); + // Bulk-load the index into fixed-size nodes and lay them over the + // dataset's block pool. Because every node is the same size, the + // blocks already on disk are reused in place and only the shortfall + // is allocated — the pool is the single owner of these addresses, + // so no flush leaves a block behind. The addresses a reader already + // holds stay valid, which is also what SWMR needs. + let tree = bt2.index.build_tree(&self.ctx); + let mut node_addrs = bt2.node_addrs.clone(); + while node_addrs.len() < tree.nodes.len() { + node_addrs.push(self.allocator.allocate(tree.node_size as u64)); + } + + for (image, &addr) in tree.encode(&self.ctx, &node_addrs).iter().zip(&node_addrs) { + self.handle.write_at(addr, image)?; + } + + // The root is the last node the bulk load emits. + let root_addr = match tree.nodes.len() { + 0 => UNDEF_ADDR, + n => node_addrs[n - 1], + }; + let hdr_encoded = tree.header(root_addr).encode(&self.ctx); self.handle.write_at(bt2.bt2_header_addr, &hdr_encoded)?; - // Update our in-memory copy's leaf addr - let bt2_mut = m.btree_v2.as_mut().unwrap(); - bt2_mut.bt2_leaf_addr = leaf_addr; + m.btree_v2.as_mut().unwrap().node_addrs = node_addrs; if sync { self.handle.sync_data()?; diff --git a/tests/h5py_cross_validation.rs b/tests/h5py_cross_validation.rs index 4b437cc..de0e99c 100644 --- a/tests/h5py_cross_validation.rs +++ b/tests/h5py_cross_validation.rs @@ -631,3 +631,89 @@ fn bt2_2_compressed_multi_unlimited_readable_by_h5py() { ); std::fs::remove_file(&path).ok(); } + +/// BT2-3: more chunks than a 2048-byte node holds, so the index must be a real +/// tree — a root of separator records over several leaves — not one oversized +/// node. libhdf5 sizes every node from the header's `node_size` and descends +/// through the child pointers, so a flat index would either read past a node or +/// lose every record beyond the first. +#[test] +fn bt2_3_multi_node_tree_readable_by_h5py() { + let Some(py) = python() else { return }; + let path = tmp("bt2_multinode"); + // 24-byte records, so a leaf holds (2048 - 10) / 24 = 84. A 20x20 extent in + // 2x2 chunks is 100 of them. + let data: Vec = (0..400).collect(); + { + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([20, 20]) + .chunk(&[2, 2]) + .max_shape(&[None, None]) + .create("grid") + .unwrap(); + ds.write_slice(&[0, 0], &[20, 20], &data).unwrap(); + file.close().unwrap(); + } + // Our own reader must agree before h5py is consulted. + { + let file = H5File::open(&path).unwrap(); + let ds = file.dataset("grid").unwrap(); + assert_eq!(ds.read_raw::().unwrap(), data); + } + read_back_with_h5py( + py, + &path, + "ds = f['grid']\n\ + assert ds.chunks == (2, 2), ds.chunks\n\ + assert ds.id.get_num_chunks() == 100, ds.id.get_num_chunks()\n\ + assert np.array_equal(ds[...], np.arange(400).reshape(20, 20)), ds[...]\n", + ); + std::fs::remove_file(&path).ok(); +} + +/// BT2-4: past 5269 records a depth-1 tree is full too, so the root becomes an +/// internal node over internal nodes. Depth 2 is the first shape where a child +/// pointer carries a subtree total (`child_total_nrecords`), whose width comes +/// from the geometry rather than the record — get it wrong and libhdf5 +/// misparses every pointer after the first. +#[test] +fn bt2_4_depth_two_tree_readable_by_h5py() { + let Some(py) = python() else { return }; + let path = tmp("bt2_depth2"); + // 1x1 chunks: 73 * 73 = 5329 records, past the 61 + 62 * 84 = 5269 a + // depth-1 tree holds. + let n = 73usize; + let data: Vec = (0..(n * n) as i32).collect(); + { + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([n, n]) + .chunk(&[1, 1]) + .max_shape(&[None, None]) + .create("grid") + .unwrap(); + ds.write_slice(&[0, 0], &[n, n], &data).unwrap(); + file.close().unwrap(); + } + { + let file = H5File::open(&path).unwrap(); + let ds = file.dataset("grid").unwrap(); + assert_eq!(ds.read_raw::().unwrap(), data); + } + read_back_with_h5py( + py, + &path, + &format!( + "ds = f['grid']\n\ + assert ds.chunks == (1, 1), ds.chunks\n\ + assert ds.id.get_num_chunks() == {}, ds.id.get_num_chunks()\n\ + assert np.array_equal(ds[...], np.arange({}).reshape({n}, {n})), ds[...]\n", + n * n, + n * n + ), + ); + std::fs::remove_file(&path).ok(); +} From 65f421e99e144b0258129f8034ace516a673b45f Mon Sep 17 00:00:00 2001 From: Sang Woo Kim Date: Thu, 13 Aug 2026 01:06:40 +0900 Subject: [PATCH 13/20] Support direct chunk writes on a v2-B-tree index MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `write_chunk_raw` rejected a BT2-indexed dataset outright, and there was no other way in: the four-cell grid of chunk-write entry points — linear vs coordinate addressing, filters applied here vs upstream — was missing its coordinate/direct corner. A dataset with two or more unlimited dimensions has no fixed chunk grid for a linear index to mean anything against, so BT2 could reach only the two `_at` forms that did not exist for direct writes. `write_chunk_raw_at` fills the cell, and `write_chunk_raw` now points at it the way `write_chunk` already pointed at `write_chunk_at`. Both coordinate forms run through one dispatch, `write_chunk_at_inner`, with `ChunkBytes` carrying whether the pipeline still has to run — so validating the coordinates and growing the dataspace has one owner rather than a second copy that can drift. Underneath, `record_btree_v2_chunk` becomes the single placement owner for BT2 chunks, shared by `write_chunk_btree_v2` and the new `write_compressed_chunk_btree_v2` — the same split the extensible- and fixed-array paths use. It also picks up the stored-size check those paths have (`H5D_CHUNK_ENCODE_SIZE_CHECK`): a direct write hands over caller-supplied bytes, which can overflow a type-11 record's chunk-size field and would otherwise truncate silently. --- src/dataset.rs | 238 ++++++++++++++++++++++++++++++--- src/io/writer.rs | 80 ++++++++++- tests/h5py_cross_validation.rs | 55 ++++++++ 3 files changed, 350 insertions(+), 23 deletions(-) diff --git a/src/dataset.rs b/src/dataset.rs index c490144..18fb4ea 100644 --- a/src/dataset.rs +++ b/src/dataset.rs @@ -476,6 +476,20 @@ pub struct H5Dataset { info: DatasetInfo, } +/// One chunk's bytes on the way to the file, and who filtered them. +/// +/// This is what separates a normal chunk write from a direct one; everything +/// else about placing a chunk is identical, so the two share a single dispatch. +#[derive(Clone, Copy)] +enum ChunkBytes<'a> { + /// The chunk's raw bytes; the dataset's filter pipeline runs before they + /// are stored. + Unfiltered(&'a [u8]), + /// Bytes already in their stored form, with `filter_mask` naming the + /// filters that were skipped. + Prefiltered { data: &'a [u8], filter_mask: u32 }, +} + impl H5Dataset { /// Create a reader-mode dataset handle (called internally by `H5File::dataset`). pub(crate) fn new_reader( @@ -1166,9 +1180,10 @@ impl H5Dataset { /// common case: a codec plugin handed you compressed frames). /// /// The dataset must be chunked **and** filtered; an unfiltered chunk index - /// has no slot to record a stored size or mask. For a v2-B-tree-indexed - /// dataset (two or more unlimited dimensions) direct chunk writes are not - /// supported. + /// has no slot to record a stored size or mask. A v2-B-tree-indexed dataset + /// (two or more unlimited dimensions) has no fixed chunk grid to linearize + /// against, so address its chunks with + /// [`write_chunk_raw_at`](Self::write_chunk_raw_at) instead. /// /// # Reading back /// @@ -1191,8 +1206,8 @@ impl H5Dataset { } if *btree2 { return Err(Hdf5Error::InvalidState( - "direct chunk writes are not supported for v2-B-tree-indexed \ - datasets (two or more unlimited dimensions)" + "this dataset uses a v2 B-tree chunk index; use \ + write_chunk_raw_at with the chunk's grid coordinates" .into(), )); } @@ -1285,6 +1300,50 @@ impl H5Dataset { /// ds.write_chunk_at(&[0, 0], &bytes).unwrap(); /// ``` pub fn write_chunk_at(&self, chunk_coords: &[usize], data: &[u8]) -> Result<()> { + self.write_chunk_at_inner(chunk_coords, ChunkBytes::Unfiltered(data), "write_chunk_at") + } + + /// Write an already-filtered chunk **verbatim** to a chunked dataset, + /// addressed by its chunk-grid coordinates. + /// + /// The coordinate-addressed twin of + /// [`write_chunk_raw`](Self::write_chunk_raw), and the form a + /// v2-B-tree-indexed dataset needs: with two or more unlimited dimensions + /// there is no fixed chunk grid for a linear index to mean anything against. + /// As with `write_chunk_at`, the dataset's logical dimensions are extended + /// to cover the written chunk. + /// + /// `data` is the already-filtered bytes of one chunk — its length is the + /// *stored* size — and `filter_mask` bit *i* set means filter *i* of the + /// pipeline was **not** applied and must be skipped on read. Pass 0 when the + /// full pipeline already ran upstream. + /// + /// The dataset must be chunked **and** filtered; an unfiltered chunk index + /// has no slot to record a stored size or mask. + pub fn write_chunk_raw_at( + &self, + chunk_coords: &[usize], + data: &[u8], + filter_mask: u32, + ) -> Result<()> { + self.write_chunk_at_inner( + chunk_coords, + ChunkBytes::Prefiltered { data, filter_mask }, + "write_chunk_raw_at", + ) + } + + /// The single owner of coordinate-addressed chunk writes: validates the + /// coordinates, grows the dataspace to cover them, and routes the bytes to + /// whichever chunk index the dataset uses. Whether the filter pipeline runs + /// here or already ran upstream is carried by `bytes`, not by a second copy + /// of this dispatch. + fn write_chunk_at_inner( + &self, + chunk_coords: &[usize], + bytes: ChunkBytes<'_>, + what: &str, + ) -> Result<()> { match &self.info { DatasetInfo::Writer { index, @@ -1294,9 +1353,9 @@ impl H5Dataset { .. } => { if !*chunked { - return Err(Hdf5Error::InvalidState( - "write_chunk_at is only for chunked datasets".into(), - )); + return Err(Hdf5Error::InvalidState(format!( + "{what} is only for chunked datasets" + ))); } let coords: Vec = chunk_coords.iter().map(|&c| c as u64).collect(); let btree2 = *btree2; @@ -1351,12 +1410,29 @@ impl H5Dataset { if fixed_array { // Fixed-array (fixed-shape) dataset: no dimension growth. - writer.write_chunk_fixed_array(*index, &coords, data)?; + match bytes { + ChunkBytes::Unfiltered(data) => { + writer.write_chunk_fixed_array(*index, &coords, data)? + } + ChunkBytes::Prefiltered { data, filter_mask } => writer + .write_compressed_chunk_fixed_array( + *index, + &coords, + data, + filter_mask, + )?, + } return Ok(()); } if btree2 { - writer.write_chunk_btree_v2(*index, &coords, data)?; + match bytes { + ChunkBytes::Unfiltered(data) => { + writer.write_chunk_btree_v2(*index, &coords, data)? + } + ChunkBytes::Prefiltered { data, filter_mask } => writer + .write_compressed_chunk_btree_v2(*index, &coords, data, filter_mask)?, + } } else { // Extensible array: linearize the chunk-grid coordinates // (row-major) into the array's chunk index. @@ -1376,7 +1452,12 @@ impl H5Dataset { ) })?; } - writer.write_chunk(*index, linear, data)?; + match bytes { + ChunkBytes::Unfiltered(data) => writer.write_chunk(*index, linear, data)?, + ChunkBytes::Prefiltered { data, filter_mask } => { + writer.write_compressed_chunk(*index, linear, data, filter_mask)? + } + } } if new_dims != dims { @@ -4184,10 +4265,11 @@ mod tests { std::fs::remove_file(&path).ok(); } - /// v2-B-tree-indexed datasets (two or more unlimited dimensions) do not - /// support direct chunk writes. + /// Two or more unlimited dimensions leave no fixed chunk grid for a linear + /// index to mean anything against, so the linear entry point points the + /// caller at the coordinate-addressed one rather than guessing a grid. #[test] - fn write_chunk_raw_rejects_btree_v2() { + fn write_chunk_raw_sends_btree_v2_to_the_coordinate_form() { let path = temp_path("wcr_btree2"); let file = H5File::create(&path).unwrap(); let ds = file @@ -4199,8 +4281,132 @@ mod tests { .unwrap(); let err = ds.write_chunk_raw(0, &[0u8; 16], 0).unwrap_err(); assert!( - err.to_string().contains("v2-B-tree"), - "expected a v2-B-tree rejection, got: {err}" + err.to_string().contains("write_chunk_raw_at"), + "expected a pointer to the coordinate form, got: {err}" + ); + std::fs::remove_file(&path).ok(); + } + + /// Direct chunk writes on a v2-B-tree index: the bytes are stored verbatim + /// and the type-11 record carries their size and the caller's mask, so a + /// chunk written with the pipeline skipped (mask 1) reads back as the raw + /// bytes while one written compressed (mask 0) is decompressed. + #[cfg(feature = "deflate")] + #[test] + fn write_chunk_raw_at_round_trips_on_btree_v2() { + use crate::format::messages::filter::{apply_filters, FilterPipeline}; + + let path = temp_path("wcr_at_btree2"); + let raw0: Vec = (0..4i32).flat_map(|v| v.to_le_bytes()).collect(); + let raw1: Vec = (100..104i32).flat_map(|v| v.to_le_bytes()).collect(); + { + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([0, 0]) + .chunk(&[2, 2]) + .max_shape(&[None, None]) + .deflate(6) + .create("grid") + .unwrap(); + let pipeline = FilterPipeline::deflate(6); + // Chunk (0,0): pipeline already applied upstream, mask 0. + ds.write_chunk_raw_at(&[0, 0], &apply_filters(&pipeline, &raw0).unwrap(), 0) + .unwrap(); + // Chunk (1,1): stored uncompressed, mask 1 says filter 0 was skipped. + ds.write_chunk_raw_at(&[1, 1], &raw1, 1).unwrap(); + file.close().unwrap(); + } + let file = H5File::open(&path).unwrap(); + let ds = file.dataset("grid").unwrap(); + assert_eq!(ds.shape(), vec![4, 4]); + let all = ds.read_raw::().unwrap(); + // Chunk (0,0) occupies rows 0..2, columns 0..2. + assert_eq!([all[0], all[1], all[4], all[5]], [0, 1, 2, 3]); + // Chunk (1,1) occupies rows 2..4, columns 2..4. + assert_eq!([all[10], all[11], all[14], all[15]], [100, 101, 102, 103]); + drop(file); + std::fs::remove_file(&path).ok(); + } + + /// The coordinate form is not BT2-only: it addresses an extensible- or + /// fixed-array dataset's grid just as well, and records the same mask. + #[cfg(feature = "deflate")] + #[test] + fn write_chunk_raw_at_round_trips_on_the_array_indexes() { + for (label, max_shape) in [ + ("wcr_at_ea", Some(vec![None, Some(4usize)])), + ("wcr_at_fa", None), + ] { + let path = temp_path(label); + let raw: Vec = (0..8i32).flat_map(|v| v.to_le_bytes()).collect(); + { + let file = H5File::create(&path).unwrap(); + let mut b = file + .new_dataset::() + .shape([4usize, 4]) + .chunk(&[2, 4]) + .deflate(6); + if let Some(ref ms) = max_shape { + b = b.max_shape(ms); + } + let ds = b.create("grid").unwrap(); + // Row-of-chunks 1, stored uncompressed with filter 0 skipped. + ds.write_chunk_raw_at(&[1, 0], &raw, 1).unwrap(); + file.close().unwrap(); + } + let file = H5File::open(&path).unwrap(); + let ds = file.dataset("grid").unwrap(); + let all = ds.read_raw::().unwrap(); + assert_eq!(&all[8..16], &(0..8).collect::>()[..], "{label}"); + drop(file); + std::fs::remove_file(&path).ok(); + } + } + + /// A direct write hands over caller-supplied bytes, so the v2 B-tree's + /// chunk-size field can overflow just as the array indexes' can. A 4-byte + /// chunk gives chunk_size_len = 2 (max 65535). + #[cfg(feature = "deflate")] + #[test] + fn write_chunk_raw_at_rejects_an_oversized_btree_v2_chunk() { + let path = temp_path("wcr_at_oversized"); + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([0, 0]) + .chunk(&[1, 1]) + .max_shape(&[None, None]) + .deflate(4) + .create("grid") + .unwrap(); + let err = ds + .write_chunk_raw_at(&[0, 0], &vec![0u8; 70000], 0) + .unwrap_err(); + assert!( + err.to_string().contains("does not fit"), + "expected a chunk-size-field overflow error, got: {err}" + ); + std::fs::remove_file(&path).ok(); + } + + /// An unfiltered v2 B-tree record has no slot for a stored size or mask, + /// the same reason the array indexes reject a direct write. + #[test] + fn write_chunk_raw_at_rejects_an_unfiltered_btree_v2() { + let path = temp_path("wcr_at_unfiltered"); + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([0, 0]) + .chunk(&[2, 2]) + .max_shape(&[None, None]) + .create("grid") + .unwrap(); + let err = ds.write_chunk_raw_at(&[0, 0], &[0u8; 16], 0).unwrap_err(); + assert!( + err.to_string().contains("filtered dataset"), + "expected a filtered-dataset error, got: {err}" ); std::fs::remove_file(&path).ok(); } diff --git a/src/io/writer.rs b/src/io/writer.rs index fbe22c5..3efca8f 100644 --- a/src/io/writer.rs +++ b/src/io/writer.rs @@ -4020,10 +4020,74 @@ impl Hdf5Writer { } None => data, }; - let stored_len = stored.len() as u64; + // filter_mask = 0: the whole pipeline ran (or the dataset is + // unfiltered), so no filter is skipped. + self.record_btree_v2_chunk(index, chunk_coords, stored, 0) + } + + /// Write a pre-filtered chunk verbatim to a BT2-indexed dataset, recording + /// the caller-supplied `filter_mask`. + /// + /// The v2-B-tree half of the HDF5 "direct chunk write" (`H5Dwrite_chunk`). + /// The bytes are stored exactly as given; `filter_mask` bit *i* set means + /// filter *i* of the pipeline was **not** applied and must be skipped on + /// read. Requires a filtered dataset — only a type-11 record has a slot for + /// a stored size and mask. + pub fn write_compressed_chunk_btree_v2( + &self, + index: usize, + chunk_coords: &[u64], + data: &[u8], + filter_mask: u32, + ) -> IoResult<()> { + if self.ds(index).lock().filter_pipeline.is_none() { + return Err(crate::io::IoError::InvalidState( + "write_compressed_chunk_btree_v2 requires a filtered dataset (no \ + slot for a compressed size or filter mask on an unfiltered chunk \ + index)" + .into(), + )); + } + self.record_btree_v2_chunk(index, chunk_coords, data, filter_mask) + } + + /// Place a chunk's already-final bytes (filtered if the dataset is + /// filtered, raw otherwise) in the file and record them in the v2 B-tree, + /// under the caller-supplied `filter_mask`. + /// + /// Shared by [`write_chunk_btree_v2`](Self::write_chunk_btree_v2) and + /// [`write_compressed_chunk_btree_v2`](Self::write_compressed_chunk_btree_v2), + /// so both reach the index through one placement rule. + fn record_btree_v2_chunk( + &self, + index: usize, + chunk_coords: &[u64], + final_bytes: &[u8], + filter_mask: u32, + ) -> IoResult<()> { + let stored_len = final_bytes.len() as u64; + let ds = self.ds(index); let mut m = ds.lock(); - let bt2 = m.btree_v2.as_ref().unwrap(); + let element_size = m.datatype.element_size() as u64; + let bt2 = m + .btree_v2 + .as_ref() + .ok_or_else(|| crate::io::IoError::InvalidState("not a B-tree v2 dataset".into()))?; + let chunk_bytes = bt2.chunk_dims.iter().product::() * element_size; + // A filtered record encodes the stored size in a `chunk_size_len`-byte + // field that truncates silently. Reject a size that would not fit, as + // the extensible-array path does — the compress path never exceeds it, + // but a direct write with caller-supplied bytes can. + if bt2.index.filtered { + let chunk_size_len = bt2.index.chunk_size_len as usize; + if chunk_size_len < 8 && stored_len >= (1u64 << (chunk_size_len * 8)) { + return Err(crate::io::IoError::InvalidState(format!( + "filtered chunk size {stored_len} does not fit in the \ + {chunk_size_len}-byte v2 B-tree chunk-size field" + ))); + } + } // Place the bytes: a rewrite whose stored size is unchanged stays // where it is (always so when unfiltered — the size is fixed by the // chunk shape), and one that no longer fits moves, releasing its old @@ -4038,14 +4102,16 @@ impl Hdf5Writer { .map(|r| (r.chunk_address, chunk_bytes)) }; let chunk_addr = self.place_chunk(old, stored_len); - self.handle.write_at(chunk_addr, stored)?; + self.handle.write_at(chunk_addr, final_bytes)?; - // Insert into the in-memory BT2 index. filter_mask = 0: the whole - // pipeline ran (or the dataset is unfiltered), so no filter is skipped. let bt2 = m.btree_v2.as_mut().unwrap(); if bt2.index.filtered { - bt2.index - .insert_filtered(chunk_coords.to_vec(), chunk_addr, stored_len as u32, 0); + bt2.index.insert_filtered( + chunk_coords.to_vec(), + chunk_addr, + stored_len as u32, + filter_mask, + ); } else { bt2.index.insert(chunk_coords.to_vec(), chunk_addr); } diff --git a/tests/h5py_cross_validation.rs b/tests/h5py_cross_validation.rs index de0e99c..4ca9430 100644 --- a/tests/h5py_cross_validation.rs +++ b/tests/h5py_cross_validation.rs @@ -632,6 +632,61 @@ fn bt2_2_compressed_multi_unlimited_readable_by_h5py() { std::fs::remove_file(&path).ok(); } +/// BT2-5: a direct chunk write on a v2 B-tree — bytes stored verbatim with a +/// per-chunk filter mask. libhdf5 must skip exactly the filters the mask marks +/// as not applied, so one chunk arrives deflated (mask 0) and its neighbour +/// uncompressed (mask 1) and both must read back as the same values. +#[cfg(feature = "deflate")] +#[test] +fn bt2_5_direct_chunk_write_mask_honored_by_h5py() { + use flate2::{write::ZlibEncoder, Compression}; + use std::io::Write; + + let Some(py) = python() else { return }; + let path = tmp("bt2_direct"); + let bytes = |vals: [i32; 4]| -> Vec { vals.iter().flat_map(|v| v.to_le_bytes()).collect() }; + let deflate = |raw: &[u8]| -> Vec { + let mut e = ZlibEncoder::new(Vec::new(), Compression::new(6)); + e.write_all(raw).unwrap(); + e.finish().unwrap() + }; + { + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([0, 0]) + .chunk(&[2, 2]) + .max_shape(&[None, None]) + .deflate(6) + .create("grid") + .unwrap(); + ds.write_chunk_raw_at(&[0, 0], &deflate(&bytes([0, 1, 4, 5])), 0) + .unwrap(); + // Same values, handed over uncompressed with filter 0 marked skipped. + ds.write_chunk_raw_at(&[0, 1], &bytes([2, 3, 6, 7]), 1) + .unwrap(); + ds.write_chunk_raw_at(&[1, 0], &deflate(&bytes([8, 9, 12, 13])), 0) + .unwrap(); + ds.write_chunk_raw_at(&[1, 1], &bytes([10, 11, 14, 15]), 1) + .unwrap(); + file.close().unwrap(); + } + read_back_with_h5py( + py, + &path, + "ds = f['grid']\n\ + assert ds.compression == 'gzip', ds.compression\n\ + assert ds.chunks == (2, 2), ds.chunks\n\ + assert np.array_equal(ds[...], np.arange(16).reshape(4, 4)), ds[...]\n\ + # The mask-1 chunks are stored raw (16 B); the mask-0 ones went through\n\ + # deflate, so their filter_mask differs.\n\ + masks = sorted(ds.id.get_chunk_info(i).filter_mask\n\ + for i in range(ds.id.get_num_chunks()))\n\ + assert masks == [0, 0, 1, 1], masks\n", + ); + std::fs::remove_file(&path).ok(); +} + /// BT2-3: more chunks than a 2048-byte node holds, so the index must be a real /// tree — a root of separator records over several leaves — not one oversized /// node. libhdf5 sizes every node from the header's `node_size` and descends From 921f4d8cdd9835e109d7b7586af6dc573903f79f Mon Sep 17 00:00:00 2001 From: Sang Woo Kim Date: Thu, 13 Aug 2026 01:07:10 +0900 Subject: [PATCH 14/20] Document the v2-B-tree node rework and direct chunk writes --- CHANGELOG.md | 25 +++++++++++++++++++++++++ README.md | 5 ++++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 24ac633..35909a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,17 @@ stored size and filter mask, so a partial `write_slice` can decompress, patch and recompress a chunk and relocate it when its size changes. +- `write_chunk_raw_at` — the coordinate-addressed direct chunk write + (`H5Dwrite_chunk`), storing already-filtered bytes verbatim under a + caller-supplied per-chunk filter mask. This is the form a v2-B-tree-indexed + dataset needs: with two or more unlimited dimensions there is no fixed chunk + grid for the linear `write_chunk_raw` index to mean anything against, so + that entry point now points here instead of rejecting the dataset outright. + It works on the extensible- and fixed-array indexes too. Direct writes now + also range-check the stored size against the index's chunk-size field on all + three indexes (libhdf5 `H5D_CHUNK_ENCODE_SIZE_CHECK`) rather than truncating + it silently. + ### Fixed - A v2 B-tree chunk index is now written with its records ordered by scaled @@ -45,6 +56,20 @@ constructor and the index previously disagreed about it (a 32- versus 36-byte record); both now take it from one value. +- A v2 B-tree chunk index is now a real tree of fixed-size nodes, and flushing + it no longer leaks. Node size was previously derived from the record count + and everything lived in one depth-0 leaf, which caused two failures: the + leaf grew with every chunk, so each flush allocated a larger block and + stranded the previous one; and one node had to hold every record, but a + node's record count is a `u16`, so a dataset past 65535 chunks truncated it + silently. Nodes are now 2048 bytes (libhdf5's `H5D_BT2_NODE_SIZE`, which the + layout message already declared) and the records are bulk-loaded into as + many levels as they need, with internal nodes carrying the separators and + subtree totals libhdf5 descends. Because every node is the same size, a + flush overwrites the blocks already on disk and allocates only the + shortfall, so no block is orphaned and the addresses a reader holds stay + valid. + - Rewriting a chunk no longer leaks its old file block. Every chunk write previously allocated fresh space and left the previous block stranded, so repeatedly rewriting the same chunk grew the file without bound — including diff --git a/README.md b/README.md index 5b9a0ee..30d66e6 100644 --- a/README.md +++ b/README.md @@ -273,7 +273,10 @@ rust-hdf5 = { version = "0.2", features = ["lz4", "zstd"] } Filters apply to every chunked layout, whichever chunk index the dataspace selects: a fixed array (no unlimited dimension), an extensible array (exactly -one), or a v2 B-tree (two or more). +one), or a v2 B-tree (two or more). Already-filtered bytes can be handed over +verbatim with a per-chunk filter mask — HDF5's direct chunk write — via +`write_chunk_raw` (linear index) or `write_chunk_raw_at` (grid coordinates, +and the form a v2-B-tree dataset uses). ## Feature flags From 529bc4d73c532b23d6441479b252835350aa3899 Mon Sep 17 00:00:00 2001 From: Sang Woo Kim Date: Thu, 13 Aug 2026 01:14:46 +0900 Subject: [PATCH 15/20] Verify the v2 B-tree node blocks and the pool's accounting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two properties of the node pool were argued in comments rather than tested, so neither the padding nor the "no flush strands a block" claim had anything holding it up. `every_btree_v2_node_block_is_fully_written_to_the_file` checks each node block against the file's length straight after a flush — the moment the node blocks are the newest allocations, so nothing else has extended the file past them. Dropping the padding fails it: a reader asking for node_size bytes past end-of-file gets zeros (libhdf5 `H5FD__sec2_read`, "end of file but not end of format address space"), not the node the allocator handed out. The comment now says that instead of asserting the read would fail. `a_btree_v2_flush_allocates_only_the_node_blocks_it_adds` walks a tree from one leaf through a depth-1 split, flushing at each step, and requires every earlier block address to be reused and the file to grow by exactly the new blocks plus the new chunk bytes. Re-flushing an unchanged index must cost nothing. Reallocating the pool fails it. The pool never shrinking is now stated where it is enforced: the node count cannot fall because `Bt2ChunkIndex` has no record-removal path, and a `debug_assert` at the flush marks where a future one would have to release the surplus. --- src/format/chunk_index/btree_v2.rs | 8 +- src/io/writer.rs | 122 ++++++++++++++++++++++++++++- 2 files changed, 124 insertions(+), 6 deletions(-) diff --git a/src/format/chunk_index/btree_v2.rs b/src/format/chunk_index/btree_v2.rs index 7f59206..2a49c25 100644 --- a/src/format/chunk_index/btree_v2.rs +++ b/src/format/chunk_index/btree_v2.rs @@ -861,8 +861,12 @@ impl Bt2Tree { ) }; debug_assert!(image.len() <= self.node_size as usize); - // A reader reads the whole block, so the padding has to be - // there even though only the prefix is checksummed. + // Only the used prefix is checksummed, but the image is padded + // to the whole block so the block exists in the file rather + // than only in the allocator's ledger: a reader asking for + // `node_size` bytes past end-of-file gets zeros instead + // (libhdf5 `H5FD__sec2_read`, "end of file but not end of + // format address space"), which is not the node it allocated. image.resize(self.node_size as usize, 0); image }) diff --git a/src/io/writer.rs b/src/io/writer.rs index 3efca8f..67ef871 100644 --- a/src/io/writer.rs +++ b/src/io/writer.rs @@ -412,10 +412,15 @@ pub struct Bt2DatasetInfo { /// Pool of `BT2_NODE_SIZE`-byte blocks holding the tree's nodes, in the /// order [`Bt2Tree::encode`] emits them. /// - /// Append-only and the single owner of the tree's node addresses: a flush - /// re-serializes the whole tree over these blocks and allocates only the - /// shortfall, so no flush can orphan a block it replaced. Every node is the - /// same size, so a block stays usable however the tree reshapes. + /// The single owner of the tree's node addresses: a flush re-serializes the + /// whole tree over these blocks and allocates only the shortfall, so no + /// flush can orphan a block it replaced. Every node is the same size, so a + /// block stays usable however the tree reshapes. + /// + /// The pool grows and never shrinks, because the node count cannot fall: + /// [`Bt2ChunkIndex`] has no record-removal path, only insert-or-replace. + /// A future removal would have to release the surplus blocks here — the + /// `debug_assert` in `flush_dataset_synced` marks the spot. pub node_addrs: Vec, /// In-memory chunk index. pub index: Bt2ChunkIndex, @@ -4365,6 +4370,10 @@ impl Hdf5Writer { // holds stay valid, which is also what SWMR needs. let tree = bt2.index.build_tree(&self.ctx); let mut node_addrs = bt2.node_addrs.clone(); + // Records are only ever inserted or replaced, so the node count + // cannot fall. If that ever changes, the surplus blocks have to be + // released here rather than left recorded and unused. + debug_assert!(node_addrs.len() <= tree.nodes.len()); while node_addrs.len() < tree.nodes.len() { node_addrs.push(self.allocator.allocate(tree.node_size as u64)); } @@ -5568,6 +5577,111 @@ mod tests { std::fs::remove_file(&path).ok(); } + /// Bytes one chunk of [`btree_v2_flush_probe`]'s dataset occupies — an + /// f64 element, so the allocator's alignment neither pads nor merges it and + /// the file's growth is exactly the bytes asked for. + const BT2_PROBE_CHUNK: u64 = 8; + + /// Write chunks of a 1x1-chunked 2-D BT2 dataset, flushing at each batch + /// boundary, and report `(node addresses, file length)` after every flush. + /// Chunks are addressed down column 0 so the record count — and hence the + /// tree's shape — grows one record at a time. + fn btree_v2_flush_probe(path: &std::path::Path, batches: &[u64]) -> Vec<(Vec, u64)> { + let writer = Hdf5Writer::create(path).unwrap(); + let idx = writer + .create_btree_v2_dataset( + "data", + DatatypeMessage::f64_type(), + &[0, 0], + &[u64::MAX, u64::MAX], + &[1, 1], + ) + .unwrap(); + let mut written = 0u64; + let mut out = Vec::new(); + for &upto in batches { + while written < upto { + writer + .write_chunk_btree_v2(idx, &[written, 0], &(written as f64).to_le_bytes()) + .unwrap(); + written += 1; + } + writer.flush_dataset(idx).unwrap(); + let addrs = writer + .ds(idx) + .lock() + .btree_v2 + .as_ref() + .unwrap() + .node_addrs + .clone(); + out.push((addrs, std::fs::metadata(path).unwrap().len())); + } + writer.extend_dataset(idx, &[written.max(1), 1]).unwrap(); + writer.close().unwrap(); + out + } + + /// A node block is only usable if the bytes are actually in the file. A + /// reader asked for `BT2_NODE_SIZE` bytes at a node address gets zeros past + /// end-of-file rather than the block it allocated, so the writer pads each + /// node image to the full block. Checking straight after a flush is what + /// makes this bite: the node blocks are the newest allocations, so nothing + /// else has extended the file past them. + #[test] + fn every_btree_v2_node_block_is_fully_written_to_the_file() { + use crate::format::chunk_index::btree_v2::BT2_NODE_SIZE; + + let path = temp_path("bt2_node_blocks"); + // 90 records crosses the 84 one leaf holds, so this covers a leaf, an + // internal node and a root. + for (addrs, file_len) in btree_v2_flush_probe(&path, &[1, 84, 90]) { + assert!(!addrs.is_empty()); + for addr in addrs { + assert!( + file_len >= addr + BT2_NODE_SIZE as u64, + "node block at {addr:#x} runs past end-of-file ({file_len})" + ); + } + } + std::fs::remove_file(&path).ok(); + } + + /// The node pool is the single owner of the tree's block addresses: a flush + /// reuses every block already in it and allocates only the shortfall. So + /// re-flushing an unchanged index must cost nothing, and a flush that grows + /// the tree must cost exactly the blocks it added — anything more means a + /// block was stranded. + #[test] + fn a_btree_v2_flush_allocates_only_the_node_blocks_it_adds() { + use crate::format::chunk_index::btree_v2::BT2_NODE_SIZE; + + let path = temp_path("bt2_pool_growth"); + // Re-flush at 84 (still one leaf), then cross into a three-node depth-1 + // tree, then keep growing. + let batches = [84u64, 84, 85, 200, 200]; + let probe = btree_v2_flush_probe(&path, &batches); + for i in 1..probe.len() { + let (prev_addrs, prev_len) = &probe[i - 1]; + let (addrs, len) = &probe[i]; + assert!( + addrs.starts_with(prev_addrs), + "flush {i} moved a node block instead of reusing it" + ); + let new_blocks = (addrs.len() - prev_addrs.len()) as u64 * BT2_NODE_SIZE as u64; + let new_chunks = (batches[i] - batches[i - 1]) * BT2_PROBE_CHUNK; + assert_eq!( + len - prev_len, + new_blocks + new_chunks, + "flush {i} grew the file by more than the blocks it added" + ); + } + // The unchanged re-flushes must be free. + assert_eq!(probe[1].1, probe[0].1); + assert_eq!(probe[4].1, probe[3].1); + std::fs::remove_file(&path).ok(); + } + #[cfg(feature = "parallel")] #[test] fn parallel_batch_write_roundtrip() { From 2ce0e04a4f84bfaadc92e87cbcdabb2ca5a04187 Mon Sep 17 00:00:00 2001 From: Sang Woo Kim Date: Thu, 13 Aug 2026 01:23:11 +0900 Subject: [PATCH 16/20] Justify BT2 node padding by what it actually prevents The padding comment claimed a reader asking for BT2_NODE_SIZE bytes past end-of-file would get zeros instead of the allocated block. That is false here: the superblock's end-of-file address is the allocator's EOF, and libhdf5 zero-fills EOF->EOA, so an unpadded tail and a zero-padded tail read identically. The test built on that claim (every_btree_v2_node_block_is_fully_written_to_the_file) passed with the padding removed. What padding does prevent: a node's record count falls as well as rises. The first leaf holds 84 records; at 85 the tree splits and that same block holds 42. Without padding the flush writes only the shorter prefix, leaving 1008 bytes of the previous 84-record image in a live node block. A conforming reader stops at the count its parent gives it, but the stale records are there for anything reading the file raw. Replace the test with one that reads the leaf block after the split and asserts the tail past the used prefix is zero. It fails when the resize() is removed. --- src/format/chunk_index/btree_v2.rs | 14 +++++--- src/io/writer.rs | 54 +++++++++++++++++++----------- 2 files changed, 44 insertions(+), 24 deletions(-) diff --git a/src/format/chunk_index/btree_v2.rs b/src/format/chunk_index/btree_v2.rs index 2a49c25..3eb46bb 100644 --- a/src/format/chunk_index/btree_v2.rs +++ b/src/format/chunk_index/btree_v2.rs @@ -862,11 +862,15 @@ impl Bt2Tree { }; debug_assert!(image.len() <= self.node_size as usize); // Only the used prefix is checksummed, but the image is padded - // to the whole block so the block exists in the file rather - // than only in the allocator's ledger: a reader asking for - // `node_size` bytes past end-of-file gets zeros instead - // (libhdf5 `H5FD__sec2_read`, "end of file but not end of - // format address space"), which is not the node it allocated. + // to the whole block so re-serializing overwrites the block + // rather than a prefix of it. A node's record count falls as + // well as rises — a full 84-record leaf becomes two 42-record + // leaves when the tree splits — so a short write would leave + // the tail of the previous, larger image behind. A conforming + // reader stops at the record count its parent gives it and + // never sees those bytes, but they are stale records in a live + // node block, and anything scanning the file raw reads them as + // real. image.resize(self.node_size as usize, 0); image }) diff --git a/src/io/writer.rs b/src/io/writer.rs index 67ef871..0244ca2 100644 --- a/src/io/writer.rs +++ b/src/io/writer.rs @@ -5622,28 +5622,44 @@ mod tests { out } - /// A node block is only usable if the bytes are actually in the file. A - /// reader asked for `BT2_NODE_SIZE` bytes at a node address gets zeros past - /// end-of-file rather than the block it allocated, so the writer pads each - /// node image to the full block. Checking straight after a flush is what - /// makes this bite: the node blocks are the newest allocations, so nothing - /// else has extended the file past them. + /// A node's record count falls as well as rises: the tree's first leaf goes + /// from a full 84 records to 42 when 85 records force it to split. The node + /// image is padded to the whole block so re-serializing overwrites the + /// block, not a prefix of it — otherwise that leaf keeps the tail of its + /// 84-record self, stale records sitting in a live node block. #[test] - fn every_btree_v2_node_block_is_fully_written_to_the_file() { - use crate::format::chunk_index::btree_v2::BT2_NODE_SIZE; + fn a_shrinking_btree_v2_node_leaves_no_stale_records_behind() { + use crate::format::chunk_index::btree_v2::{Bt2ChunkIndex, BT2_NODE_SIZE}; let path = temp_path("bt2_node_blocks"); - // 90 records crosses the 84 one leaf holds, so this covers a leaf, an - // internal node and a root. - for (addrs, file_len) in btree_v2_flush_probe(&path, &[1, 84, 90]) { - assert!(!addrs.is_empty()); - for addr in addrs { - assert!( - file_len >= addr + BT2_NODE_SIZE as u64, - "node block at {addr:#x} runs past end-of-file ({file_len})" - ); - } - } + let probe = btree_v2_flush_probe(&path, &[84, 85]); + let node0 = probe.last().unwrap().0[0]; + + // What the first leaf holds once the tree has split. + let ctx = FormatContext { + sizeof_addr: 8, + sizeof_size: 8, + }; + let mut index = Bt2ChunkIndex::new_unfiltered(2); + for i in 0..85u64 { + index.insert(vec![i, 0], 0); + } + let tree = index.build_tree(&ctx); + assert!( + tree.nodes[0].num_records < 84, + "this test needs the first leaf to shrink, got {}", + tree.nodes[0].num_records + ); + // signature(4) + version(1) + type(1) + records + checksum(4) + let used = 10 + tree.nodes[0].num_records as usize * tree.record_size as usize; + + let bytes = std::fs::read(&path).unwrap(); + let block = &bytes[node0 as usize..node0 as usize + BT2_NODE_SIZE as usize]; + assert!( + block[used..].iter().all(|&b| b == 0), + "leaf block at {node0:#x} still holds {} bytes of its previous, larger image", + block[used..].iter().rposition(|&b| b != 0).unwrap_or(0) + 1 + ); std::fs::remove_file(&path).ok(); } From 07d09bbf2969f57d19d3925811efad94e104051d Mon Sep 17 00:00:00 2001 From: Sang Woo Kim Date: Thu, 13 Aug 2026 01:27:24 +0900 Subject: [PATCH 17/20] Free the v2 B-tree node blocks a shrinking tree gives up MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The flush grew the node pool and asserted it never shrank, on the grounds that Bt2ChunkIndex only inserts and replaces. That leaves the invariant enforceable by convention: `records` and `filtered_records` are public fields, a debug_assert is gone in release builds, and any future removal path silently strands the surplus blocks. Make the pool hold exactly one block per node whichever way the count moved: allocate the shortfall, free the surplus. Under SWMR keep the surplus out of the free list, since a reader may still hold a header naming those blocks — the rule place_chunk already applies to a relocated chunk. The test drops records itself, as a removal path would, and checks both halves: the pool follows the tree down to one block, and the next node-sized allocation lands inside the region the freed blocks covered. --- src/io/writer.rs | 102 +++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 94 insertions(+), 8 deletions(-) diff --git a/src/io/writer.rs b/src/io/writer.rs index 0244ca2..3bb4fc9 100644 --- a/src/io/writer.rs +++ b/src/io/writer.rs @@ -417,10 +417,11 @@ pub struct Bt2DatasetInfo { /// flush can orphan a block it replaced. Every node is the same size, so a /// block stays usable however the tree reshapes. /// - /// The pool grows and never shrinks, because the node count cannot fall: - /// [`Bt2ChunkIndex`] has no record-removal path, only insert-or-replace. - /// A future removal would have to release the surplus blocks here — the - /// `debug_assert` in `flush_dataset_synced` marks the spot. + /// The pool holds exactly one block per node after every flush, in both + /// directions: a taller tree allocates the shortfall, a smaller one frees + /// the surplus. Nothing here depends on the record count only ever rising, + /// so a record-removal path can be added to [`Bt2ChunkIndex`] without the + /// blocks it drops going unreachable. pub node_addrs: Vec, /// In-memory chunk index. pub index: Bt2ChunkIndex, @@ -4370,13 +4371,20 @@ impl Hdf5Writer { // holds stay valid, which is also what SWMR needs. let tree = bt2.index.build_tree(&self.ctx); let mut node_addrs = bt2.node_addrs.clone(); - // Records are only ever inserted or replaced, so the node count - // cannot fall. If that ever changes, the surplus blocks have to be - // released here rather than left recorded and unused. - debug_assert!(node_addrs.len() <= tree.nodes.len()); while node_addrs.len() < tree.nodes.len() { node_addrs.push(self.allocator.allocate(tree.node_size as u64)); } + // A tree with fewer nodes than last flush releases the surplus + // rather than leaving it recorded and unreachable, so the pool is + // exactly one block per node whichever way the count moved. Under + // SWMR a reader may still hold a header naming those blocks, so + // keep them out of the free list — the same rule `place_chunk` + // applies to a relocated chunk. + for addr in node_addrs.split_off(tree.nodes.len()) { + if !self.swmr_active { + self.allocator.free(addr, tree.node_size as u64); + } + } for (image, &addr) in tree.encode(&self.ctx, &node_addrs).iter().zip(&node_addrs) { self.handle.write_at(addr, image)?; @@ -5622,6 +5630,84 @@ mod tests { out } + /// The node pool tracks the tree in both directions. Dropping records is + /// what a removal path would do — [`Bt2ChunkIndex`] has none today, so the + /// test drops them itself — and the flush that follows must hand the blocks + /// its smaller tree no longer needs back to the allocator instead of + /// leaving them recorded and unreachable. + #[test] + fn a_btree_v2_flush_frees_the_node_blocks_its_tree_gave_up() { + use crate::format::chunk_index::btree_v2::BT2_NODE_SIZE; + + let path = temp_path("bt2_node_shrink"); + let writer = Hdf5Writer::create(&path).unwrap(); + let idx = writer + .create_btree_v2_dataset( + "data", + DatatypeMessage::f64_type(), + &[0, 0], + &[u64::MAX, u64::MAX], + &[1, 1], + ) + .unwrap(); + // 85 records is one past a leaf, so the tree is two leaves and a root. + for i in 0..85u64 { + writer + .write_chunk_btree_v2(idx, &[i, 0], &(i as f64).to_le_bytes()) + .unwrap(); + } + writer.flush_dataset(idx).unwrap(); + let grown = writer + .ds(idx) + .lock() + .btree_v2 + .as_ref() + .unwrap() + .node_addrs + .clone(); + assert_eq!(grown.len(), 3, "expected two leaves and a root"); + + // Back to 84 records: one leaf, so two of the three blocks are surplus. + writer + .ds(idx) + .lock() + .btree_v2 + .as_mut() + .unwrap() + .index + .records + .truncate(84); + writer.flush_dataset(idx).unwrap(); + let shrunk = writer + .ds(idx) + .lock() + .btree_v2 + .as_ref() + .unwrap() + .node_addrs + .clone(); + assert_eq!( + shrunk, + grown[..1], + "the pool still records the surplus blocks" + ); + + // The surplus went back to the allocator, not on the floor: the next + // node-sized allocation lands inside the region the two blocks covered. + let reused = writer.allocator.allocate(BT2_NODE_SIZE as u64); + assert!( + (grown[1]..grown[1] + 2 * BT2_NODE_SIZE as u64).contains(&reused), + "a node block allocated at {reused:#x}, outside the freed \ + [{:#x}, {:#x}) the flush gave up", + grown[1], + grown[1] + 2 * BT2_NODE_SIZE as u64 + ); + + writer.extend_dataset(idx, &[85, 1]).unwrap(); + writer.close().unwrap(); + std::fs::remove_file(&path).ok(); + } + /// A node's record count falls as well as rises: the tree's first leaf goes /// from a full 84 records to 42 when 85 records force it to split. The node /// image is padded to the whole block so re-serializing overwrites the From 6995508767df4da508f849eb829fc12d12719891 Mon Sep 17 00:00:00 2001 From: Sang Woo Kim Date: Thu, 13 Aug 2026 01:27:40 +0900 Subject: [PATCH 18/20] Note the node-pool shrink in the changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 35909a2..08b2b73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,7 +68,8 @@ subtree totals libhdf5 descends. Because every node is the same size, a flush overwrites the blocks already on disk and allocates only the shortfall, so no block is orphaned and the addresses a reader holds stay - valid. + valid. A tree that loses nodes releases the surplus blocks to the allocator, + except under SWMR where a reader may still hold a header naming them. - Rewriting a chunk no longer leaks its old file block. Every chunk write previously allocated fresh space and left the previous block stranded, so From 3a0c7b78e15c06b9cf340f14e9b9fb2f9925dac0 Mon Sep 17 00:00:00 2001 From: Sang Woo Kim Date: Thu, 13 Aug 2026 01:36:09 +0900 Subject: [PATCH 19/20] Keep the existing rows when a buffered append flushes (issue #3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An append that does not complete its chunk is buffered until close. flush_append_buffers built a fresh fill-value chunk and copied the buffered frames into it at their offset, so every frame the chunk already held was replaced by fill: create, append [1,2,3], close, reopen, append [4,5,6] read back as [0,0,0,4,5,6]. The in-session append paths in H5Dataset::append and append_vlen_strings each did the read-modify-write correctly, in two copies of the same loop; the flush was a third, shorter copy that dropped it. Rather than add the read to the third copy, give the three one owner — append_frames_into_chunk — that writes straight through when the span covers the whole chunk and reads-modifies-writes otherwise. The owner also drops the "existing content was not found in the chunk index" error the other two raised for an absent chunk. A chunk the index does not reach has had no write land in it, so the fill value is its content; that is what write_slice already assumes, and it is what lets extend-then-append flush without a spurious error. --- CHANGELOG.md | 10 +++++ src/dataset.rs | 85 ++++++++++++++++++++++++++--------------- src/io/writer.rs | 99 ++++++++++++++++++++++++++++++++---------------- 3 files changed, 130 insertions(+), 64 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 08b2b73..278b275 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,16 @@ ### Fixed +- Appending after reopening a file no longer erases the rows already in the + chunk the new frame lands in (issue #3). An append that leaves its chunk + partial is buffered until close, and the flush built a fresh fill-value + chunk around the buffered frame instead of reading what the chunk already + held — so `append(&[1, 2, 3])`, close, reopen, `append(&[4, 5, 6])` read + back as `[0, 0, 0, 4, 5, 6]`. The same file written in one session was + correct, because the in-session append path did read-modify-write it. + All three append entry points now place their frames through one owner + that preserves everything outside the span it writes. + - A v2 B-tree chunk index is now written with its records ordered by scaled offsets. libhdf5 searches a B-tree node by bisection (`H5D__bt2_compare` orders records with `H5VM_vector_cmp_u`), and records were appended in diff --git a/src/dataset.rs b/src/dataset.rs index 18fb4ea..c1152ef 100644 --- a/src/dataset.rs +++ b/src/dataset.rs @@ -1626,38 +1626,15 @@ impl H5Dataset { let frames_to_fill = chunk_dim0 - (abs_frame % chunk_dim0); if remaining_frames >= frames_to_fill { - // Full chunk — write + // These frames complete the chunk's remaining span. let end = byte_pos + frames_to_fill * frame_bytes; - if frames_to_fill == chunk_dim0 { - writer.write_chunk( - ds_index, - chunk_idx as u64, - &combined[byte_pos..end], - )?; - } else { - // Partial-chunk write: this branch only runs with - // offset_in_chunk > 0, meaning the chunk already - // holds earlier frames on disk. Read-modify-write - // so those frames survive — a fresh fill buffer - // would erase them. - let offset_in_chunk = (abs_frame % chunk_dim0) * frame_bytes; - let mut chunk_buf = - match writer.read_chunk_if_present(ds_index, chunk_idx as u64)? { - Some(existing) => existing, - None => { - return Err(Hdf5Error::InvalidState(format!( - "cannot append into partially-written chunk {}: \ - its existing content was not found in the chunk \ - index (the file may be inconsistent)", - chunk_idx - ))); - } - }; - chunk_buf - [offset_in_chunk..offset_in_chunk + frames_to_fill * frame_bytes] - .copy_from_slice(&combined[byte_pos..end]); - writer.write_chunk(ds_index, chunk_idx as u64, &chunk_buf)?; - } + let offset_in_chunk = (abs_frame % chunk_dim0) * frame_bytes; + writer.append_frames_into_chunk( + ds_index, + chunk_idx as u64, + offset_in_chunk, + &combined[byte_pos..end], + )?; byte_pos = end; frame_pos += frames_to_fill; } else { @@ -3418,6 +3395,52 @@ mod tests { std::fs::remove_file(&path).ok(); } + /// An append that leaves its chunk partial is buffered until close, and + /// the flush has to keep the frames that chunk already holds. It built a + /// fresh fill-value chunk around the buffered frame instead, so reopening + /// a file and appending one row erased every earlier row of that chunk + /// (issue #3). Four sessions: the second lands beside an existing row, the + /// third closes chunk 0 and opens chunk 1, the fourth lands beside the row + /// the third left in chunk 1. + #[test] + fn append_after_reopen_keeps_the_partial_chunk_it_lands_in() { + let path = temp_path("append_reopen_partial"); + + { + let file = H5File::create(&path).unwrap(); + let ds = file + .new_dataset::() + .shape([0, 3]) + .chunk(&[4, 3]) + .max_shape(&[None, Some(3)]) + .create("values") + .unwrap(); + ds.append(&[1, 2, 3]).unwrap(); + file.close().unwrap(); + } + for rows in [ + vec![4, 5, 6], + vec![7, 8, 9, 10, 11, 12, 13, 14, 15], + vec![16, 17, 18], + ] { + let file = H5File::open_rw(&path).unwrap(); + file.dataset_writer("values") + .unwrap() + .append(&rows) + .unwrap(); + file.close().unwrap(); + } + + let file = H5File::open(&path).unwrap(); + let ds = file.dataset("values").unwrap(); + assert_eq!(ds.shape(), vec![6, 3]); + assert_eq!( + ds.read_raw::().unwrap(), + (1..=18).collect::>() + ); + std::fs::remove_file(&path).ok(); + } + #[cfg(feature = "deflate")] #[test] fn vlen_append_after_reopen_filtered() { diff --git a/src/io/writer.rs b/src/io/writer.rs index 3bb4fc9..de5b34f 100644 --- a/src/io/writer.rs +++ b/src/io/writer.rs @@ -2712,30 +2712,13 @@ impl Hdf5Writer { if remaining_frames >= frames_to_fill { let end = byte_pos + frames_to_fill * frame_bytes; - if frames_to_fill == chunk_dim0 { - self.write_chunk(ds_index, chunk_idx as u64, &combined[byte_pos..end])?; - } else { - // Partial-chunk write: this branch only runs with - // offset_in_chunk > 0, meaning the chunk already holds - // earlier frames on disk. Read-modify-write so those - // frames survive — a fresh fill buffer would erase them. - let offset_in_chunk = (abs_frame % chunk_dim0) * frame_bytes; - let mut chunk_buf = - match self.read_chunk_if_present(ds_index, chunk_idx as u64)? { - Some(existing) => existing, - None => { - return Err(crate::io::IoError::InvalidState(format!( - "cannot append into partially-written chunk {}: its \ - existing content was not found in the chunk index \ - (the file may be inconsistent)", - chunk_idx - ))); - } - }; - chunk_buf[offset_in_chunk..offset_in_chunk + frames_to_fill * frame_bytes] - .copy_from_slice(&combined[byte_pos..end]); - self.write_chunk(ds_index, chunk_idx as u64, &chunk_buf)?; - } + let offset_in_chunk = (abs_frame % chunk_dim0) * frame_bytes; + self.append_frames_into_chunk( + ds_index, + chunk_idx as u64, + offset_in_chunk, + &combined[byte_pos..end], + )?; byte_pos = end; frame_pos += frames_to_fill; } else { @@ -2955,15 +2938,66 @@ impl Hdf5Writer { crate::format::messages::fill_value::tiled_fill(chunk_bytes, fv) } + /// Place `frames` in chunk `chunk_idx`, `offset_in_chunk` bytes from its + /// start, keeping every byte outside that span. + /// + /// The single owner of an append's chunk write. A span covering the whole + /// chunk is written straight through; a narrower one is read-modify-write, + /// because the bytes around it belong to frames appended earlier. Building + /// a fresh fill-value buffer for a chunk that already holds frames is what + /// erased them. A chunk the index does not reach has had no write land in + /// it, so the fill value *is* its content — the rule + /// [`write_slice`](Self::write_slice) already applies to a partially + /// covered chunk. + pub(crate) fn append_frames_into_chunk( + &self, + ds_index: usize, + chunk_idx: u64, + offset_in_chunk: usize, + frames: &[u8], + ) -> IoResult<()> { + let chunk_bytes = { + let ds = self.ds(ds_index); + let m = ds.lock(); + let chunked = m + .chunked + .as_ref() + .ok_or_else(|| crate::io::IoError::InvalidState("not a chunked dataset".into()))?; + chunked.chunk_dims.iter().product::() as usize * m.datatype.element_size() as usize + }; + if offset_in_chunk + frames.len() > chunk_bytes { + return Err(crate::io::IoError::InvalidState(format!( + "{} bytes at offset {offset_in_chunk} do not fit chunk {chunk_idx}, \ + which is {chunk_bytes} bytes", + frames.len() + ))); + } + if offset_in_chunk == 0 && frames.len() == chunk_bytes { + return self.write_chunk(ds_index, chunk_idx, frames); + } + let mut chunk_buf = match self.read_chunk_if_present(ds_index, chunk_idx)? { + Some(existing) if existing.len() == chunk_bytes => existing, + Some(existing) => { + return Err(crate::io::IoError::InvalidState(format!( + "stored chunk {chunk_idx} is {} bytes but the chunk shape needs \ + {chunk_bytes}", + existing.len() + ))) + } + None => self.new_chunk_buffer(ds_index, chunk_bytes), + }; + chunk_buf[offset_in_chunk..offset_in_chunk + frames.len()].copy_from_slice(frames); + self.write_chunk(ds_index, chunk_idx, &chunk_buf) + } + /// Read an already-written chunk's *decompressed* bytes when the chunk /// is allocated and resolvable from the in-memory extensible-array /// index. Handles index-block and data-block chunks, filtered and /// unfiltered. /// /// Returns `Ok(None)` only when the chunk has never been written - /// (address `UNDEF`) or the index genuinely does not reach it. A - /// caller doing a read-modify-write of a partial chunk treats `None` - /// as an error rather than silently overwriting the chunk. + /// (address `UNDEF`) or the index genuinely does not reach it, which for + /// a read-modify-write means the chunk's content is the fill value. pub(crate) fn read_chunk_if_present( &self, ds_index: usize, @@ -4587,8 +4621,10 @@ impl Hdf5Writer { // Internal helpers // ------------------------------------------------------------------ - /// Flush any partial append buffers, padding each chunk's unwritten - /// tail with the dataset's fill value (zeros when none is defined). + /// Flush any partial append buffers into the chunks they belong to, + /// through [`append_frames_into_chunk`](Self::append_frames_into_chunk): + /// frames already in the chunk survive, and the rest of it reads back as + /// the dataset's fill value (zeros when none is defined). fn flush_append_buffers(&mut self) -> IoResult<()> { for i in 0..self.dataset_count() { // Snapshot everything needed under one brief slot guard, then drop @@ -4616,21 +4652,18 @@ impl Hdf5Writer { (buf, buffered_frames, chunk_dims, es, dims) }; - let chunk_bytes: usize = chunk_dims.iter().map(|&d| d as usize).product::() * es; let chunk_dim0 = chunk_dims[0] as usize; let current_dim0 = dims[0] as usize; let base_frame = current_dim0 - buffered_frames; let chunk_idx = base_frame / chunk_dim0; - let mut chunk_buf = self.new_chunk_buffer(i, chunk_bytes); let frame_bytes = if dims.len() > 1 { dims[1..].iter().map(|&d| d as usize).product::() * es } else { es }; let offset_in_chunk = (base_frame % chunk_dim0) * frame_bytes; - chunk_buf[offset_in_chunk..offset_in_chunk + buf.len()].copy_from_slice(&buf); - self.write_chunk(i, chunk_idx as u64, &chunk_buf)?; + self.append_frames_into_chunk(i, chunk_idx as u64, offset_in_chunk, &buf)?; } Ok(()) } From 64468ab4de710135b573533d8fdb02d61a6f0b5b Mon Sep 17 00:00:00 2001 From: Sang Woo Kim Date: Thu, 13 Aug 2026 01:44:16 +0900 Subject: [PATCH 20/20] Release 0.4.0: chunked hyperslab writes and a real v2 B-tree index Minor rather than patch: giving the v2 B-tree a multi-node shape removed Bt2ChunkIndex::encode and BT2_FILT_CHUNK_SIZE_LEN and added a chunk-size-width argument to Bt2ChunkIndex::new_filtered and Bt2Header::new_for_filtered_chunks, all reachable through the public format::chunk_index::btree_v2 path. A 0.3.3 would have reached those callers as a compatible upgrade. The README's dependency snippets still named 0.2. --- CHANGELOG.md | 19 ++++++++++++++++++- Cargo.toml | 2 +- README.md | 4 ++-- 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 278b275..ffd6d29 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,23 @@ # Changelog -## Unreleased +## 0.4.0 + +### Changed + +Breaking, all in `format::chunk_index::btree_v2`, from giving the v2 B-tree a +real multi-node shape and a derived filtered size-field width: + +- `Bt2ChunkIndex::encode` is gone. It returned one header image and one leaf + image, which cannot describe a tree deeper than a single node. Build the + tree with `Bt2ChunkIndex::build_tree`, then `Bt2Tree::encode` for the node + images and `Bt2Tree::header` for the header. +- `Bt2ChunkIndex::new_filtered` and `Bt2Header::new_for_filtered_chunks` take + the chunk-size field width as a third argument. It is derived from the chunk + size (`compute_chunk_size_len`) because that is what libhdf5 recomputes when + it reads a version-4 layout message; a fixed width produced records libhdf5 + misparsed. +- `BT2_FILT_CHUNK_SIZE_LEN` is gone for the same reason — the width is derived + per dataset, never a constant. ### Added diff --git a/Cargo.toml b/Cargo.toml index 69025e3..859bd02 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "rust-hdf5" description = "Pure Rust HDF5 library with full read/write and SWMR support" -version = "0.3.2" +version = "0.4.0" edition = "2021" rust-version = "1.89" license = "MIT" diff --git a/README.md b/README.md index 30d66e6..aa2282d 100644 --- a/README.md +++ b/README.md @@ -34,7 +34,7 @@ Read and write HDF5 files with contiguous, chunked, and compressed datasets, hie ```toml [dependencies] -rust-hdf5 = "0.2" +rust-hdf5 = "0.4" ``` > Requires Rust 1.89+ (uses `std::fs::File::lock` for cross-platform @@ -268,7 +268,7 @@ for ensuring no second writer attaches during streaming. ```toml # Enable LZ4 + Zstandard [dependencies] -rust-hdf5 = { version = "0.2", features = ["lz4", "zstd"] } +rust-hdf5 = { version = "0.4", features = ["lz4", "zstd"] } ``` Filters apply to every chunked layout, whichever chunk index the dataspace