docs: M1 changelog after review: probe identity, speed, bounded reads
- Per-file probe output is identical for 696 of 697 files, not all: cve-2025-2310.h5's error string depends on which parallel chunk decode fails first, atf2ff2c4as on this branch. - The parser cores are generic (S: Storage + ?Sized); provisional A/B numbers againstf2ff2c4, including the one bench that still shows ObjectHeader::parse slower when old and new are separate binaries. - Reads sized by untrusted fields are bounded; the harness only accepts the known whole-file fallbacks. - range-reads.md records why M1 went generic rather than &dyn. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
+43
-9
@@ -16,8 +16,11 @@
|
|||||||
and (with `std`) `Arc`s of a `Storage`. Slices and `Vec`s serve borrowed
|
and (with `std`) `Arc`s of a `Storage`. Slices and `Vec`s serve borrowed
|
||||||
bytes, so parsing an in-memory file costs no copy.
|
bytes, so parsing an in-memory file costs no copy.
|
||||||
- **The metadata parsers read through `Storage`.** Each converted parser has
|
- **The metadata parsers read through `Storage`.** Each converted parser has
|
||||||
an `*_in(&dyn Storage, ..)` core, and its `&[u8]` function is now a thin
|
an `*_in<S: Storage + ?Sized>(&S, ..)` core (a `&dyn Storage` works too),
|
||||||
wrapper over it, so no caller changes: the superblock
|
and its `&[u8]` function is now a thin wrapper over it, so no caller
|
||||||
|
changes. The wrappers compile to a `[u8]` instance of the same code, so a
|
||||||
|
structure read in memory is a bounds check and a borrowed slice, with no
|
||||||
|
indirect call and no copy. Converted: the superblock
|
||||||
(`Superblock::parse_in`), its extension and cache image
|
(`Superblock::parse_in`), its extension and cache image
|
||||||
(`read_superblock_extension_in`, `cache_image_state_in`), object headers
|
(`read_superblock_extension_in`, `cache_image_state_in`), object headers
|
||||||
with their continuation chunks (`ObjectHeader::parse_in`), local and
|
with their continuation chunks (`ObjectHeader::parse_in`), local and
|
||||||
@@ -32,6 +35,17 @@
|
|||||||
(a prefix, then the structure) instead of slicing the whole file; the
|
(a prefix, then the structure) instead of slicing the whole file; the
|
||||||
open-ended `&file_data[addr..]` slices in these modules are gone. Bounds
|
open-ended `&file_data[addr..]` slices in these modules are gone. Bounds
|
||||||
errors keep their values (absolute position, file length).
|
errors keep their values (absolute position, file length).
|
||||||
|
- Reads sized by untrusted fields are bounded by what the parser uses, so
|
||||||
|
a crafted size cannot turn one structure into a read of the rest of the
|
||||||
|
file on a range backend: local-heap names are read in growing pieces
|
||||||
|
(64 bytes first) rather than to the end of the data segment; a fractal
|
||||||
|
heap indirect block is read up to the entry covering the object (the
|
||||||
|
whole block only when that entry is unallocated); paged fixed and
|
||||||
|
extensible array data blocks over 1 MiB are read page by page, only the
|
||||||
|
pages in use; and a block under one checksum whose claimed size runs
|
||||||
|
past the end of the file fails its bounds check before any read (with
|
||||||
|
the `checksum` feature). An object header's prefix is one read (was
|
||||||
|
two).
|
||||||
- Structures still indexed by a v2 B-tree — dense attribute storage, a SOHM
|
- Structures still indexed by a v2 B-tree — dense attribute storage, a SOHM
|
||||||
B-tree index and huge fractal-heap objects found through their B-tree —
|
B-tree index and huge fractal-heap objects found through their B-tree —
|
||||||
are not converted yet (the
|
are not converted yet (the
|
||||||
@@ -42,11 +56,27 @@
|
|||||||
- **No behaviour change**, checked three ways (2026-09-26, tank): every
|
- **No behaviour change**, checked three ways (2026-09-26, tank): every
|
||||||
existing test passes unchanged; the conformance sweep
|
existing test passes unchanged; the conformance sweep
|
||||||
(`conformance/run.sh --no-fetch`) gives a byte-identical `results.json`
|
(`conformance/run.sh --no-fetch`) gives a byte-identical `results.json`
|
||||||
and identical per-file probe output for all 697 files at `f2ff2c4` and on
|
at `f2ff2c4` and on this branch, and identical per-file probe output for
|
||||||
this branch; and a transcript of every converted `&[u8]` function's
|
696 of the 697 files — the exception, `cve-2025-2310.h5`, reports one of
|
||||||
result over the fixtures, the conformance corpus and the h5py-written
|
two errors depending on which parallel chunk decode fails first, at
|
||||||
files below (748 files, 7 603 object headers) is byte-identical between
|
`f2ff2c4` as on this branch; and a transcript of every converted `&[u8]`
|
||||||
the two builds.
|
function's result over the fixtures, the conformance corpus and the
|
||||||
|
h5py-written files below (748 files, 7 603 object headers) is
|
||||||
|
byte-identical between the two builds.
|
||||||
|
- **Speed on local files** (provisional: tank was shared with other jobs;
|
||||||
|
both builds linked into one binary and timed alternately, 200 rounds;
|
||||||
|
new Criterion bench `clawhdf5/benches/local_metadata_bench.rs` over a
|
||||||
|
400-group version-1 file, `clawhdf5-format/tests/fixtures/v1_groups_400.h5`):
|
||||||
|
against `f2ff2c4`, listing the file through the facade is 2.7% faster,
|
||||||
|
`ObjectHeader::parse` is within ±1%, symbol-table nodes and the group
|
||||||
|
B-tree walk are about 19% faster (their entry loops were tightened),
|
||||||
|
local-heap names and `resolve_group_children` 1.5–3% faster. The same
|
||||||
|
harness run on two copies of the old code differs by up to 2%. The
|
||||||
|
Criterion bench itself, old and new as separate binaries run alternately
|
||||||
|
(3 rounds), agrees except for `ObjectHeader::parse`, which it puts about
|
||||||
|
7% slower (25.9 vs 24.1 µs for 401 headers) while the listing that
|
||||||
|
parses those headers is 5–8% faster; that one remains unexplained and is
|
||||||
|
to be rechecked on an idle machine.
|
||||||
- New equivalence harness `clawhdf5-format/tests/storage_equivalence.rs`:
|
- New equivalence harness `clawhdf5-format/tests/storage_equivalence.rs`:
|
||||||
every converted parser runs over the file as a slice and over
|
every converted parser runs over the file as a slice and over
|
||||||
`storage::CountingStorage` — a `Storage` that serves an in-memory buffer
|
`storage::CountingStorage` — a `Storage` that serves an in-memory buffer
|
||||||
@@ -55,8 +85,12 @@
|
|||||||
the fixtures, files h5py writes for it (extensible arrays with super
|
the fixtures, files h5py writes for it (extensible arrays with super
|
||||||
blocks and paged data blocks, paged fixed arrays, large v1 and dense
|
blocks and paged data blocks, paged fixed arrays, large v1 and dense
|
||||||
groups, a user block, SOHM list and B-tree indexes, dense, shared and
|
groups, a user block, SOHM list and B-tree indexes, dense, shared and
|
||||||
committed-type attributes), and with `CLAWHDF5_STORAGE_CORPUS=<dir>` a
|
committed-type attributes, fixed and extensible array data blocks over
|
||||||
corpus (all 653 HDF5 files of the conformance corpus pass). Milestones M2
|
1 MiB, whole and truncated), and with `CLAWHDF5_STORAGE_CORPUS=<dir>` a
|
||||||
|
corpus (all 653 HDF5 files of the conformance corpus pass). Only the
|
||||||
|
structures listed above as not converted may answer
|
||||||
|
`ContiguousStorageRequired`, and only in the checks that reach them; a
|
||||||
|
converted parser falling back to the whole file fails it. Milestones M2
|
||||||
and M3 extend it.
|
and M3 extend it.
|
||||||
|
|
||||||
### Chunked full reads (2026-09-26)
|
### Chunked full reads (2026-09-26)
|
||||||
|
|||||||
@@ -261,6 +261,11 @@ every `file_data[a..b]` becomes `file.read_at(a, b - a)?`.
|
|||||||
(binary size matters for wasm); `&dyn Storage` costs one indirect call per
|
(binary size matters for wasm); `&dyn Storage` costs one indirect call per
|
||||||
structure read, negligible next to parsing. Hot raw-data loops keep their
|
structure read, negligible next to parsing. Hot raw-data loops keep their
|
||||||
speed through `as_contiguous()`.
|
speed through `as_contiguous()`.
|
||||||
|
*M1 outcome:* `&dyn` was not negligible for small structures — with it,
|
||||||
|
`ObjectHeader::parse` was ~25% and a 400-group facade listing ~14% slower
|
||||||
|
than the slice code (provisional, shared machine). The cores are now
|
||||||
|
generic (`S: Storage + ?Sized`), so the `&[u8]` wrappers get a `[u8]`
|
||||||
|
instance and `dyn Storage` is one more instance, not one per backend.
|
||||||
|
|
||||||
### (b) A page-cache "virtual slice"
|
### (b) A page-cache "virtual slice"
|
||||||
|
|
||||||
@@ -392,7 +397,7 @@ fast path within benchmark noise.
|
|||||||
B-tree v1/v2, fractal heap, fixed/extensible array, symbol table, group
|
B-tree v1/v2, fractal heap, fixed/extensible array, symbol table, group
|
||||||
v1/v2, shared messages, attributes, fill value, data layout — one commit
|
v1/v2, shared messages, attributes, fill value, data layout — one commit
|
||||||
each. The old `&[u8]` signature stays as a thin wrapper over the new one
|
each. The old `&[u8]` signature stays as a thin wrapper over the new one
|
||||||
(`fn parse(data: &[u8], ..) { parse_in(data as &dyn Storage, ..) }`), so
|
(`fn parse(data: &[u8], ..) { parse_in(data, ..) }`, generic core), so
|
||||||
callers and the other crates don't move yet.
|
callers and the other crates don't move yet.
|
||||||
- Replace the 5 open-ended slices and 38 `len()` checks with bounded reads.
|
- Replace the 5 open-ended slices and 38 `len()` checks with bounded reads.
|
||||||
|
|
||||||
@@ -433,8 +438,13 @@ Total: roughly 6–10 engineer-weeks for M0–M4 (estimate, not measured).
|
|||||||
- `impl Storage for [u8]` returns `Cow::Borrowed` — no copy, no allocation.
|
- `impl Storage for [u8]` returns `Cow::Borrowed` — no copy, no allocation.
|
||||||
- Hot loops (raw-data copies, contiguous typed reads, `read_selection_native`)
|
- Hot loops (raw-data copies, contiguous typed reads, `read_selection_native`)
|
||||||
branch once on `as_contiguous()` and then run today's code.
|
branch once on `as_contiguous()` and then run today's code.
|
||||||
- `&dyn` dispatch is per structure, not per byte; parse code keeps working on
|
- The parser cores are generic over `S: Storage + ?Sized`, so the in-memory
|
||||||
|
instance has no dispatch at all; a remote backend behind `&dyn Storage`
|
||||||
|
pays one indirect call per structure read. Parse code keeps working on
|
||||||
the returned slice.
|
the returned slice.
|
||||||
|
- Reads sized by untrusted fields cover what the parser uses (see
|
||||||
|
`CHANGELOG.md`, M1), so a hostile size costs no read of the rest of the
|
||||||
|
file.
|
||||||
- Gate: `crates/clawhdf5/benches/mmap_bench.rs`, the concurrent-read benches in
|
- Gate: `crates/clawhdf5/benches/mmap_bench.rs`, the concurrent-read benches in
|
||||||
`clawhdf5-bench`, and the conformance run time, before and after each M1/M2
|
`clawhdf5-bench`, and the conformance run time, before and after each M1/M2
|
||||||
commit, on an otherwise idle machine. Anything outside noise blocks the
|
commit, on an otherwise idle machine. Anything outside noise blocks the
|
||||||
|
|||||||
Reference in New Issue
Block a user