perf(btreemap): batch the per-field node reads and writes - #441
Closed
hpeebles wants to merge 1 commit into
Closed
Conversation
Loading a node performed one memory read per field: every child address, key size, key, and value size separately — dozens of reads per node. A memory access costs a fixed amount per call plus a fee per byte, so the load now fetches the front of the initial page in one read and parses the fields out of the buffer, with any field beyond the read falling back to an individual read as before. The read is sized so that it only ever buys bytes the load will use. For key types bounded by at most 64 bytes the header pins down the extent of the children and key areas, and that estimate is all that is fetched. For larger or unbounded key bounds the interleaved layout offers no extent worth prefetching — the sizes the walk needs sit between payloads it skips — so those types read exactly as before; an earlier draft that read a fixed 1 KiB for them regressed the large-value benchmarks by up to 8%. Saving mirrors this: fields are serialized into a buffer flushed in as few writes as possible, except that slices of 48 bytes or more are written directly, uncopied, flushing whatever is buffered around them. The threshold is measured, and lower than intuition suggests: copying into the buffer costs a few instructions per byte while a separate write costs a fixed amount for the call and its page mapping, which puts the crossover around forty bytes — earlier drafts with a 128- and 512-byte threshold both measured thousands of instructions per save slower on nodes with ~128-byte keys and values. The `NodeWriter` keeps sole ownership of the overflow-address field at bytes [7, 15): it updates the chain pointer in place as overflow pages are allocated and released, so the save skips over it rather than risk clobbering a pointer the writer just installed. The writer's unused field helpers are gone. Full canbench sweep: 147 improved / 0 regressed / 105 unchanged of 252, median -3.9%, best -23.5%; the largest positive movement is +1.8% on a benchmark whose code path is untouched (noise). Memory-manager maps gain the most (-16 to -23%), followed by maps with small keys and values (-10 to -19%). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
|
|
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves BTreeMap v2 node I/O performance by batching many small stable-memory reads/writes into fewer larger operations while preserving existing per-field behavior in cases where batching would over-fetch and regress performance.
Changes:
- Batch the front-of-page node load into a single read (with per-field fallbacks) and clamp the read to the end of memory to avoid traps.
- Serialize node saves through a small coalescing buffer, directly writing large slices without copying, and avoid writing the overflow-pointer field from the serializer (leaving it to
NodeWriter). - Add a regression test ensuring batched reads near the end of memory clamp rather than trap; remove unused
NodeWriterconvenience methods.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/btreemap/node/v2.rs |
Implements batched initial-page reads during load and buffered/coalesced writes during save; adjusts overflow handling to avoid clobbering the chain pointer. |
src/btreemap/node/tests.rs |
Adds a test covering clamping the batched read when a node is near the end of memory. |
src/btreemap/node/io.rs |
Removes unused NodeWriter helpers (write_u32, write_u64, write_struct) after save-side refactor. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+201
to
+207
| let page_u32 = |reader: &NodeReader<M>, offset: Address| -> u32 { | ||
| let start = offset.get() as usize; | ||
| match page.get(start..start + 4) { | ||
| Some(bytes) => u32::from_le_bytes(bytes.try_into().unwrap()), | ||
| None => read_u32(reader, offset), | ||
| } | ||
| }; |
Contributor
Author
|
I've just been told that there are plans to make stable read/writes as fast (or thereabouts) as heap read/writes. Which would make this PR unnecessary. |
Contributor
Author
|
It turns out that even if stable read/writes are just as fast as to the heap, the changes in this PR would still actually result in roughly the same proportional improvements, since with these changes applied there are far fewer read/writes through the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Loading a node performed one memory read per field — every child address, key size, key, and value size separately — dozens of reads per node, and saving mirrored that with one write per field. A memory access costs a fixed amount per call plus a fee per byte, so this PR batches both directions while keeping every path that can't be batched profitably on its existing per-field behavior.
Load
The load now fetches the front of the initial page in one read and parses the fields out of the buffer, with any field beyond the read falling back to an individual read. The read is sized so that it only ever buys bytes the load will use:
The previous
CHILDREN_BATCH_READ_THRESHOLD = 4heuristic ("individual reads are faster for ≤ 4 children") is gone: re-measured under the new read pattern, it no longer paid for itself — the sweep below shows no regression on any internal-node-heavy benchmark.Save
Fields are serialized into a buffer flushed in as few writes as possible, except that slices of ≥ 48 bytes are written directly, uncopied, flushing whatever is buffered around them. The threshold is measured, and lower than intuition suggests: copying into the buffer costs a few instructions per byte, while a separate write costs a fixed amount for the call and its page mapping, putting the crossover around forty bytes. (Drafts with 128- and 512-byte thresholds both measured thousands of instructions per save slower on nodes with ~128-byte keys and values.)
The save no longer writes the overflow-address field at bytes [7, 15): the
NodeWriterowns it, updating the chain pointer in place as overflow pages are allocated and released, andfinish()writes theNULLterminator into it whenever the node ends up with no overflow pages — so even a fresh node at a reused address cannot retain a stale pointer. (The old explicit write was redundant with that terminator.)A new test covers the one newly reachable trap: a node placed so close to the end of memory that the batched-read estimate extends past the last byte ever written; the read clamps rather than traps.
Benchmarks
Full
canbenchsweep againstmain(canbench 0.2.0, all 252 benchmarks):Zero regressions. The single largest positive movement (+1.84%,
remove_vec_64_128) is on a code path this PR does not touch (unbounded keys take the unchanged per-field route) and is within canbench's noise threshold.Top improvements:
mem_manager_remove_u64_u64mem_manager_insert_u64_u64mem_manager_get_u64_u64get_blob8_u64get_zipf_100k_u64_u64_nocacheinsert_blob8_u64remove_blob8_u64mem_manager_get_u64_blob512pop_last_blob_32_0scan_iter_rev_1k_0bcontains_blob_8_128get_blob_4_128remove_100k_u64_u64_nocacheMemory-manager maps gain the most (−16 to −23%), followed by maps with small keys and values (−10 to −19%); sparse-node scans and range queries land around −14 to −16%. One footnote:
insert_vec_8_128shows a one-time +1 page (64 KiB) heap high-water increase from the save buffer — a peak-allocation effect, not a per-operation cost.🤖 Generated with Claude Code