From f0ecae38b68d519bc357e1a2601639184d2ea827 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 09:01:54 -0500 Subject: [PATCH] perf(py): datasets and groups keep their address; groups their links Every ds[...] and g[k] resolved the path from the root again, two or three times per open, and resolving a name in a large group scans its links: visiting a group was O(n^2). 4000 scalar datasets in one group took 39 s (v1 group) and 131 s (dense) to list, read and re-read; now 0.3 s each. A Dataset keeps its object address, a Group (and the file's root) its address and, after the first lookup, its link table. New facade API File::dataset_at(address), tested in integration_tests. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 11 + crates/clawhdf5-py/src/attrs.rs | 7 +- crates/clawhdf5-py/src/dataset.rs | 22 +- crates/clawhdf5-py/src/file.rs | 41 +-- crates/clawhdf5-py/src/group.rs | 276 +++++++++++------- crates/clawhdf5-py/src/node.rs | 74 +++-- crates/clawhdf5-py/tests/test_read_vs_h5py.py | 33 +++ crates/clawhdf5/src/reader.rs | 16 + crates/clawhdf5/tests/integration_tests.rs | 32 ++ 9 files changed, 349 insertions(+), 163 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a29e25a..8e61de0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -80,6 +80,17 @@ (h5py 4.1 ms; release build on tank, best of 5). `test_a_long_index_list_decodes_each_chunk_once` compares 1-D, 2-D and contiguous cases with h5py under a 2 s bound (5.8 s before, debug build). +- **Groups and datasets remember where they are.** Every `ds[...]`, and + every `g[k]`, resolved its path from the root again (two or three times + per open), and in a large group each resolution scans the group's links, + so visiting a group was quadratic: 4000 scalar datasets in one group took + 39 s (`libver='earliest'`) and 131 s (`'latest'`) to list, read and + re-read in `test_big_groups_are_not_quadratic`; now 0.3 s each (debug + build). A `Dataset` keeps its object's address, and a `Group` (and the + file's root) its address and, once listed, its link table. New facade + API: `File::dataset_at(address)` opens a dataset without resolving a + path. libhdf5's `h5stat_newgrat.h5` (35001 members in the root): listing + takes 0.03 s and 2000 opens 1 ms (h5py: 0.022 s). - **CI builds and tests the Python package.** It was excluded from CI. `scripts/ci-test.sh` now lints `clawhdf5-py`, builds the wheel with maturin, unpacks it under `target/` and runs the pytest suite; skipped diff --git a/crates/clawhdf5-py/src/attrs.rs b/crates/clawhdf5-py/src/attrs.rs index d1b06cb..fd3f0b3 100644 --- a/crates/clawhdf5-py/src/attrs.rs +++ b/crates/clawhdf5-py/src/attrs.rs @@ -34,9 +34,10 @@ pub struct PyAttrs { } impl PyAttrs { - /// The attributes of the object at `path` in a file opened for reading. - pub(crate) fn read(file: Arc, path: &str) -> PyResult { - let attrs = node::attributes(&file, path)?; + /// The attributes of the object at `addr` (whose path is `path`) in a + /// file opened for reading. + pub(crate) fn read(file: Arc, addr: u64, path: &str) -> PyResult { + let attrs = node::attributes(&file, addr, path)?; Ok(Self { inner: AttrsInner::Read { file, attrs }, }) diff --git a/crates/clawhdf5-py/src/dataset.rs b/crates/clawhdf5-py/src/dataset.rs index aa75e7d..c20f293 100644 --- a/crates/clawhdf5-py/src/dataset.rs +++ b/crates/clawhdf5-py/src/dataset.rs @@ -9,6 +9,7 @@ use std::sync::Arc; use clawhdf5_format::datatype::Datatype; +use clawhdf5_format::object_header::ObjectHeader; use pyo3::exceptions::{PyTypeError, PyValueError}; use pyo3::prelude::*; use pyo3::types::{PyList, PyTuple}; @@ -29,6 +30,9 @@ use crate::{PyEmpty, node, to_py_err}; pub struct PyDataset { file: Arc, path: String, + /// Where the dataset's object header is: reads open it from here rather + /// than resolve `path` again. + addr: u64, /// `None` for a dataset with a null dataspace (h5py's `Empty`). shape: Option>, /// The chunk shape, for a chunked dataset. @@ -43,12 +47,13 @@ impl PyDataset { py: Python<'_>, file: Arc, path: String, + addr: u64, + hdr: &ObjectHeader, ) -> PyResult { crate::no_panic(|| { - let hdr = node::header(&file, &path)?; - let null = node::is_null(&node::dataspace(&file, &hdr)?); + let null = node::is_null(&node::dataspace(&file, hdr)?); let (shape, datatype) = { - let ds = file.dataset(&path).map_err(to_py_err)?; + let ds = file.dataset_at(addr).map_err(to_py_err)?; let shape = if null { None } else { @@ -60,10 +65,11 @@ impl PyDataset { .map_err(|e| e.value(py).to_string()); let chunks = shape .as_ref() - .and_then(|s| node::chunk_shape(&file, &hdr, s.len())); + .and_then(|s| node::chunk_shape(&file, hdr, s.len())); Ok(Self { file, path, + addr, shape, chunks, datatype, @@ -96,10 +102,10 @@ impl PyDataset { let (reads, list_axis) = plan.reads(dims, chunk_len, elem_size); let read_shape = plan.read_shape(); let file = &*self.file; - let path = self.path.as_str(); + let addr = self.addr; // Everything below touches only Rust data: release the GIL. let read = || -> Result { - let ds = file.dataset(path)?; + let ds = file.dataset_at(addr)?; let mut blocks = Vec::with_capacity(reads.len()); for read in reads { let raw = ds.read_selection(&read.sel)?; @@ -250,7 +256,7 @@ impl PyDataset { }; let max = self .file - .dataset(&self.path) + .dataset_at(self.addr) .and_then(|ds| ds.max_dimensions()) .map_err(to_py_err)? .unwrap_or_else(|| shape.clone()); @@ -288,7 +294,7 @@ impl PyDataset { /// The dataset's attributes (read-only, dict-like). #[getter] fn attrs(&self) -> PyResult { - PyAttrs::read(Arc::clone(&self.file), &self.path) + PyAttrs::read(Arc::clone(&self.file), self.addr, &self.path) } /// Read with h5py indexing: integers, slices with positive steps, diff --git a/crates/clawhdf5-py/src/file.rs b/crates/clawhdf5-py/src/file.rs index a2c48ea..5f7f51b 100644 --- a/crates/clawhdf5-py/src/file.rs +++ b/crates/clawhdf5-py/src/file.rs @@ -3,12 +3,11 @@ use std::path::PathBuf; use std::sync::{Arc, Mutex}; -use pyo3::exceptions::PyKeyError; use pyo3::prelude::*; use pyo3::types::PyList; use crate::attrs::PyAttrs; -use crate::group::{self, PyGroup, WriteGroupState, finalize_write_group}; +use crate::group::{PyGroup, ReadGroup, WriteGroupState, finalize_write_group}; use crate::{DatasetSpec, OwnedAttrValue, apply_dataset_spec, extract_numpy_data, to_py_err}; /// Internal state for write mode. @@ -40,7 +39,8 @@ pub struct PyFile { } enum FileInner { - Read(Arc), + /// The root group; it holds the file. + Read(ReadGroup), Write(WriteState), } @@ -61,7 +61,7 @@ impl PyFile { crate::no_panic(|| clawhdf5_rs::File::open(path).map_err(to_py_err)) })?; Ok(Self { - inner: Some(FileInner::Read(Arc::new(file))), + inner: Some(FileInner::Read(root_group(Arc::new(file)))), filename, }) } @@ -110,33 +110,28 @@ impl PyFile { /// Get a child object (dataset or group) by path; `f['/']` is the root. fn __getitem__(&self, py: Python<'_>, key: &str) -> PyResult> { - group::get_item(py, self.read_file()?, "", key) + self.read_file()?.get_item(py, key) } /// `f.get(key, default=None)`. #[pyo3(signature = (key, default=None))] fn get(&self, py: Python<'_>, key: &str, default: Option>) -> PyResult> { - match group::get_item(py, self.read_file()?, "", key) { - Err(e) if e.is_instance_of::(py) => { - Ok(default.unwrap_or_else(|| py.None())) - } - other => other, - } + self.read_file()?.get(py, key, default) } /// List the names of all children in the root group. fn keys(&self, py: Python<'_>) -> PyResult> { - let names = group::member_names(self.read_file()?, "")?; + let names = self.read_file()?.member_names()?; Ok(PyList::new(py, names)?.into_any().unbind()) } fn values(&self, py: Python<'_>) -> PyResult> { - let vals = group::values(py, self.read_file()?, "")?; + let vals = self.read_file()?.values(py)?; Ok(PyList::new(py, vals)?.into_any().unbind()) } fn items(&self, py: Python<'_>) -> PyResult> { - let items = group::items(py, self.read_file()?, "")?; + let items = self.read_file()?.items(py)?; Ok(PyList::new(py, items)?.into_any().unbind()) } @@ -145,7 +140,7 @@ impl PyFile { } fn __len__(&self) -> PyResult { - Ok(group::member_names(self.read_file()?, "")?.len()) + Ok(self.read_file()?.member_names()?.len()) } /// The root group's name, `/`. @@ -211,7 +206,7 @@ impl PyFile { #[getter] fn attrs(&self) -> PyResult { match self.inner.as_ref() { - Some(FileInner::Read(file)) => PyAttrs::read(Arc::clone(file), ""), + Some(FileInner::Read(root)) => root.attrs(), Some(FileInner::Write(state)) => Ok(PyAttrs::from_write(Arc::clone(&state.root_attrs))), None => Err(PyErr::new::( "file is closed", @@ -221,8 +216,8 @@ impl PyFile { fn __repr__(&self) -> String { match &self.inner { - Some(FileInner::Read(f)) => { - format!("", f.as_bytes().len()) + Some(FileInner::Read(root)) => { + format!("", root.file.as_bytes().len()) } Some(FileInner::Write(s)) => { format!("", s.path.display()) @@ -232,12 +227,13 @@ impl PyFile { } fn __contains__(&self, key: &str) -> PyResult { - Ok(group::contains(self.read_file()?, "", key)) + Ok(self.read_file()?.contains(key)) } } impl PyFile { - fn read_file(&self) -> PyResult<&Arc> { + /// The root group of a file opened for reading. + fn read_file(&self) -> PyResult<&ReadGroup> { match &self.inner { Some(FileInner::Read(f)) => Ok(f), Some(FileInner::Write(_)) => Err(PyErr::new::( @@ -275,6 +271,11 @@ fn parse_compression( } } +fn root_group(file: Arc) -> ReadGroup { + let root = file.superblock().root_group_address; + ReadGroup::new(file, String::new(), root) +} + /// Build and write the HDF5 file from accumulated write state. fn finalize_write(state: WriteState) -> PyResult<()> { crate::no_panic(|| { diff --git a/crates/clawhdf5-py/src/group.rs b/crates/clawhdf5-py/src/group.rs index 365846b..7585e57 100644 --- a/crates/clawhdf5-py/src/group.rs +++ b/crates/clawhdf5-py/src/group.rs @@ -1,13 +1,14 @@ //! PyGroup — navigable HDF5 group with read and write support. -use std::sync::{Arc, Mutex}; +use std::collections::HashMap; +use std::sync::{Arc, Mutex, OnceLock}; -use pyo3::exceptions::{PyIOError, PyKeyError}; +use pyo3::exceptions::{PyIOError, PyKeyError, PyValueError}; use pyo3::prelude::*; use pyo3::types::PyList; use crate::attrs::PyAttrs; -use crate::{DatasetSpec, OwnedAttrValue, apply_dataset_spec, extract_numpy_data, node, to_py_err}; +use crate::{DatasetSpec, OwnedAttrValue, apply_dataset_spec, extract_numpy_data, node}; /// Shared state for a group being written. pub(crate) struct WriteGroupState { @@ -28,17 +29,14 @@ pub struct PyGroup { } enum GroupInner { - Read { - file: Arc, - path: String, - }, + Read(ReadGroup), Write(Arc>), } impl PyGroup { - pub(crate) fn from_read(file: Arc, path: String) -> Self { + pub(crate) fn from_read(file: Arc, path: String, addr: u64) -> Self { Self { - inner: GroupInner::Read { file, path }, + inner: GroupInner::Read(ReadGroup::new(file, path, addr)), } } @@ -48,9 +46,9 @@ impl PyGroup { } } - fn read_parts(&self, what: &str) -> PyResult<(&Arc, &str)> { + fn read_group(&self, what: &str) -> PyResult<&ReadGroup> { match &self.inner { - GroupInner::Read { file, path } => Ok((file, path)), + GroupInner::Read(g) => Ok(g), GroupInner::Write(_) => Err(PyIOError::new_err(format!( "cannot {what} a group opened for writing" ))), @@ -58,77 +56,90 @@ impl PyGroup { } } -// Read-mode operations shared by `Group` and `File` (a file is its root -// group, as in h5py). - -/// `group[key]`. -pub(crate) fn get_item( - py: Python<'_>, - file: &Arc, - path: &str, - key: &str, -) -> PyResult> { - node::open(py, file, node::join(path, key)) +/// A group in a file opened for reading (a file is its root group, as in +/// h5py). It keeps its own address and, once listed, its links, so looking +/// up a child neither resolves the path from the root nor scans the group's +/// links again: visiting every member of a large group is linear, not +/// quadratic. +pub(crate) struct ReadGroup { + pub file: Arc, + pub path: String, + pub addr: u64, + /// Link name -> object address (soft links resolved), filled on first use. + links: OnceLock>, + /// Names of the datasets and subgroups, sorted (h5py's order). + members: OnceLock>, } -/// Names of the group's datasets and subgroups, sorted (h5py's order). -pub(crate) fn member_names(file: &clawhdf5_rs::File, path: &str) -> PyResult> { - crate::no_panic(|| { - let group = if path.is_empty() { - file.root() - } else { - file.group(path).map_err(to_py_err)? - }; - let mut names = group.datasets().map_err(to_py_err)?; - names.extend(group.groups().map_err(to_py_err)?); - names.sort_by(|a, b| a.as_bytes().cmp(b.as_bytes())); - names.dedup(); - Ok(names) - }) -} - -pub(crate) fn contains(file: &clawhdf5_rs::File, path: &str, key: &str) -> bool { - node::exists(file, &node::join(path, key)) -} - -pub(crate) fn values( - py: Python<'_>, - file: &Arc, - path: &str, -) -> PyResult>> { - member_names(file, path)? - .iter() - .map(|n| get_item(py, file, path, n)) - .collect() -} - -pub(crate) fn items( - py: Python<'_>, - file: &Arc, - path: &str, -) -> PyResult)>> { - member_names(file, path)? - .into_iter() - .map(|n| { - let v = get_item(py, file, path, &n)?; - Ok((n, v)) - }) - .collect() -} - -#[pymethods] -impl PyGroup { - /// Get a child object (dataset or subgroup) by name or path. - fn __getitem__(&self, py: Python<'_>, key: &str) -> PyResult> { - let (file, path) = self.read_parts("read children from")?; - get_item(py, file, path, key) +impl ReadGroup { + pub(crate) fn new(file: Arc, path: String, addr: u64) -> Self { + Self { + file, + path, + addr, + links: OnceLock::new(), + members: OnceLock::new(), + } } - /// `group.get(key, default=None)`. - #[pyo3(signature = (key, default=None))] - fn get(&self, py: Python<'_>, key: &str, default: Option>) -> PyResult> { - let (file, path) = self.read_parts("read children from")?; - match get_item(py, file, path, key) { + fn links(&self) -> PyResult<&HashMap> { + if let Some(links) = self.links.get() { + return Ok(links); + } + let entries = crate::no_panic(|| { + clawhdf5_format::group_v2::resolve_group_children( + self.file.as_bytes(), + self.file.superblock(), + self.addr, + ) + .map_err(|e| PyValueError::new_err(format!("{}: {e}", node::name(&self.path)))) + })?; + let map = entries + .into_iter() + .map(|e| (e.name, e.object_header_address)) + .collect(); + Ok(self.links.get_or_init(|| map)) + } + + /// The path and address of `key` (a name, a relative or an absolute path). + fn locate(&self, key: &str) -> PyResult<(String, u64)> { + let path = node::join(&self.path, key); + let rel = if self.path.is_empty() { + Some(path.as_str()) + } else if path == self.path { + Some("") + } else { + path.strip_prefix(self.path.as_str()) + .and_then(|r| r.strip_prefix('/')) + }; + let addr = match rel { + // A direct child: the link table, when it has the name. + Some(name) if !name.is_empty() && !name.contains('/') => { + match self.links()?.get(name) { + Some(&a) => a, + None => node::resolve_from(&self.file, self.addr, name, &path)?, + } + } + Some(rel) => node::resolve_from(&self.file, self.addr, rel, &path)?, + None => node::address(&self.file, &path)?, + }; + Ok((path, addr)) + } + + /// `group[key]`. + pub(crate) fn get_item(&self, py: Python<'_>, key: &str) -> PyResult> { + let (path, addr) = self.locate(key)?; + node::open(py, &self.file, path, addr) + } + + /// `group.get(key, default)`. + pub(crate) fn get( + &self, + py: Python<'_>, + key: &str, + default: Option>, + ) -> PyResult> { + match self.get_item(py, key) { Err(e) if e.is_instance_of::(py) => { Ok(default.unwrap_or_else(|| py.None())) } @@ -136,11 +147,70 @@ impl PyGroup { } } + /// Names of the group's datasets and subgroups, sorted (h5py's order). + pub(crate) fn member_names(&self) -> PyResult<&[String]> { + if let Some(m) = self.members.get() { + return Ok(m); + } + let mut names = Vec::new(); + for (name, &addr) in self.links()? { + let hdr = node::header_at(&self.file, addr, &node::join(&self.path, name))?; + if matches!( + node::kind(&hdr), + Some(node::Kind::Dataset | node::Kind::Group) + ) { + names.push(name.clone()); + } + } + names.sort_by(|a, b| a.as_bytes().cmp(b.as_bytes())); + Ok(self.members.get_or_init(|| names)) + } + + pub(crate) fn contains(&self, key: &str) -> bool { + self.locate(key) + .and_then(|(path, addr)| node::header_at(&self.file, addr, &path)) + .ok() + .and_then(|h| node::kind(&h)) + .is_some_and(|k| k != node::Kind::Datatype) + } + + pub(crate) fn values(&self, py: Python<'_>) -> PyResult>> { + self.member_names()? + .iter() + .map(|n| self.get_item(py, n)) + .collect() + } + + pub(crate) fn items(&self, py: Python<'_>) -> PyResult)>> { + self.member_names()? + .iter() + .map(|n| Ok((n.clone(), self.get_item(py, n)?))) + .collect() + } + + pub(crate) fn attrs(&self) -> PyResult { + PyAttrs::read(Arc::clone(&self.file), self.addr, &self.path) + } +} + +#[pymethods] +impl PyGroup { + /// Get a child object (dataset or subgroup) by name or path. + fn __getitem__(&self, py: Python<'_>, key: &str) -> PyResult> { + self.read_group("read children from")?.get_item(py, key) + } + + /// `group.get(key, default=None)`. + #[pyo3(signature = (key, default=None))] + fn get(&self, py: Python<'_>, key: &str, default: Option>) -> PyResult> { + self.read_group("read children from")?.get(py, key, default) + } + /// List the names of all children (datasets and subgroups). fn keys(&self, py: Python<'_>) -> PyResult> { match &self.inner { - GroupInner::Read { file, path } => { - let list = PyList::new(py, member_names(file, path)?)?; + GroupInner::Read(g) => { + let list = PyList::new(py, g.member_names()?)?; Ok(list.into_any().unbind()) } GroupInner::Write(state) => { @@ -153,15 +223,13 @@ impl PyGroup { } fn values(&self, py: Python<'_>) -> PyResult> { - let (file, path) = self.read_parts("read children from")?; - Ok(PyList::new(py, values(py, file, path)?)? - .into_any() - .unbind()) + let g = self.read_group("read children from")?; + Ok(PyList::new(py, g.values(py)?)?.into_any().unbind()) } fn items(&self, py: Python<'_>) -> PyResult> { - let (file, path) = self.read_parts("read children from")?; - Ok(PyList::new(py, items(py, file, path)?)?.into_any().unbind()) + let g = self.read_group("read children from")?; + Ok(PyList::new(py, g.items(py)?)?.into_any().unbind()) } fn __iter__(&self, py: Python<'_>) -> PyResult> { @@ -170,7 +238,7 @@ impl PyGroup { fn __len__(&self) -> PyResult { match &self.inner { - GroupInner::Read { file, path } => Ok(member_names(file, path)?.len()), + GroupInner::Read(g) => Ok(g.member_names()?.len()), GroupInner::Write(state) => Ok(state.lock().unwrap().datasets.len()), } } @@ -179,7 +247,7 @@ impl PyGroup { #[getter] fn name(&self) -> String { match &self.inner { - GroupInner::Read { path, .. } => node::name(path), + GroupInner::Read(g) => node::name(&g.path), GroupInner::Write(state) => node::name(&state.lock().unwrap().name), } } @@ -235,7 +303,7 @@ impl PyGroup { #[getter] fn attrs(&self) -> PyResult { match &self.inner { - GroupInner::Read { file, path } => PyAttrs::read(Arc::clone(file), path), + GroupInner::Read(g) => g.attrs(), GroupInner::Write(state) => { let store = Arc::clone(&state.lock().unwrap().attrs); Ok(PyAttrs::from_write(store)) @@ -245,9 +313,9 @@ impl PyGroup { fn __repr__(&self) -> String { match &self.inner { - GroupInner::Read { file, path } => { - let n = member_names(file, path).map_or(0, |m| m.len()); - format!("", node::name(path)) + GroupInner::Read(g) => { + let n = g.member_names().map_or(0, |m| m.len()); + format!("", node::name(&g.path)) } GroupInner::Write(state) => { let name = &state.lock().unwrap().name; @@ -258,7 +326,7 @@ impl PyGroup { fn __contains__(&self, key: &str) -> PyResult { match &self.inner { - GroupInner::Read { file, path } => Ok(contains(file, path, key)), + GroupInner::Read(g) => Ok(g.contains(key)), GroupInner::Write(state) => { let guard = state.lock().unwrap(); Ok(guard.datasets.iter().any(|d| d.name == key)) @@ -299,15 +367,19 @@ mod tests { let finished = g.finish(); b.add_group(finished); let bytes = b.finish().unwrap(); - let file = clawhdf5_rs::File::from_bytes(bytes).unwrap(); - assert_eq!( - member_names(&file, "").unwrap(), - vec!["alpha", "mid", "zeta"] - ); - assert_eq!(member_names(&file, "mid").unwrap(), vec!["x"]); - assert!(contains(&file, "", "mid/x")); - assert!(contains(&file, "mid", "/alpha")); - assert!(!contains(&file, "", "nope")); + let file = Arc::new(clawhdf5_rs::File::from_bytes(bytes).unwrap()); + let root = file.superblock().root_group_address; + let top = ReadGroup::new(Arc::clone(&file), String::new(), root); + assert_eq!(top.member_names().unwrap(), ["alpha", "mid", "zeta"]); + let (path, addr) = top.locate("mid").unwrap(); + assert_eq!(path, "mid"); + let mid = ReadGroup::new(Arc::clone(&file), path, addr); + assert_eq!(mid.member_names().unwrap(), ["x"]); + assert!(top.contains("mid/x")); + assert!(mid.contains("/alpha")); + assert!(mid.contains("x") && mid.contains("./x")); + assert!(!top.contains("nope")); + assert!(!mid.contains("alpha")); } #[test] diff --git a/crates/clawhdf5-py/src/node.rs b/crates/clawhdf5-py/src/node.rs index bb6c2d2..fa9ede7 100644 --- a/crates/clawhdf5-py/src/node.rs +++ b/crates/clawhdf5-py/src/node.rs @@ -33,24 +33,40 @@ pub(crate) fn name(path: &str) -> String { format!("/{path}") } -/// The object header of the object at `path`. -pub(crate) fn header(file: &clawhdf5_rs::File, path: &str) -> PyResult { +/// The address of the object at `path`, resolved from the root group. +pub(crate) fn address(file: &clawhdf5_rs::File, path: &str) -> PyResult { + resolve_from(file, file.superblock().root_group_address, path, path) +} + +/// The address of `rel` resolved from the group at `group` (`full` is the +/// resulting path, for the error message). +pub(crate) fn resolve_from( + file: &clawhdf5_rs::File, + group: u64, + rel: &str, + full: &str, +) -> PyResult { + if rel.is_empty() { + return Ok(group); + } crate::no_panic(|| { - let sb = file.superblock(); - let data = file.as_bytes(); - let addr = if path.is_empty() { - sb.root_group_address - } else { - clawhdf5_format::group_v2::resolve_path_any(data, sb, path).map_err(|e| { + clawhdf5_format::group_v2::resolve_path_from(file.as_bytes(), file.superblock(), group, rel) + .map_err(|e| { PyKeyError::new_err(format!( "Unable to open object (object '{}' doesn't exist): {e}", - name(path) + name(full) )) - })? - }; - let addr = usize::try_from(addr) + }) + }) +} + +/// The object header at `addr` (the object at `path`). +pub(crate) fn header_at(file: &clawhdf5_rs::File, addr: u64, path: &str) -> PyResult { + crate::no_panic(|| { + let sb = file.superblock(); + let at = usize::try_from(addr) .map_err(|_| PyValueError::new_err(format!("{}: address out of range", name(path))))?; - ObjectHeader::parse(data, addr, sb.offset_size, sb.length_size) + ObjectHeader::parse(file.as_bytes(), at, sb.offset_size, sb.length_size) .map_err(|e| PyValueError::new_err(format!("{}: {e}", name(path)))) }) } @@ -80,19 +96,21 @@ pub(crate) fn kind(hdr: &ObjectHeader) -> Option { } } -/// Open the object at `path` as a `Dataset` or `Group`. +/// Open the object at `addr` (whose path is `path`) as a `Dataset` or +/// `Group`. Both keep the address, so later reads resolve nothing. pub(crate) fn open( py: Python<'_>, file: &Arc, path: String, + addr: u64, ) -> PyResult> { - let hdr = header(file, &path)?; + let hdr = header_at(file, addr, &path)?; match kind(&hdr) { - Some(Kind::Dataset) => Ok(PyDataset::open(py, Arc::clone(file), path)? + Some(Kind::Dataset) => Ok(PyDataset::open(py, Arc::clone(file), path, addr, &hdr)? .into_pyobject(py)? .into_any() .unbind()), - Some(Kind::Group) => Ok(PyGroup::from_read(Arc::clone(file), path) + Some(Kind::Group) => Ok(PyGroup::from_read(Arc::clone(file), path, addr) .into_pyobject(py)? .into_any() .unbind()), @@ -107,14 +125,6 @@ pub(crate) fn open( } } -/// Whether `path` names a dataset or group. -pub(crate) fn exists(file: &clawhdf5_rs::File, path: &str) -> bool { - header(file, path) - .ok() - .and_then(|h| kind(&h)) - .is_some_and(|k| k != Kind::Datatype) -} - /// The dataspace message of an object header. pub(crate) fn dataspace(file: &clawhdf5_rs::File, hdr: &ObjectHeader) -> PyResult { crate::no_panic(|| { @@ -166,12 +176,16 @@ pub(crate) fn is_null(space: &Dataspace) -> bool { space.space_type == DataspaceType::Null } -/// The attributes of the object at `path`, sorted by name (h5py's order). -/// Attributes whose messages cannot be parsed are left out, as the facade's -/// `attrs()` does. -pub(crate) fn attributes(file: &clawhdf5_rs::File, path: &str) -> PyResult> { +/// The attributes of the object at `addr` (whose path is `path`), sorted by +/// name (h5py's order). Attributes whose messages cannot be parsed are left +/// out, as the facade's `attrs()` does. +pub(crate) fn attributes( + file: &clawhdf5_rs::File, + addr: u64, + path: &str, +) -> PyResult> { + let hdr = header_at(file, addr, path)?; crate::no_panic(|| { - let hdr = header(file, path)?; let sb = file.superblock(); let (mut attrs, _errors) = clawhdf5_format::attribute::extract_attributes_tolerant( file.as_bytes(), diff --git a/crates/clawhdf5-py/tests/test_read_vs_h5py.py b/crates/clawhdf5-py/tests/test_read_vs_h5py.py index f51c534..a02484c 100644 --- a/crates/clawhdf5-py/tests/test_read_vs_h5py.py +++ b/crates/clawhdf5-py/tests/test_read_vs_h5py.py @@ -563,3 +563,36 @@ def test_a_long_index_list_decodes_each_chunk_once(h5py, tmp_path): took = time.perf_counter() - t0 assert_same(got, theirs[name][key], f"{name}[{len(key)}-key]") assert took < 2.0, f"{name}: {took:.2f} s" + + +@pytest.mark.parametrize("libver", ["earliest", "latest"]) +def test_big_groups_are_not_quadratic(h5py, tmp_path, libver): + """A dataset or group remembers where its object is, and a group its + links, so reads and walks over a large group do not resolve every path + from the root again (it was O(n) per access: O(n^2) to visit a group).""" + import time + + path = str(tmp_path / f"big_{libver}.h5") + n = 4000 + with h5py.File(path, "w", libver=libver) as f: + g = f.create_group("g") + for i in range(n): + g.create_dataset(f"d{i:05d}", data=np.int32(i)) + g.create_group("sub").create_dataset("leaf", data=np.arange(3)) + with h5py.File(path, "r") as theirs, clawhdf5.File(path, "r") as ours: + t0 = time.perf_counter() + g = ours["g"] + assert list(g.keys()) == list(theirs["g"].keys()) + total = sum(int(v[()]) for k, v in g.items() if k.startswith("d")) + assert total == n * (n - 1) // 2 + seen = 0 + for k in g: + if k.startswith("d"): + seen += int(g[k][()]) == int(k[1:]) + assert seen == n + ds = g["d00007"] + assert all(ds[()] == 7 for _ in range(2000)) + assert list(g["sub"]["leaf"][:]) == [0, 1, 2] + assert ours["/g/sub/leaf"][1] == 1 and g["/g/d00003"][()] == 3 + took = time.perf_counter() - t0 + assert took < 5.0, f"{took:.2f} s" diff --git a/crates/clawhdf5/src/reader.rs b/crates/clawhdf5/src/reader.rs index 83cbfa8..2e86dfe 100644 --- a/crates/clawhdf5/src/reader.rs +++ b/crates/clawhdf5/src/reader.rs @@ -176,6 +176,22 @@ impl File { }) } + /// A `Dataset` handle for the object header at `address` (an address + /// from a group listing, or one kept from an earlier lookup), without + /// resolving a path. Resolving a path walks every group on it, which in + /// a large group costs a scan of its links; keep the address instead to + /// open the same dataset repeatedly. + pub fn dataset_at(&self, address: u64) -> Result, Error> { + let hdr = self.parse_header(address)?; + if !has_message(&hdr, MessageType::DataLayout) { + return Err(Error::NotADataset(format!("object at address {address}"))); + } + Ok(Dataset { + file: self, + header: hdr, + }) + } + /// Resolve a path and return a `Group` handle. /// /// The path uses `/` separators (e.g., `"sensors"`). diff --git a/crates/clawhdf5/tests/integration_tests.rs b/crates/clawhdf5/tests/integration_tests.rs index e8ddab9..47798e8 100644 --- a/crates/clawhdf5/tests/integration_tests.rs +++ b/crates/clawhdf5/tests/integration_tests.rs @@ -986,3 +986,35 @@ fn u64_data_roundtrip() { values ); } + +// --------------------------------------------------------------------------- +// Opening a dataset by address +// --------------------------------------------------------------------------- + +#[test] +fn dataset_at_opens_the_same_dataset_as_its_path() { + let mut b = FileBuilder::new(); + let mut g = b.create_group("grp"); + g.create_dataset("vals").with_f64_data(&[1.0, 2.5, -3.0]); + b.add_group(g.finish()); + let file = File::from_bytes(b.finish().unwrap()).unwrap(); + + let addr = + clawhdf5_format::group_v2::resolve_path_any(file.as_bytes(), file.superblock(), "grp/vals") + .unwrap(); + let by_addr = file.dataset_at(addr).unwrap(); + assert_eq!(by_addr.read_f64().unwrap(), vec![1.0, 2.5, -3.0]); + assert_eq!( + by_addr.shape().unwrap(), + file.dataset("grp/vals").unwrap().shape().unwrap() + ); + + // The group's own header is not a dataset. + let group_addr = + clawhdf5_format::group_v2::resolve_path_any(file.as_bytes(), file.superblock(), "grp") + .unwrap(); + assert!(matches!( + file.dataset_at(group_addr), + Err(clawhdf5::Error::NotADataset(_)) + )); +}