diff --git a/CHANGELOG.md b/CHANGELOG.md index 1949400..6b264f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,10 +63,20 @@ insertion. The heap is changed as `H5HF` changes it — best-fit free sections from its free-space manager (kept as libhdf5 keeps `FSHD`/ `FSSE`), new direct blocks through the root indirect block (created, - doubled), huge objects through the huge-object B-tree (deleted with the - last huge object), removed objects' space merged back — with libhdf5's - statistics: after the same attribute workload the heap, its free space - and both index B-trees equal libhdf5's. Attributes are encoded as libhdf5 + doubled), blocks too small for an attribute skipped as libhdf5 skips + them (`H5HF__hdr_skip_blocks`: an indirect free section with its row + sections, serialized as libhdf5 serializes them, merged with the range + skipped just before it, and later attributes given skipped blocks from + either end or the middle of a range, which splits it), huge objects + through the huge-object B-tree (deleted with the last huge object), + removed objects' space merged back — with libhdf5's statistics: after + the same attribute workload the heap, its free space and both index + B-trees equal libhdf5's (`dense_skipped_blocks_match_libhdf5` covers + every way of skipping, with libhdf5 doing one edit per session as the + editor does). In a random attribute workload (1-4 KiB attributes among + small ones) 24% of `set_attr` calls were refused before skipping was + implemented; 2.2% are now, all replacements of the last attribute in a + heap block. Attributes are encoded as libhdf5 encodes them for a file h5py opens `r+` (message version 1, 3 for non-ASCII names; simple dataspaces with their maximum dimensions). Still refused: see `docs/known-issues.md`. diff --git a/crates/clawhdf5-tools/tests/edit_coverage_interop.rs b/crates/clawhdf5-tools/tests/edit_coverage_interop.rs index fff256c..c2da7cf 100644 --- a/crates/clawhdf5-tools/tests/edit_coverage_interop.rs +++ b/crates/clawhdf5-tools/tests/edit_coverage_interop.rs @@ -1230,10 +1230,11 @@ fn dense_attributes_on_clawhdf5_files() { let mut ed = FileEditor::open(&path).unwrap(); for i in 0..30u64 { for (k, o) in ["/", "d"].iter().enumerate() { - // Small attributes: a larger one needs a heap block bigger than - // the next (see dense_attribute_refusals_change_nothing). + // Strings up to 300 bytes and of 5000 (huge objects), and + // every tenth one of 1-4 KiB (a heap block larger than the + // next: blocks skipped). let v = match AV::make(i, k as u64 + 7) { - AV::Str(s) if s.len() > 400 => AV::Str(s[..400].to_string()), + AV::Str(s) if i % 10 == 3 => AV::Str(s.repeat(3000 / s.len().max(1) + 1)), v => v, }; ed.set_attr(o, &format!("n{i}"), &v.value()) @@ -1323,10 +1324,12 @@ fn check_root_and_attrs(path: &Path, want: &[AttrOp]) { assert!(o.status.success(), "attribute check failed:\n{}", text(&o)); } -/// What the editor refuses in dense storage — an object larger than the -/// next heap block (libhdf5 would skip blocks and record their space as -/// free, which this editor does not do) — is `Error::Unsupported`, and the -/// file is left byte for byte as it was. +/// An attribute that needs a heap block larger than the next one: libhdf5 +/// skips the smaller blocks (`H5HF__hdr_skip_blocks`) and records them as +/// an indirect free section, and so does the editor — the heap and its +/// free space come out as libhdf5's. Replacing that attribute, the only +/// object in its block, with one of another size is still refused (libhdf5 +/// frees the block), and the refusal leaves the file byte-identical. #[test] fn dense_attribute_refusals_change_nothing() { if !tools_ok() { @@ -1334,16 +1337,29 @@ fn dense_attribute_refusals_change_nothing() { } let dir = tmpdir(); let path = dir.path().join("dense_refuse.h5"); - py(&format!( - "import h5py, numpy as np\n\ - with h5py.File({p:?}, 'w', libver='v110') as f:\n\ - \x20 g = f.create_group('g')\n\ - \x20 for i in range(12): g.attrs[f'k{{i}}'] = i\n", - p = path.to_str().unwrap() - )); + let twin = dir.path().join("dense_refuse_h5py.h5"); + for p in [&path, &twin] { + py(&format!( + "import h5py, numpy as np\n\ + with h5py.File({p:?}, 'w', libver='v110') as f:\n\ + \x20 g = f.create_group('g')\n\ + \x20 for i in range(12): g.attrs[f'k{{i}}'] = i\n", + p = p.to_str().unwrap() + )); + } + let big = AV::Str("x".repeat(2000)); + let mut ed = FileEditor::open(&path).unwrap(); + ed.set_attr("g", "big", &big.value()).unwrap(); + drop(ed); + py_r_plus( + &twin, + &[format!("\x20 f['g'].attrs.create('big', {})\n", big.py())], + ); + assert_eq!(dense_info(&path, "g"), dense_info(&twin, "g")); + check_tools(&path, true); let before = std::fs::read(&path).unwrap(); let mut ed = FileEditor::open(&path).unwrap(); - unsupported(ed.set_attr("g", "big", &clawhdf5::AttrValue::String("x".repeat(2000)))); + unsupported(ed.set_attr("g", "big", &clawhdf5::AttrValue::String("y".repeat(1500)))); drop(ed); assert!( std::fs::read(&path).unwrap() == before, @@ -1353,13 +1369,155 @@ fn dense_attribute_refusals_change_nothing() { py(&format!( "import h5py\n\ with h5py.File({p:?}, 'r+') as f:\n\ - \x20 f['g'].attrs['big'] = 'x' * 2000\n\ + \x20 f['g'].attrs['big'] = 'y' * 1500\n\ \x20 assert len(f['g'].attrs) == 13\n", p = path.to_str().unwrap() )); check_tools(&path, true); } +/// Attributes of every size up to the heap's 4 KiB managed limit, in an +/// order that makes libhdf5 skip heap blocks in every way it does: a first +/// attribute too large for the heap's starting block (at the move to dense +/// storage), a block larger than the rest of the current row, a root +/// indirect block doubled past rows, two skipped ranges merged; later small +/// attributes go into the skipped blocks (from either end of a skipped +/// range, and from its middle, which splits it). libhdf5 does the same +/// operations, one file session each (as the editor re-reads the heap for +/// each edit); after every phase the heaps, their free sections (single, +/// row and indirect) and index B-trees must be libhdf5's. +#[test] +fn dense_skipped_blocks_match_libhdf5() { + if !tools_ok() { + return; + } + for (libver, h5dump) in [("'earliest'", true), ("'v110'", true), ("'latest'", false)] { + let dir = tmpdir(); + let a = dir.path().join("skip_h5py.h5"); + let b = dir.path().join("skip_edit.h5"); + for p in [&a, &b] { + py(&format!( + "import h5py, numpy as np\n\ + with h5py.File({p:?}, 'w', libver={libver}) as f:\n\ + \x20 objs = [f.create_group('g'), f.create_group('t', track_order=True), \ + f.create_dataset('d', data=np.arange(4, dtype=' = Vec::new(); + for (k, o) in objs.iter().enumerate() { + want.push(( + o.to_string(), + "c0".into(), + AV::Str("s".repeat(1200 + 700 * k)), + )); + for i in 1..8i64 { + want.push((o.to_string(), format!("c{i}"), AV::Ints(vec![i]))); + } + } + for i in 0..8i64 { + want.push(("h".into(), format!("c{i}"), AV::Ints(vec![i]))); + } + // "h" starts with small attributes, so its heap has a root direct + // block and then a one-row root indirect block when the first + // large attribute comes: the rest of that row and the rows the + // doubling adds are skipped as two ranges, which merge; the next + // attributes take blocks from the start, the end and the middle + // (splitting it) of the merged range, and use up whole rows. + let h_phases: [&[usize]; 4] = [ + &[0; 20], + &[4060, 1500, 0, 3000, 0, 1500, 0, 600], + &[1000, 0, 700, 3000, 0, 0, 1500, 0, 900, 0, 0, 0, 1800], + &[0, 800, 0, 0, 0, 2000, 0, 0, 0, 0, 0, 0, 3500, 0, 0, 0, 0, 0], + ]; + // Sizes (string lengths, or 0 for a small int array) in phases. + let phases: [&[usize]; 4] = [ + &[0, 3000, 0, 700, 3900, 0, 150], + &[2500, 90, 0, 3500, 1800, 60, 3990, 0], + &[0, 0, 40, 300, 0, 900, 20, 0, 0, 500, 0, 30, 1000, 0, 0, 200], + &[0, 250, 0, 40, 600, 0, 0, 1100, 80, 0, 0, 350, 0, 0, 0, 10], + ]; + let mut n = 0; + let value = |len: usize, n: usize, k: usize| { + if len == 0 { + AV::Ints((0..1 + (n + k) as i64 % 7).collect()) + } else { + AV::Str( + (0..len + 11 * k) + .map(|j| (b'a' + ((j + n) % 26) as u8) as char) + .collect(), + ) + } + }; + for (ph, sizes) in phases.iter().enumerate() { + let mut ops: Vec = Vec::new(); + for &len in *sizes { + for (k, o) in objs.iter().enumerate() { + ops.push((o.to_string(), format!("n{n}"), value(len, n, k))); + } + n += 1; + } + for &len in h_phases[ph] { + ops.push(("h".into(), format!("n{n}"), value(len, n, 0))); + n += 1; + } + // libhdf5: one session per operation. + let lines: Vec = ops + .iter() + .map(|(o, nm, v)| { + format!( + "with h5py.File({p:?}, 'r+') as f: f[{o:?}].attrs.create({nm:?}, {})\n", + v.py(), + p = a.to_str().unwrap() + ) + }) + .collect(); + let sp = a.with_extension(format!("ops{ph}.py")); + std::fs::write(&sp, format!("import h5py, numpy as np\n{}", lines.concat())).unwrap(); + let out = Command::new(python()).arg(&sp).output().unwrap(); + assert!( + out.status.success(), + "h5py workload failed:\n{}", + text(&out) + ); + let mut ed = FileEditor::open(&b).unwrap(); + for (o, nm, v) in &ops { + ed.set_attr(o, nm, &v.value()) + .unwrap_or_else(|e| panic!("{libver} phase {ph}: set {o}/{nm}: {e}")); + set_want(&mut want, o, nm, v.clone()); + } + drop(ed); + for o in objs.iter().chain(&["h"]) { + assert_eq!( + dense_info(&b, o), + dense_info(&a, o), + "{libver}: dense storage of {o} differs from libhdf5's after phase {ph}" + ); + } + check_tools(&b, h5dump); + check_attr_values(&b, &want); + } + // libhdf5 goes on with the editor's file. + py(&format!( + "import h5py, numpy as np\n\ + with h5py.File({p:?}, 'r+') as f:\n\ + \x20 for o in ['g', 't', 'd', 'h']:\n\ + \x20 for i in range(6): f[o].attrs[f'late{{i}}'] = 'z' * (i * 700 + 5)\n\ + \x20 del f[o].attrs['c1']\n", + p = b.to_str().unwrap() + )); + want.retain(|(_, nm, _)| nm != "c1"); + check_tools(&b, h5dump); + check_attr_values(&b, &want); + } +} + /// Space one edit frees is reused by later edits of the same editor: the /// chunks a shrink removes are where the chunks of the following growth /// go, so the file does not grow; with a new editor per edit (nothing to diff --git a/crates/clawhdf5/src/edit/fheap.rs b/crates/clawhdf5/src/edit/fheap.rs index 03e6b1e..dfd3af5 100644 --- a/crates/clawhdf5/src/edit/fheap.rs +++ b/crates/clawhdf5/src/edit/fheap.rs @@ -3,23 +3,29 @@ //! best-fitting free section the heap's free-space manager records (a new //! direct block when none fits: the root direct block of an empty heap, the //! next block of the root indirect block — created from the root direct -//! block, and doubled, as needed), huge objects (larger than the heap's +//! block, and doubled, as needed — with the blocks too small for the object +//! skipped and kept as free space for later objects), huge objects (larger than the heap's //! managed maximum) into their own file space tracked by the huge-object //! version-2 B-tree; removed objects return their space to the free-space //! manager, merged with adjacent free space. //! //! The heap's free-space manager (`FSHD` header, `FSSE` section info) is -//! kept exactly as libhdf5 keeps it: sections sorted by size then offset, +//! kept exactly as libhdf5 keeps it: single sections and the indirect +//! sections of skipped blocks (with their first and normal row sections), +//! sections sorted by size then offset, //! the header's counts and sizes, section info moved when its size changes, //! the manager deleted when it tracks nothing. Header statistics (managed //! space, allocated space, free space, allocation iterator, object counts) //! follow libhdf5's arithmetic. //! +//! The heap is read afresh for each edit, so the editor does what libhdf5 +//! does with one edit per file session (libhdf5 keeps an in-memory span per +//! indirect section that it recomputes when it reloads the section info). +//! //! What libhdf5 would do differently is refused ([`Error::Unsupported`], //! before anything is written): I/O filters on the heap, child indirect -//! blocks, skipped blocks (a first object too large for the next block), -//! free sections other than "single" ones, freeing a whole direct block, -//! directly addressed huge objects, tiny objects. +//! blocks (and free space in them), freeing a whole direct block, directly +//! addressed huge objects, tiny objects. use std::collections::{BTreeMap, BTreeSet}; @@ -67,8 +73,129 @@ const FS_NCLASSES: u16 = 4; const HUGE_BT2_TYPE: u8 = 1; const HUGE_BT2_NODE: u32 = 512; -/// The heap's free-space manager: only "single" sections (free space inside -/// a direct block), keyed by heap offset. +/// The heap's doubling table as its free sections need it: the block size +/// of each direct row, the direct block overhead, the width and the size +/// of a heap offset. +#[derive(Clone, Debug)] +struct Geo { + width: usize, + /// Block size of rows 0 .. max direct rows. + rblock: Vec, + /// `H5HF_MAN_ABS_DIRECT_OVERHEAD`. + ov: u64, + heap_off_size: usize, +} + +impl Geo { + /// Free space in a block of `row` (a row section's size). + fn rfree(&self, row: usize) -> u64 { + self.rblock[row] - self.ov + } + + /// Heap offset of `row`'s first block in the root indirect block. + fn row_off(&self, row: usize) -> u64 { + (0..row).map(|r| self.rblock[r] * self.width as u64).sum() + } + + /// `H5HF__dtable_span_size`. + fn span(&self, row: usize, col: usize, n: usize) -> u64 { + let w = self.width; + let end = row * w + col + n - 1; + let (end_row, end_col) = (end / w, end % w); + if row == end_row { + return self.rblock[row] * (end_col - col + 1) as u64; + } + let mut r = row; + let mut acc = 0; + if col > 0 { + acc = self.rblock[r] * (w - col) as u64; + r += 1; + } + while r < end_row { + acc += self.rblock[r] * w as u64; + r += 1; + } + acc + self.rblock[r] * (end_col + 1) as u64 + } +} + +/// A row section (`H5HF_FSPACE_SECT_FIRST_ROW` / `NORMAL_ROW`): `n` +/// unallocated direct blocks of one row of the root indirect block, from +/// column `col`; its size is one block's free space. +#[derive(Clone, Debug)] +struct RowSect { + addr: u64, + row: usize, + col: usize, + n: usize, +} + +/// An indirect section (`H5HF_FSPACE_SECT_INDIRECT`): `n` unallocated +/// entries of the root indirect block from (`row`, `col`) — blocks skipped +/// over to allocate a larger one (`H5HF__hdr_skip_blocks`). It is never in +/// the free-space manager itself; its row sections are, the first as the +/// serialized "first row" (which stands for the indirect section on disk), +/// the others as unserialized "ghost" normal rows. `span` is kept as +/// libhdf5 keeps it (reducing a row subtracts the row section's size, not +/// the block's), since merging compares it. +#[derive(Clone, Debug)] +struct Ind { + addr: u64, + row: usize, + col: usize, + n: usize, + span: u64, + rows: Vec, +} + +impl Ind { + /// `H5HF__sect_indirect_new` + `H5HF__sect_indirect_init_rows` for the + /// root indirect block (direct rows only). + fn new(geo: &Geo, row: usize, col: usize, n: usize) -> Self { + let w = geo.width; + let end = row * w + col + n - 1; + let (end_row, end_col) = (end / w, end % w); + let addr = geo.row_off(row) + geo.rblock[row] * col as u64; + let mut rows = Vec::new(); + let mut entries = if row == end_row { + end_col - col + 1 + } else { + w - col + }; + let mut row_col = col; + let mut off = addr; + for u in row..=end_row { + rows.push(RowSect { + addr: off, + row: u, + col: row_col, + n: entries, + }); + off += entries as u64 * geo.rblock[u]; + entries = if u + 1 < end_row { w } else { end_col + 1 }; + row_col = 0; + } + Self { + addr, + row, + col, + n, + span: geo.span(row, col, n), + rows, + } + } +} + +/// A free section `find` chose. +enum Found { + Single(u64, u64), + /// Row `.1` of indirect section `.0`. + Row(usize, usize), +} + +/// The heap's free-space manager: "single" sections (free space inside a +/// direct block) keyed by heap offset, and the indirect sections (with +/// their row sections) of skipped direct blocks. struct FreeSpace { addr: u64, max_sect_addr: u16, @@ -80,6 +207,8 @@ struct FreeSpace { sect_size: u64, alloc_sect_size: u64, sects: BTreeMap, + inds: Vec, + geo: Geo, } impl FreeSpace { @@ -87,7 +216,7 @@ impl FreeSpace { 6 + 4 * ls as usize + 8 + ls as usize + os as usize + 2 * ls as usize + 4 } - fn open(img: &Image<'_>, addr: u64) -> Result { + fn open(img: &Image<'_>, addr: u64, geo: &Geo) -> Result { let (os, ls) = (img.os, img.ls); let len = Self::hdr_len(os, ls); let d = img.read(addr, len)?; @@ -120,9 +249,6 @@ impl FreeSpace { let sect_addr = next(os as usize); let sect_size = next(l); let alloc_sect_size = next(l); - if ghost != 0 || tot_count != serial { - return Err(unsupported("free space in row or indirect sections")); - } let mut fs = Self { addr, max_sect_addr, @@ -134,8 +260,13 @@ impl FreeSpace { sect_size, alloc_sect_size, sects: BTreeMap::new(), + inds: Vec::new(), + geo: geo.clone(), }; if serial == 0 { + if tot_count != 0 || ghost != 0 || tot_space != 0 { + return Err(bad("free-space counts disagree")); + } return Ok(fs); } if sect_addr == undef(os) || sect_size < 9 + os as u64 || sect_size > alloc_sect_size { @@ -154,9 +285,10 @@ impl FreeSpace { let cnt_w = enc_size(serial); let len_w = enc_size(max_sect_size); let off_w = (usize::from(max_sect_addr)).div_ceil(8); + let ind_w = geo.heap_off_size + 6; let mut q = 5 + os as usize; let mut seen = 0u64; - let mut total = 0u64; + let mut addrs = BTreeSet::new(); while seen < serial { if q + cnt_w + len_w > n - 4 { return Err(bad("section info ends early")); @@ -165,8 +297,8 @@ impl FreeSpace { q += cnt_w; let size = le(&s[q..q + len_w]); q += len_w; - if count == 0 || size == 0 { - return Err(bad("empty section size node")); + if count == 0 || size == 0 || count > serial - seen { + return Err(bad("bad section size node")); } for _ in 0..count { if q + off_w + 1 > n - 4 { @@ -175,51 +307,141 @@ impl FreeSpace { let off = le(&s[q..q + off_w]); let class = s[q + off_w]; q += off_w + 1; - if class != 0 { - return Err(unsupported("free space in row or indirect sections")); + match class { + 0 => { + if fs.sects.insert(off, size).is_some() { + return Err(bad("duplicate free section")); + } + } + 1 => { + // H5HF__sect_indirect_deserialize. + if q + ind_w > n - 4 { + return Err(bad("section info ends early")); + } + let iblock_off = le(&s[q..q + geo.heap_off_size]); + let b = &s[q + geo.heap_off_size..]; + let row = usize::from(u16::from_le_bytes([b[0], b[1]])); + let col = usize::from(u16::from_le_bytes([b[2], b[3]])); + let cnt = usize::from(u16::from_le_bytes([b[4], b[5]])); + q += ind_w; + if iblock_off != 0 { + return Err(unsupported("free space in child indirect blocks")); + } + if cnt == 0 || col >= geo.width { + return Err(bad("bad indirect free section")); + } + let end_row = (row * geo.width + col + cnt - 1) / geo.width; + if end_row >= geo.rblock.len() { + return Err(unsupported("free space in child indirect blocks")); + } + let ind = Ind::new(geo, row, col, cnt); + if ind.addr != off || geo.rfree(row) != size { + return Err(bad("bad indirect free section")); + } + fs.inds.push(ind); + } + _ => return Err(bad("free section of an unknown class")), } - if fs.sects.insert(off, size).is_some() { + if !addrs.insert(off) { return Err(bad("duplicate free section")); } seen += 1; - total += size; } } - if total != tot_space { - return Err(bad("free-space total disagrees with its sections")); + let (space, total, ser, gh) = fs.totals(); + if (space, total, ser, gh) != (tot_space, tot_count, serial, ghost) { + return Err(bad("free-space totals disagree with its sections")); } Ok(fs) } + /// Every row section: (address, size, indirect section, row). + fn rows(&self) -> impl Iterator + '_ { + self.inds.iter().enumerate().flat_map(move |(i, ind)| { + ind.rows + .iter() + .enumerate() + .map(move |(r, rs)| (rs.addr, self.geo.rfree(rs.row), i, r)) + }) + } + + /// (total space, sections, serialized sections, ghost sections). + fn totals(&self) -> (u64, u64, u64, u64) { + let nrows: u64 = self.inds.iter().map(|i| i.rows.len() as u64).sum(); + let space = self.sects.values().sum::() + self.rows().map(|r| r.1).sum::(); + let singles = self.sects.len() as u64; + let firsts = self.inds.len() as u64; + (space, singles + nrows, singles + firsts, nrows - firsts) + } + + fn is_empty(&self) -> bool { + self.sects.is_empty() && self.inds.is_empty() + } + + /// The offset of the section nearest `addr` below it (or above it) in + /// the merge list — singles and first rows, by offset — when that + /// section is a first row, the only kind a first row merges with. + fn merge_neighbour(&self, addr: u64, below: bool) -> Option { + let firsts = self.inds.iter().map(|i| (i.addr, true)); + let singles = self.sects.keys().map(|&o| (o, false)); + let all = firsts.chain(singles); + let pick = if below { + all.filter(|e| e.0 < addr).max_by_key(|e| e.0) + } else { + all.filter(|e| e.0 > addr).min_by_key(|e| e.0) + }; + pick.and_then(|(a, first)| first.then_some(a)) + } + /// Best fit (`H5FS__sect_find_node`): the smallest section of at least - /// `size` bytes, the lowest offset among equals. - fn find(&self, size: u64) -> Option<(u64, u64)> { - self.sects + /// `size` bytes, the lowest offset among equals, singles and row + /// sections alike. + fn find(&self, size: u64) -> Option { + let single = self + .sects .iter() .filter(|&(_, &s)| s >= size) - .min_by_key(|&(&o, &s)| (s, o)) - .map(|(&o, &s)| (o, s)) + .map(|(&o, &s)| ((s, o), Found::Single(o, s))); + let rows = self + .rows() + .filter(|r| r.1 >= size) + .map(|(a, s, i, r)| ((s, a), Found::Row(i, r))); + single.chain(rows).min_by_key(|(k, _)| *k).map(|(_, f)| f) + } + + /// The serialized sections, by size then offset: (size, offset, + /// indirect section if a first row). + fn serial(&self) -> Vec<(u64, u64, Option)> { + let mut v: Vec<(u64, u64, Option)> = + self.sects.iter().map(|(&o, &s)| (s, o, None)).collect(); + v.extend( + self.inds + .iter() + .enumerate() + .map(|(i, ind)| (self.geo.rfree(ind.row), ind.addr, Some(i))), + ); + v.sort_unstable(); + v } /// The serialized section info's size (`H5FS__sect_serialize_size`). fn needed(&self, os: u8) -> u64 { - let n = self.sects.len() as u64; let prefix = 4 + 1 + u64::from(os) + 4; + let serial = self.serial(); + let n = serial.len() as u64; if n == 0 { return prefix; } - let sizes: BTreeSet = self.sects.values().copied().collect(); + let sizes: BTreeSet = serial.iter().map(|s| s.0).collect(); prefix + sizes.len() as u64 * (enc_size(n) + enc_size(self.max_sect_size)) as u64 + n * (u64::from(self.max_sect_addr).div_ceil(8) + 1) + + self.inds.len() as u64 * (self.geo.heap_off_size as u64 + 6) } fn serialize(&self, os: u8) -> Vec { - let n = self.sects.len() as u64; - let mut by_size: BTreeMap> = BTreeMap::new(); - for (&o, &s) in &self.sects { - by_size.entry(s).or_default().push(o); - } + let serial = self.serial(); + let n = serial.len() as u64; let cnt_w = enc_size(n); let len_w = enc_size(self.max_sect_size); let off_w = usize::from(self.max_sect_addr).div_ceil(8); @@ -229,13 +451,28 @@ impl FreeSpace { let mut a = vec![0u8; os as usize]; put_uint(&mut a, self.addr, os); d.extend_from_slice(&a); - for (size, offs) in by_size { - d.extend_from_slice(&(offs.len() as u64).to_le_bytes()[..cnt_w]); + let mut i = 0; + while i < serial.len() { + let size = serial[i].0; + let same = serial[i..].iter().take_while(|s| s.0 == size).count(); + d.extend_from_slice(&(same as u64).to_le_bytes()[..cnt_w]); d.extend_from_slice(&size.to_le_bytes()[..len_w]); - for o in offs { + for &(_, o, ind) in &serial[i..i + same] { d.extend_from_slice(&o.to_le_bytes()[..off_w]); - d.push(0); + match ind { + None => d.push(0), + Some(k) => { + // H5HF__sect_indirect_serialize (root block: offset 0). + let ind = &self.inds[k]; + d.push(1); + d.extend_from_slice(&vec![0u8; self.geo.heap_off_size]); + for v in [ind.row, ind.col, ind.n] { + d.extend_from_slice(&(v as u16).to_le_bytes()); + } + } + } } + i += same; } d } @@ -244,13 +481,12 @@ impl FreeSpace { let (os, ls) = (img.os, img.ls); let len = Self::hdr_len(os, ls); let l = ls as usize; - let n = self.sects.len() as u64; - let tot: u64 = self.sects.values().sum(); + let (tot, n, serial, ghost) = self.totals(); let mut d = Vec::with_capacity(len); d.extend_from_slice(b"FSHD"); d.push(0); d.push(FS_CLIENT_FHEAP); - for v in [tot, n, n, 0] { + for v in [tot, n, serial, ghost] { d.extend_from_slice(&v.to_le_bytes()[..l]); } for v in [self.nclasses, self.shrink, self.expand, self.max_sect_addr] { @@ -440,7 +676,7 @@ impl Heap { } } if fs_addr != undef(os) { - let fs = FreeSpace::open(img, fs_addr)?; + let fs = FreeSpace::open(img, fs_addr, &h.geo(os))?; if fs.max_sect_addr != max_index { return Err(bad("free-space manager does not match the heap")); } @@ -678,10 +914,23 @@ impl Heap { return Err(unsupported("tiny objects")); } let (off, size) = match self.fs.as_ref().and_then(|fs| fs.find(n)) { - Some((o, s)) => { + Some(Found::Single(o, s)) => { self.fs.as_mut().expect("found above").sects.remove(&o); (o, s) } + // H5HF__man_iblock_alloc_row: a skipped block, created now. + Some(Found::Row(i, r)) => { + let entry = self.alloc_row(i, r); + self.fs_dirty = true; + let w = usize::from(self.width); + let (row, col) = (entry / w, entry % w); + let bsize = self.row_size(row); + let block_off = self.row_off(row) + bsize * col as u64; + let a = self.dblock_create(img, block_off, bsize)?; + self.ents[entry] = a; + self.iblock_dirty = true; + (block_off + self.overhead(os), bsize - self.overhead(os)) + } None => self.dblock_new(img, n)?, }; // H5HF__sect_single_reduce: the object goes at the section's start. @@ -705,7 +954,16 @@ impl Heap { /// `H5FS_ADD_RETURNED_SPACE`), creating the free-space manager if the /// heap has none. fn fs_add(&mut self, img: &mut Image<'_>, off: u64, size: u64) -> Result<(), Error> { + self.fs_mut(img)?.sects.insert(off, size); + self.fs_dirty = true; + Ok(()) + } + + /// The free-space manager, created if the heap has none + /// (`H5HF__space_start`). + fn fs_mut(&mut self, img: &mut Image<'_>) -> Result<&mut FreeSpace, Error> { if self.fs.is_none() { + let geo = self.geo(img.os); let addr = img.alloc(FreeSpace::hdr_len(img.os, img.ls) as u64)?; self.fs = Some(FreeSpace { addr, @@ -718,18 +976,160 @@ impl Heap { sect_size: 0, alloc_sect_size: 0, sects: BTreeMap::new(), + inds: Vec::new(), + geo, }); self.fs_addr = addr; } - self.fs - .as_mut() - .expect("created above") - .sects - .insert(off, size); + Ok(self.fs.as_mut().expect("created above")) + } + + /// The doubling table's direct rows, for the free sections. + fn geo(&self, os: u8) -> Geo { + Geo { + width: usize::from(self.width), + rblock: (0..self.max_direct_rows) + .map(|r| self.row_size(r)) + .collect(), + ov: self.overhead(os), + heap_off_size: self.heap_off_size, + } + } + + /// `H5HF__hdr_skip_blocks`: `n` entries of the root indirect block + /// from `start` are skipped — the iterator moves past them and they + /// become an indirect free section (`H5HF__sect_indirect_add`), whose + /// first row is added as returned space, so it merges with an indirect + /// section just below it (`H5FS__sect_merge`). + fn skip_blocks(&mut self, img: &mut Image<'_>, start: usize, n: usize) -> Result<(), Error> { + let w = usize::from(self.width); + let (row, col) = (start / w, start % w); + if (start + n - 1) / w >= self.max_direct_rows { + return Err(unsupported("child indirect blocks")); + } + let geo = self.geo(img.os); + self.man_iter_off += geo.span(row, col, n); + let iter = self.man_iter_off; + let fs = self.fs_mut(img)?; + let mut cur = Ind::new(&geo, row, col, n); + loop { + let mut modified = false; + // The nearest sections below and above in the merge list + // (singles and first rows, by offset). + let below = fs.merge_neighbour(cur.addr, true); + let above = fs.merge_neighbour(cur.addr, false); + if let Some(a) = below + && let Some(k) = fs.inds.iter().position(|x| x.addr == a) + && fs.inds[k].addr + fs.inds[k].span == cur.addr + { + let mut lower = fs.inds.remove(k); + if cur.addr >= iter { + return Err(unsupported("free space past the heap's end")); + } + merge_ind(&mut lower, cur, w); + cur = lower; + modified = true; + } + if let Some(a) = above + && let Some(k) = fs.inds.iter().position(|x| x.addr == a) + && cur.addr + cur.span == a + { + let upper = fs.inds.remove(k); + if upper.addr >= iter { + return Err(unsupported("free space past the heap's end")); + } + merge_ind(&mut cur, upper, w); + modified = true; + } + if !modified { + break; + } + } + // H5HF__sect_row_can_shrink: a section past the iterator would + // shrink the heap; skipped blocks never are. + if cur.addr >= iter { + return Err(unsupported("free space past the heap's end")); + } + fs.inds.push(cur); self.fs_dirty = true; Ok(()) } + /// `H5HF__sect_row_reduce` with `H5HF__sect_indirect_reduce_row`: take + /// one block out of row `r` of indirect section `i`; returns the root + /// indirect block entry to create it at. + fn alloc_row(&mut self, i: usize, r: usize) -> usize { + let w = usize::from(self.width); + let fs = self.fs.as_mut().expect("section found"); + let geo = fs.geo.clone(); + let ind = &mut fs.inds[i]; + let rs = ind.rows[r].clone(); + let row_start = rs.row * w + rs.col; + let row_end = row_start + rs.n - 1; + let start = ind.row * w + ind.col; + let end = start + ind.n - 1; + let (start_row, end_row) = (ind.row, end / w); + let from_start = !(row_end == end && start_row != end_row); + let entry = if from_start { row_start } else { row_end }; + ind.span -= geo.rfree(rs.row); + let mut peer = None; + if ind.n > 1 { + if entry == start { + ind.addr += geo.rblock[ind.row]; + ind.col += 1; + if ind.col == w { + ind.row += 1; + ind.col = 0; + // The row's last block: the row goes (below). + } + ind.n -= 1; + } else if entry == end { + ind.n -= 1; + } else { + // Split: the rows before this one become a peer section. + let peer_n = entry - start; + let peer_rows = rs.row - start_row; + let rest = ind.rows.split_off(peer_rows); + let p = Ind { + addr: ind.addr, + row: ind.row, + col: ind.col, + n: peer_n, + span: rs.addr - ind.addr, + rows: std::mem::replace(&mut ind.rows, rest), + }; + ind.addr = rs.addr + geo.rblock[rs.row]; + ind.span -= p.span; + ind.row = rs.row; + ind.col = rs.col + 1; + ind.n -= peer_n + 1; + peer = Some(p); + } + } else { + ind.n -= 1; + } + // The row section itself. + let ri = if peer.is_some() { 0 } else { r }; + let row = &mut ind.rows[ri]; + if row.n == 1 { + ind.rows.remove(ri); + } else { + if from_start { + row.addr += geo.rblock[row.row]; + row.col += 1; + } + row.n -= 1; + } + let gone = ind.rows.is_empty(); + if gone { + fs.inds.remove(i); + } + if let Some(p) = peer { + fs.inds.push(p); + } + entry + } + /// `H5HF__man_dblock_new`: a direct block for an object of `request` /// bytes; returns its free section (not in the free-space manager). fn dblock_new(&mut self, img: &mut Image<'_>, request: u64) -> Result<(u64, u64), Error> { @@ -753,30 +1153,16 @@ impl Heap { self.total_man_free += self.start_block_size - self.overhead(os); return Ok((self.overhead(os), self.start_block_size - self.overhead(os))); } - if self.root == undef(os) { - return Err(unsupported( - "a first object too large for the starting block", - )); - } - // H5HF__hdr_update_iter. - if self.root_rows == 0 { - self.root_create(img, min)?; - } - let (mut row, mut col) = self.iter_pos(); - let min_row = self.size_to_row(min); - if min_row > row && row < usize::from(self.root_rows) { - return Err(unsupported("skipping blocks too small for an object")); - } - while row >= usize::from(self.root_rows) { - self.root_double(img, min)?; - (row, col) = self.iter_pos(); - } + // H5HF__hdr_update_iter, then a block at the iterator. + self.update_iter(img, min)?; + let (row, col) = self.iter_pos(); if row >= self.max_direct_rows { return Err(unsupported("child indirect blocks")); } let size = self.row_size(row); if min > size { - return Err(unsupported("skipping blocks too small for an object")); + // libhdf5: "skipping direct block sizes not supported". + return Err(unsupported("a direct block smaller than the object")); } self.man_iter_off += size; let entry = row * usize::from(self.width) + col; @@ -787,6 +1173,37 @@ impl Heap { Ok((block_off + self.overhead(os), size - self.overhead(os))) } + /// `H5HF__hdr_update_iter`: make the iterator point at a block of at + /// least `min` bytes, creating the root indirect block, skipping + /// smaller blocks and doubling the root indirect block as needed. + fn update_iter(&mut self, img: &mut Image<'_>, min: u64) -> Result<(), Error> { + if self.root_rows == 0 { + return self.root_create(img, min); + } + let w = usize::from(self.width); + let min_row = self.size_to_row(min); + let (mut row, col) = self.iter_pos(); + let nrows = usize::from(self.root_rows); + if min_row > row && row < nrows { + let entry = row * w + col; + let skip = if min_row >= nrows { + nrows * w - entry + } else { + min_row * w - entry + }; + self.skip_blocks(img, entry, skip)?; + row = self.iter_pos().0; + } + while row >= usize::from(self.root_rows) { + self.root_double(img, min)?; + row = self.iter_pos().0; + } + if row >= self.max_direct_rows { + return Err(unsupported("child indirect blocks")); + } + Ok(()) + } + /// The allocation iterator's (row, column) in the root indirect block. fn iter_pos(&self) -> (usize, usize) { if self.man_iter_off >= self.man_size { @@ -827,8 +1244,9 @@ impl Heap { Ok(a) } - /// `H5HF__man_iblock_root_create`: the root direct block becomes entry - /// 0 of a new root indirect block. + /// `H5HF__man_iblock_root_create`: a root indirect block, with the + /// root direct block (if any) as entry 0; when the block needed is + /// larger than the starting size, the smaller blocks are skipped. fn root_create(&mut self, img: &mut Image<'_>, min: u64) -> Result<(), Error> { let os = img.os; let mut nrows = if self.start_root_rows == 0 { @@ -846,9 +1264,6 @@ impl Heap { if nrows > self.max_direct_rows { return Err(unsupported("child indirect blocks")); } - if min > self.start_block_size { - return Err(unsupported("skipping blocks too small for an object")); - } let have_direct = self.root != undef(os); let rows = u16::try_from(nrows).map_err(|_| bad("too many rows"))?; let a = img.alloc(self.iblock_len(rows, os) as u64)?; @@ -860,43 +1275,57 @@ impl Heap { self.man_iter_off = 0; } self.iblock_dirty = true; - self.root_rows = rows; - self.root = a; let w = u64::from(self.width); let ov = self.overhead(os); let mut acc: u64 = (0..nrows).map(|u| (self.row_size(u) - ov) * w).sum(); if have_direct { acc -= self.row_size(0) - ov; } + // The header points at the new block before the skipped blocks + // are added (the free sections need the rows in place). + self.root_rows = rows; + self.root = a; + if min > self.start_block_size { + let first = usize::from(have_direct); + self.skip_blocks(img, first, (nrows - 1) * usize::from(self.width) - first)?; + } self.man_size = self.row_off(nrows); self.total_man_free += acc; Ok(()) } /// `H5HF__man_iblock_root_double`: the root indirect block gets twice - /// its rows, at a new address. + /// its rows (at least enough for a block of `min` bytes, the smaller + /// blocks before it skipped), at a new address. fn root_double(&mut self, img: &mut Image<'_>, min: u64) -> Result<(), Error> { let os = img.os; + let w = usize::from(self.width); let old = usize::from(self.root_rows); - let (row, _) = self.iter_pos(); - let next_size = self.row_size(row.min(self.max_root_rows - 1)); + let (next_row, next_col) = self.iter_pos(); + let next_entry = next_row * w + next_col; + let next_size = self.row_size(next_row.min(self.max_root_rows - 1)); + let (mut min_nrows, mut new_next_entry, mut skip) = (0, 0, false); if old < self.max_direct_rows && min > next_size { - return Err(unsupported("skipping blocks too small for an object")); + skip = true; + min_nrows = 1 + self.size_to_row(min); + new_next_entry = (min_nrows - 1) * w; } - let new = (2 * old).min(self.max_root_rows); + let new = min_nrows.max((2 * old).min(self.max_root_rows)); if new > self.max_direct_rows || new == old { return Err(unsupported("child indirect blocks")); } img.free(self.root, self.iblock_len(self.root_rows, os) as u64); let rows = u16::try_from(new).map_err(|_| bad("too many rows"))?; let a = img.alloc(self.iblock_len(rows, os) as u64)?; - let w = usize::from(self.width); self.ents.resize(new * w, undef(os)); + self.root = a; + self.iblock_dirty = true; + if skip { + self.skip_blocks(img, next_entry, new_next_entry - next_entry)?; + } let ov = self.overhead(os); let acc: u64 = (old * w..new * w).map(|u| self.row_size(u / w) - ov).sum(); self.root_rows = rows; - self.root = a; - self.iblock_dirty = true; self.man_size = 2 * self.row_off(new - 1); self.total_man_free += acc; Ok(()) @@ -1091,7 +1520,7 @@ impl Heap { self.fs_dirty = false; if let Some(mut fs) = self.fs.take() { let hdr_len = FreeSpace::hdr_len(os, img.ls) as u64; - if fs.sects.is_empty() { + if fs.is_empty() { // H5HF__space_close: a manager that tracks nothing is // deleted. img.free(fs.addr, hdr_len); @@ -1124,6 +1553,24 @@ impl Heap { } } +/// `H5HF__sect_indirect_merge_row` for two sections of the root indirect +/// block, `b` right after `a`: `b`'s rows join `a` (its first row joins +/// `a`'s last row when they share a row). +fn merge_ind(a: &mut Ind, mut b: Ind, w: usize) { + let end_row1 = (a.row * w + a.col + a.n - 1) / w; + if !b.rows.is_empty() { + if end_row1 == b.row { + let first = b.rows.remove(0); + if let Some(last) = a.rows.last_mut() { + last.n += first.n; + } + } + a.rows.append(&mut b.rows); + } + a.n += b.n; + a.span += b.span; +} + /// The ID in a huge-object B-tree record (type 1: address, length, ID). fn huge_rec_id(r: &[u8], os: u8, ls: u8) -> u64 { get_uint(&r[os as usize + ls as usize..], ls) diff --git a/crates/clawhdf5/src/edit/mod.rs b/crates/clawhdf5/src/edit/mod.rs index e1d301a..a5a3c1b 100644 --- a/crates/clawhdf5/src/edit/mod.rs +++ b/crates/clawhdf5/src/edit/mod.rs @@ -958,8 +958,7 @@ impl FileEditor { /// /// [`Error::Unsupported`] for shared attribute messages, heaps this /// editor cannot extend the way libhdf5 would (child indirect blocks, - /// skipped blocks, free space other than within direct blocks, freeing - /// a whole block), and version-1 object headers asked for an attribute + /// freeing a whole block), and version-1 object headers asked for an attribute /// larger than a header message holds. pub fn set_attr(&mut self, path: &str, name: &str, value: &AttrValue) -> Result<(), Error> { if name.is_empty() { diff --git a/docs/known-issues.md b/docs/known-issues.md index a178d47..2539f63 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -74,13 +74,19 @@ reuse were added). `clawhdf5::FileEditor` refuses, with an optional filter only when its own build lacks it, which none does for these; - attributes in dense storage when the heap cannot take them the way - libhdf5 would: an attribute that needs a heap block larger than the next - one (libhdf5 skips blocks and records them as free space — in practice an - attribute of roughly 1 to 4 KiB going into a young heap), a heap with I/O - filters or child indirect blocks (more than about 512 KiB of attributes), - free space the heap tracks outside direct blocks, replacing the last - attribute left in a heap block by one of another size (libhdf5 frees the - block), directly addressed huge objects; and shared attribute messages; + libhdf5 would: replacing the last attribute left in a heap block by one + of another size (libhdf5 frees the block), a heap with I/O filters or + child indirect blocks (more than about 512 KiB of attributes), free + space in child indirect blocks, directly addressed huge objects; and + shared attribute messages. Measured 2026-09-26 on tank with the review's + random-edit harness (120 runs of 150 random edits, `earliest`/`v110`/ + `latest`, about 5600 `set_attr` calls of 8 bytes to 6 KiB): 2.2% of + `set_attr` calls are refused, every one the last-attribute-in-a-block + replacement; before blocks could be skipped (an attribute needing a heap + block larger than the next one — any attribute of about 1 KiB or more + once a heap has started, or at the move to dense storage), 24% were, + since an object whose move to dense storage was refused kept refusing + every new attribute; - version-1 object headers asked for an attribute larger than a header message (they have no dense storage); - partial edge chunks stored unfiltered (`H5Pset_chunk_opts`), external