Serve dep-graph edge lists in place from the retained file mapping#60
Open
xmakro wants to merge 1 commit into
Open
Serve dep-graph edge lists in place from the retained file mapping#60xmakro wants to merge 1 commit into
xmakro wants to merge 1 commit into
Conversation
Decoding the previous dep graph used to copy every edge byte out of the file into a freshly allocated buffer (edge_list_data), the largest allocation and copy of the load. Instead, retain the file mapping for the session and serve the edge lists directly from it: the on-disk varint encoding was already the in-memory representation, and each node's edge header now records a position in the file instead of a position in the copied buffer. Decoding a record shrinks to reading its fixed header and skipping over the edges. This also removes the DEP_NODE_PAD padding and the capacity-estimation heuristic: the fixed-size overread at the end of an edge list stays in bounds because the records are always followed by the per-kind node counts, the session count and the fixed-size tail. Retaining an incremental file's mapping for the session and lazily decoding from it is the same lifecycle the query result cache already uses (OnDiskCache::serialized_data). It also moves the edge bytes from anonymous heap memory to evictable file-backed pages. On Windows the mapping must not outlive the load, because the save later renames over the mapped file; there the bytes are copied out once, a single copy comparable to the per-record copies this replaces. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Standalone against upstream master (
perf/base-0713= upstream 3659db0), no dependency on the carry/append-only stack. This is the upstream-first wedge: it lands the "previous dep-graph file stays available for the session" infrastructure as a pure simplification + win, which #42/#43/#44/#58/#59 then build on.Decoding the previous dep graph used to copy every edge byte out of the file into a freshly allocated buffer (
edge_list_data), the largest allocation and copy of the load. Instead, retain the file mapping for the session and serve the edge lists directly from it: the on-disk varint encoding was already the in-memory representation, and each node's edge header now records a position in the file instead of a position in the copied buffer. Decoding a record shrinks to reading its fixed header and skipping over the edges.This also deletes
DEP_NODE_PADand the capacity-estimation heuristic: the fixed-size overread at the end of an edge list stays in bounds because the records are always followed by the per-kind node counts, the session count and the fixed-size tail.Precedent: retaining an incremental file's mapping for the session and lazily decoding from it is the query result cache's existing lifecycle (
OnDiskCache::serialized_data, dropped just before the save insave.rs: "Drop the memory map so that we can remove the file and write to it"). On Windows the mapping must not outlive the load because the save later renames over the mapped file, so the bytes are copied out once — a single copy comparable in total bytes to the per-record copies this replaces. (TheOnDiskCache-style drop-before-rename shape was considered instead of the copy, but releasing the backing mid-session would put synchronization onedge_targets_from, the hottest read path of the marking walk.)Local rustc-perf (primary crates, instructions:u), idle machine, this change alone vs its base commit:
27 of 60 cells improved by at least 0.25%, 0 regressed, 0 failed benchmarks.
On max-rss: the retained mapping keeps the record headers resident at peak, which the deleted copy discarded after decode. Those pages are clean and file-backed — the kernel reclaims them under pressure with no swap I/O — whereas the buffer this PR deletes was unswappable anonymous heap. Peak-RSS accounting does not credit evictability, so it shows as a regression; effective memory behavior under pressure is better, not worse.
Validation:
tests/incremental178 passed / 0 failed; 12 generations each of incr-unchanged, of alternating real edits, and of alternating edits under-Zincremental-verify-ich: every generation clean.