perf(agent): open a store without copying the whole file
`read_from_disk` memory-mapped the file and then copied the entire mapping into a `Vec` to hand to `File::from_bytes` — but `File::open` memory-maps it itself whenever the facade's `mmap` feature is on, which it is by default. So every open mapped the file, memcpy'd all of it, and parsed the copy. Store open at 100k x 384: 455 ms -> 327 ms, about 28% faster (two runs after the change, 326.8 and 328.1 ms). Peak memory is unchanged, which is worth saying because the opposite is the natural assumption. The footprint harness now tracks a high-water mark next to the retained figure, and it shows the peak falling after the parse, during the index build — so a buffer allocated and freed inside the parse never reaches it. Confirmed rather than assumed: holding a deliberate extra copy of the whole file across the parse leaves the peak exactly where it was, which is also what proved the instrument was working before trusting its answer. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This commit is contained in:
@@ -123,6 +123,31 @@ vectors, and recall is measured against brute-force ground truth rather than
|
|||||||
against the f32 index, whose own approximation errors a re-scored search is
|
against the f32 index, whose own approximation errors a re-scored search is
|
||||||
entitled to get right.
|
entitled to get right.
|
||||||
|
|
||||||
|
### Opening a store (`read_from_disk`)
|
||||||
|
|
||||||
|
`HDF5Memory::open` memory-mapped the file, copied the whole mapping into a
|
||||||
|
`Vec`, and handed that to `File::from_bytes` — while `File::open` memory-maps
|
||||||
|
the file itself. Dropping the copy takes **store open from 455 ms to 327 ms**
|
||||||
|
at 100 000 x 384 (`--e2e-only --full`; two runs after the change, 326.8 and
|
||||||
|
328.1 ms).
|
||||||
|
|
||||||
|
It does **not** lower the process's peak memory, which is worth stating
|
||||||
|
precisely because it is the obvious thing to assume. The harness now reports a
|
||||||
|
high-water mark alongside the retained figure:
|
||||||
|
|
||||||
|
| N | reopened MiB | peak during open MiB |
|
||||||
|
|---:|---:|---:|
|
||||||
|
| 1 000 | 4 | 5 |
|
||||||
|
| 10 000 | 44 | 61 |
|
||||||
|
| 100 000 | 399 | 562 |
|
||||||
|
|
||||||
|
The peak is set *after* the parse, by the index build, so a buffer allocated
|
||||||
|
and freed during the parse never reaches the high-water mark. Holding a
|
||||||
|
deliberate extra copy of the file across the whole parse leaves the peak
|
||||||
|
unmoved, which is how this was confirmed rather than assumed. What the change
|
||||||
|
saves is the copy itself: a full-file memcpy on every open, and the transient
|
||||||
|
that goes with it.
|
||||||
|
|
||||||
## Read harness
|
## Read harness
|
||||||
|
|
||||||
Produced by `cargo run --release -p clawhdf5-bench --bin read_harness`: a 4096 x
|
Produced by `cargo run --release -p clawhdf5-bench --bin read_harness`: a 4096 x
|
||||||
|
|||||||
@@ -2,6 +2,16 @@
|
|||||||
|
|
||||||
## Unreleased
|
## Unreleased
|
||||||
|
|
||||||
|
### Performance
|
||||||
|
- `clawhdf5-agent`: **opening a store is ~28% faster** (455 ms -> 327 ms at
|
||||||
|
100k x 384). `read_from_disk` memory-mapped the file and then copied the
|
||||||
|
entire mapping into a `Vec` for `File::from_bytes`, when `File::open`
|
||||||
|
memory-maps it directly — so every open paid a full-file memcpy for nothing.
|
||||||
|
Process peak memory is unchanged: the peak falls after the parse, during the
|
||||||
|
index build, so the transient never reached the high-water mark. The
|
||||||
|
footprint harness now reports that peak next to the retained figure, which
|
||||||
|
is how this was checked rather than assumed.
|
||||||
|
|
||||||
### Integrity
|
### Integrity
|
||||||
- `clawhdf5-format`: **Fixed and Extensible Array chunk indexes now verify
|
- `clawhdf5-format`: **Fixed and Extensible Array chunk indexes now verify
|
||||||
their checksums** (the `checksum` feature, on by default). Every structure
|
their checksums** (the `checksum` feature, on by default). Every structure
|
||||||
|
|||||||
@@ -113,13 +113,11 @@ pub type StoreState = (MemoryConfig, MemoryCache, SessionCache, KnowledgeCache);
|
|||||||
/// [`read_from_disk`], plus the checkpoint's [`WalMark`] (if any) so the
|
/// [`read_from_disk`], plus the checkpoint's [`WalMark`] (if any) so the
|
||||||
/// caller can skip WAL entries this file already contains.
|
/// caller can skip WAL entries this file already contains.
|
||||||
pub fn read_from_disk_with_mark(path: &Path) -> Result<(StoreState, Option<WalMark>), MemoryError> {
|
pub fn read_from_disk_with_mark(path: &Path) -> Result<(StoreState, Option<WalMark>), MemoryError> {
|
||||||
let mmap = clawhdf5_io::MmapReader::open(path).map_err(MemoryError::Io)?;
|
// `File::open` memory-maps the file itself (the facade's `mmap` feature is
|
||||||
|
// on by default). Mapping it here and handing over `as_bytes().to_vec()`
|
||||||
// Advise the OS we'll need the whole file for parsing
|
// did the same work and then copied the whole store — a second full copy
|
||||||
mmap.advise_willneed(0, mmap.len());
|
// of the file, live for the whole parse, on top of the mapping.
|
||||||
|
let file = clawhdf5::File::open(path)
|
||||||
// Parse the HDF5 file from the mmap'd bytes
|
|
||||||
let file = clawhdf5::File::from_bytes(mmap.as_bytes().to_vec())
|
|
||||||
.map_err(|e| MemoryError::Hdf5(format!("cannot open {}: {e}", path.display())))?;
|
.map_err(|e| MemoryError::Hdf5(format!("cannot open {}: {e}", path.display())))?;
|
||||||
|
|
||||||
let (mut config, cache, sessions, knowledge) = schema::validate_and_load(&file)?;
|
let (mut config, cache, sessions, knowledge) = schema::validate_and_load(&file)?;
|
||||||
@@ -133,9 +131,7 @@ pub fn read_from_disk_with_mark(path: &Path) -> Result<(StoreState, Option<WalMa
|
|||||||
pub fn read_from_disk_with_meta(
|
pub fn read_from_disk_with_meta(
|
||||||
path: &Path,
|
path: &Path,
|
||||||
) -> Result<(StoreState, schema::CheckpointMeta), MemoryError> {
|
) -> Result<(StoreState, schema::CheckpointMeta), MemoryError> {
|
||||||
let mmap = clawhdf5_io::MmapReader::open(path).map_err(MemoryError::Io)?;
|
let file = clawhdf5::File::open(path)
|
||||||
mmap.advise_willneed(0, mmap.len());
|
|
||||||
let file = clawhdf5::File::from_bytes(mmap.as_bytes().to_vec())
|
|
||||||
.map_err(|e| MemoryError::Hdf5(format!("cannot open {}: {e}", path.display())))?;
|
.map_err(|e| MemoryError::Hdf5(format!("cannot open {}: {e}", path.display())))?;
|
||||||
let (mut config, cache, sessions, knowledge) = schema::validate_and_load(&file)?;
|
let (mut config, cache, sessions, knowledge) = schema::validate_and_load(&file)?;
|
||||||
config.path = path.to_path_buf();
|
config.path = path.to_path_buf();
|
||||||
|
|||||||
@@ -230,13 +230,27 @@ struct CountingAllocator;
|
|||||||
|
|
||||||
static LIVE_BYTES: std::sync::atomic::AtomicI64 = std::sync::atomic::AtomicI64::new(0);
|
static LIVE_BYTES: std::sync::atomic::AtomicI64 = std::sync::atomic::AtomicI64::new(0);
|
||||||
|
|
||||||
|
/// High-water mark of [`LIVE_BYTES`] since it was last reset.
|
||||||
|
///
|
||||||
|
/// Live bytes at a checkpoint cannot see a buffer that was allocated and
|
||||||
|
/// freed in between, and that is exactly the shape of a transient copy —
|
||||||
|
/// which still has to fit in memory while it exists.
|
||||||
|
static PEAK_BYTES: std::sync::atomic::AtomicI64 = std::sync::atomic::AtomicI64::new(0);
|
||||||
|
|
||||||
|
fn note_peak(live: i64) {
|
||||||
|
PEAK_BYTES.fetch_max(live, std::sync::atomic::Ordering::Relaxed);
|
||||||
|
}
|
||||||
|
|
||||||
// SAFETY: every method forwards to the system allocator with the same layout
|
// SAFETY: every method forwards to the system allocator with the same layout
|
||||||
// it was given, and only adds bookkeeping around it.
|
// it was given, and only adds bookkeeping around it.
|
||||||
unsafe impl std::alloc::GlobalAlloc for CountingAllocator {
|
unsafe impl std::alloc::GlobalAlloc for CountingAllocator {
|
||||||
unsafe fn alloc(&self, layout: std::alloc::Layout) -> *mut u8 {
|
unsafe fn alloc(&self, layout: std::alloc::Layout) -> *mut u8 {
|
||||||
let ptr = unsafe { std::alloc::System.alloc(layout) };
|
let ptr = unsafe { std::alloc::System.alloc(layout) };
|
||||||
if !ptr.is_null() {
|
if !ptr.is_null() {
|
||||||
LIVE_BYTES.fetch_add(layout.size() as i64, std::sync::atomic::Ordering::Relaxed);
|
let live = LIVE_BYTES
|
||||||
|
.fetch_add(layout.size() as i64, std::sync::atomic::Ordering::Relaxed)
|
||||||
|
+ layout.size() as i64;
|
||||||
|
note_peak(live);
|
||||||
}
|
}
|
||||||
ptr
|
ptr
|
||||||
}
|
}
|
||||||
@@ -249,10 +263,9 @@ unsafe impl std::alloc::GlobalAlloc for CountingAllocator {
|
|||||||
unsafe fn realloc(&self, ptr: *mut u8, layout: std::alloc::Layout, new_size: usize) -> *mut u8 {
|
unsafe fn realloc(&self, ptr: *mut u8, layout: std::alloc::Layout, new_size: usize) -> *mut u8 {
|
||||||
let new_ptr = unsafe { std::alloc::System.realloc(ptr, layout, new_size) };
|
let new_ptr = unsafe { std::alloc::System.realloc(ptr, layout, new_size) };
|
||||||
if !new_ptr.is_null() {
|
if !new_ptr.is_null() {
|
||||||
LIVE_BYTES.fetch_add(
|
let delta = new_size as i64 - layout.size() as i64;
|
||||||
new_size as i64 - layout.size() as i64,
|
let live = LIVE_BYTES.fetch_add(delta, std::sync::atomic::Ordering::Relaxed) + delta;
|
||||||
std::sync::atomic::Ordering::Relaxed,
|
note_peak(live);
|
||||||
);
|
|
||||||
}
|
}
|
||||||
new_ptr
|
new_ptr
|
||||||
}
|
}
|
||||||
@@ -266,6 +279,19 @@ fn heap_bytes() -> u64 {
|
|||||||
LIVE_BYTES.load(std::sync::atomic::Ordering::Relaxed).max(0) as u64
|
LIVE_BYTES.load(std::sync::atomic::Ordering::Relaxed).max(0) as u64
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Start watching for a new high-water mark from the current live total.
|
||||||
|
fn reset_peak() {
|
||||||
|
PEAK_BYTES.store(
|
||||||
|
LIVE_BYTES.load(std::sync::atomic::Ordering::Relaxed),
|
||||||
|
std::sync::atomic::Ordering::Relaxed,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The highest live total seen since [`reset_peak`].
|
||||||
|
fn peak_bytes() -> u64 {
|
||||||
|
PEAK_BYTES.load(std::sync::atomic::Ordering::Relaxed).max(0) as u64
|
||||||
|
}
|
||||||
|
|
||||||
fn mib(bytes: u64) -> f64 {
|
fn mib(bytes: u64) -> f64 {
|
||||||
bytes as f64 / (1 << 20) as f64
|
bytes as f64 / (1 << 20) as f64
|
||||||
}
|
}
|
||||||
@@ -580,19 +606,24 @@ fn bench_footprint(n: usize) {
|
|||||||
let path = mem.config().path.clone();
|
let path = mem.config().path.clone();
|
||||||
drop(mem);
|
drop(mem);
|
||||||
let before_open = heap_bytes();
|
let before_open = heap_bytes();
|
||||||
|
reset_peak();
|
||||||
let reopened = HDF5Memory::open(&path).unwrap();
|
let reopened = HDF5Memory::open(&path).unwrap();
|
||||||
let after_open = heap_bytes();
|
let after_open = heap_bytes();
|
||||||
let loaded = after_open.saturating_sub(before_open);
|
let loaded = after_open.saturating_sub(before_open);
|
||||||
|
// Peak over the open, not just what it leaves behind: a buffer allocated
|
||||||
|
// and freed during the parse never shows up in the live total.
|
||||||
|
let peak = peak_bytes().saturating_sub(before_open);
|
||||||
drop(reopened);
|
drop(reopened);
|
||||||
|
|
||||||
let raw = (n * DIM * 4) as u64;
|
let raw = (n * DIM * 4) as u64;
|
||||||
println!(
|
println!(
|
||||||
"| {n} | {:.0} | {:.0} | {:.0} | {:.0} | {:.0} | {:.2}x |",
|
"| {n} | {:.0} | {:.0} | {:.0} | {:.0} | {:.0} | {:.0} | {:.2}x |",
|
||||||
mib(raw),
|
mib(raw),
|
||||||
mib(after_entries.saturating_sub(base)),
|
mib(after_entries.saturating_sub(base)),
|
||||||
mib(after_store.saturating_sub(after_entries)),
|
mib(after_store.saturating_sub(after_entries)),
|
||||||
mib(after_indexes.saturating_sub(after_store)),
|
mib(after_indexes.saturating_sub(after_store)),
|
||||||
mib(loaded),
|
mib(loaded),
|
||||||
|
mib(peak),
|
||||||
loaded as f64 / raw as f64,
|
loaded as f64 / raw as f64,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
@@ -644,9 +675,9 @@ fn main() {
|
|||||||
if args.iter().any(|a| a == "--footprint") {
|
if args.iter().any(|a| a == "--footprint") {
|
||||||
println!("\n### Resident memory, {DIM}-dim f32\n");
|
println!("\n### Resident memory, {DIM}-dim f32\n");
|
||||||
println!(
|
println!(
|
||||||
"| N | vectors (raw) | entries MiB | store MiB | indexes MiB | reopened MiB | reopened / raw |"
|
"| N | vectors (raw) | entries MiB | store MiB | indexes MiB | reopened MiB | peak during open MiB | reopened / raw |"
|
||||||
);
|
);
|
||||||
println!("|---:|---:|---:|---:|---:|---:|---:|");
|
println!("|---:|---:|---:|---:|---:|---:|---:|---:|");
|
||||||
for &n in sizes {
|
for &n in sizes {
|
||||||
bench_footprint(n);
|
bench_footprint(n);
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user