writer: order dense name indexes by hash, then name
libhdf5 compares the name when two hashes are equal; the writer broke ties by insertion order, and libhdf5 could not find one of two names whose lookup3 hashes collide (k69209 / k155448). Test fails before the fix with h5py's KeyError. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
@@ -2,6 +2,17 @@
|
|||||||
|
|
||||||
## Unreleased
|
## Unreleased
|
||||||
|
|
||||||
|
### Writer: large dense indexes (2026-09-26)
|
||||||
|
- **Links and attributes whose name hashes collide are found by name.** The
|
||||||
|
dense name indexes (a group's links: B-tree v2 type 5; an object's
|
||||||
|
attributes: type 8) are ordered by the name's lookup3 hash and, when two
|
||||||
|
hashes are equal, by the name itself, as libhdf5 compares them. The writer
|
||||||
|
broke ties by insertion order, so libhdf5 could not open one of two
|
||||||
|
colliding names (`"k69209"` and `"k155448"` share hash `0x3a0b13e6`;
|
||||||
|
collisions are likely from about 77 000 names). Regression test
|
||||||
|
`names_whose_hashes_collide_are_found_by_name` in
|
||||||
|
`crates/clawhdf5/tests/writer_groups_interop.rs`.
|
||||||
|
|
||||||
### Concurrent reads (2026-09-26)
|
### Concurrent reads (2026-09-26)
|
||||||
- **Full reads of chunked datasets scale with threads again when rayon's
|
- **Full reads of chunked datasets scale with threads again when rayon's
|
||||||
pool has one thread.** Each full read handed its chunks to rayon to
|
pool has one thread.** Each full read handed its chunks to rayon to
|
||||||
|
|||||||
@@ -910,20 +910,27 @@ pub(crate) fn build_dense_attrs(
|
|||||||
let heap_id_length = heap.heap_id_length;
|
let heap_id_length = heap.heap_id_length;
|
||||||
let heap_ids = &heap.heap_ids;
|
let heap_ids = &heap.heap_ids;
|
||||||
|
|
||||||
// Build B-tree v2 type 8 records (17 bytes each)
|
// Build B-tree v2 type 8 records (17 bytes each), in the index's key
|
||||||
|
// order: libhdf5 compares the name hash, then — for names whose hashes
|
||||||
|
// collide — the names themselves (`strcmp`).
|
||||||
let record_size: u16 = heap_id_length + 1 + 4 + 4;
|
let record_size: u16 = heap_id_length + 1 + 4 + 4;
|
||||||
let mut records: Vec<(u32, u32, Vec<u8>)> = Vec::with_capacity(attrs.len());
|
let mut order: Vec<usize> = (0..attrs.len()).collect();
|
||||||
for (i, heap_id) in heap_ids.iter().enumerate() {
|
order.sort_by(|&a, &b| {
|
||||||
let mut rec = Vec::with_capacity(record_size as usize);
|
name_hashes[a]
|
||||||
rec.extend_from_slice(heap_id);
|
.cmp(&name_hashes[b])
|
||||||
rec.push(0); // msg_flags
|
.then_with(|| attrs[a].name.as_bytes().cmp(attrs[b].name.as_bytes()))
|
||||||
rec.extend_from_slice(&(i as u32).to_le_bytes()); // creation_order
|
});
|
||||||
rec.extend_from_slice(&name_hashes[i].to_le_bytes()); // hash
|
let records: Vec<Vec<u8>> = order
|
||||||
records.push((name_hashes[i], i as u32, rec));
|
.into_iter()
|
||||||
}
|
.map(|i| {
|
||||||
records.sort_by(|a, b| a.0.cmp(&b.0).then(a.1.cmp(&b.1)));
|
let mut rec = Vec::with_capacity(record_size as usize);
|
||||||
|
rec.extend_from_slice(&heap_ids[i]);
|
||||||
let records: Vec<Vec<u8>> = records.into_iter().map(|(_, _, rec)| rec).collect();
|
rec.push(0); // msg_flags
|
||||||
|
rec.extend_from_slice(&(i as u32).to_le_bytes()); // creation_order
|
||||||
|
rec.extend_from_slice(&name_hashes[i].to_le_bytes()); // hash
|
||||||
|
rec
|
||||||
|
})
|
||||||
|
.collect();
|
||||||
let bthd_addr = btree_addr;
|
let bthd_addr = btree_addr;
|
||||||
let mut blob = heap.blob;
|
let mut blob = heap.blob;
|
||||||
blob.extend_from_slice(&single_leaf_v2_btree(
|
blob.extend_from_slice(&single_leaf_v2_btree(
|
||||||
@@ -1039,14 +1046,18 @@ pub(crate) fn build_dense_links(
|
|||||||
let heap = build_single_block_fractal_heap(&serialized, base_address, 32, 7)?;
|
let heap = build_single_block_fractal_heap(&serialized, base_address, 32, 7)?;
|
||||||
let heap_id_length = heap.heap_id_length;
|
let heap_id_length = heap.heap_id_length;
|
||||||
|
|
||||||
// Type 5 records: hash(4) + heap_id. The B-tree's key is the name hash,
|
// Type 5 records: hash(4) + heap_id. The B-tree's key is the name hash
|
||||||
// so records are sorted by (hash, order).
|
// and, for names whose hashes collide, the name (libhdf5 compares them
|
||||||
|
// with `strcmp`): records out of that order are not found by name.
|
||||||
let mut by_name: Vec<(u32, usize)> = links
|
let mut by_name: Vec<(u32, usize)> = links
|
||||||
.iter()
|
.iter()
|
||||||
.enumerate()
|
.enumerate()
|
||||||
.map(|(i, l)| (crate::checksum::jenkins_lookup3(l.name.as_bytes()), i))
|
.map(|(i, l)| (crate::checksum::jenkins_lookup3(l.name.as_bytes()), i))
|
||||||
.collect();
|
.collect();
|
||||||
by_name.sort_unstable();
|
by_name.sort_unstable_by(|&(ha, a), &(hb, b)| {
|
||||||
|
ha.cmp(&hb)
|
||||||
|
.then_with(|| links[a].name.as_bytes().cmp(links[b].name.as_bytes()))
|
||||||
|
});
|
||||||
let name_records: Vec<Vec<u8>> = by_name
|
let name_records: Vec<Vec<u8>> = by_name
|
||||||
.iter()
|
.iter()
|
||||||
.map(|&(hash, i)| {
|
.map(|&(hash, i)| {
|
||||||
|
|||||||
@@ -627,6 +627,45 @@ fn non_ascii_names_are_utf8() {
|
|||||||
assert_eq!(f.dataset("größe/wert").unwrap().read_i32().unwrap(), [1]);
|
assert_eq!(f.dataset("größe/wert").unwrap().read_i32().unwrap(), [1]);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn names_whose_hashes_collide_are_found_by_name() {
|
||||||
|
skip_if_no_python!();
|
||||||
|
// "k69209" and "k155448" have the same lookup3 hash (0x3a0b13e6). The
|
||||||
|
// dense name indexes (links: type 5, attributes: type 8) are ordered by
|
||||||
|
// hash and then by name, and libhdf5's lookup relies on it. The writer
|
||||||
|
// broke ties by insertion order, so with "k69209" added first libhdf5
|
||||||
|
// could not open "k155448" by name.
|
||||||
|
let dir = tempfile::tempdir().unwrap();
|
||||||
|
let mut b = FileBuilder::new();
|
||||||
|
let mut g = b.create_group("g");
|
||||||
|
for (i, n) in ["k69209", "k155448"]
|
||||||
|
.into_iter()
|
||||||
|
.chain((0..10).map(|_| ""))
|
||||||
|
.enumerate()
|
||||||
|
{
|
||||||
|
let name = if n.is_empty() { format!("d{i}") } else { n.into() };
|
||||||
|
g.create_dataset(&name).with_i32_data(&[i as i32]);
|
||||||
|
}
|
||||||
|
b.add_group(g.finish());
|
||||||
|
let x = b.create_dataset("x");
|
||||||
|
x.with_i32_data(&[0]);
|
||||||
|
x.set_attr("k69209", AttrValue::I64(1));
|
||||||
|
x.set_attr("k155448", AttrValue::I64(2));
|
||||||
|
for i in 0..10 {
|
||||||
|
x.set_attr(&format!("a{i}"), AttrValue::I64(10 + i));
|
||||||
|
}
|
||||||
|
let path = write(&dir, "collide.h5", b);
|
||||||
|
let out = h5py(
|
||||||
|
&path,
|
||||||
|
"with h5py.File(path, 'r') as f:\n\
|
||||||
|
\x20 g, a = f['g'], f['x'].attrs\n\
|
||||||
|
\x20 print(json.dumps([int(g['k69209'][0]), int(g['k155448'][0]), 'k155448' in g,\n\
|
||||||
|
\x20 int(a['k69209']), int(a['k155448']), 'k155448' in a]))",
|
||||||
|
);
|
||||||
|
assert_eq!(out, "[0, 1, true, 1, 2, true]");
|
||||||
|
h5dump_ok(&path);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn a_group_attribute_set_again_takes_the_new_value() {
|
fn a_group_attribute_set_again_takes_the_new_value() {
|
||||||
skip_if_no_python!();
|
skip_if_no_python!();
|
||||||
|
|||||||
Reference in New Issue
Block a user