edit: skip heap blocks too small for an attribute, as libhdf5 does
An attribute needing a heap block larger than the next one was refused
("skipping blocks too small for an object", "a first object too large for
the starting block"); once an object's move to dense storage was refused
it refused every new attribute, so 24% of set_attr calls in the review's
random workload failed.
Following H5HF__hdr_update_iter, H5HF__man_iblock_root_create/_double and
H5HF__hdr_skip_blocks, the smaller blocks are now skipped: the iterator
moves past them and they become an indirect free section with a first
row section (serialized, class 1, as H5HF__sect_indirect_serialize writes
it) and ghost normal rows, added as returned space so it merges with a
range skipped just before it (H5HF__sect_indirect_merge_row). Later
objects that best-fit a row section get a block created there
(H5HF__man_iblock_alloc_row / H5HF__sect_indirect_reduce_row: from the
start or end of the range, or from its middle, which splits it, with
libhdf5's span bookkeeping). Heaps with such sections, as libhdf5 writes
them, are now read too (they were refused at open).
dense_skipped_blocks_match_libhdf5 drives every path (merge, split, end,
last entry, row wrap) on earliest/v110/latest files against libhdf5
doing the same edits one session each; heaps, free sections and index
B-trees are equal after every phase. The refusal test now checks the
skip against libhdf5 and keeps a real refusal (last object in a block);
clawhdf5-written heaps get 1-4 KiB attributes too. The three tests fail
on the previous fheap.rs. Random workload refusals: 24% -> 2.2%, all the
documented last-object-in-a-block case.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
@@ -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='<i4'))]\n\
|
||||
\x20 for k, o in enumerate(objs):\n\
|
||||
\x20 o.attrs.create('c0', np.bytes_('s' * (1200 + 700 * k)))\n\
|
||||
\x20 for i in range(1, 8): o.attrs.create(f'c{{i}}', np.array([i], dtype='<i8'))\n\
|
||||
\x20 h = f.create_group('h', track_order=True)\n\
|
||||
\x20 for i in range(8): h.attrs.create(f'c{{i}}', np.array([i], dtype='<i8'))\n",
|
||||
p = p.to_str().unwrap()
|
||||
));
|
||||
}
|
||||
let objs = ["g", "t", "d"];
|
||||
let mut want: Vec<AttrOp> = 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<AttrOp> = 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<String> = 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
|
||||
|
||||
Reference in New Issue
Block a user