Repository navigation
Walk trees for compaction without allocating per page - #75
Closed
venkat1701 wants to merge 2 commits into
Closed
venkat1701 wants to merge 2 commits into
venkat1701 wants to merge 2 commits into
Conversation
Codecs that hold tree references only had mapRefs, which builds a new record even when every reference maps to itself. They now return the same record in that case, and a new forEachRef visits the references without building anything.
The liveness walk and relocation tracked pages in a HashMap of boxed longs and created a visitor lambda per leaf value. They now use a primitive open-addressing set, sized from the segment page counts, and one walker object per walk. Pairing old and new pages no longer reloads copied leaves from disk.
venkat1701
force-pushed
the
perf/compaction-allocation
branch
from
October 8, 2026 20:56
14fa0c0 to
5d6371d
Compare
Collaborator
Author
|
Closing this. #81 replaced tree-rewriting compaction with moving pages through the page directory, so most of this stack no longer applies. A review also found two data-loss bugs in #73 (a retirement generation older than a commit that still used the moved pages, and bulk-loaded pages left behind in a victim). The pieces that still help, the posting list decode, records reused by mapRefs, and the checkpoint crash test, are in #82. |
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.
Part of #50. Stacked on #74 (which is stacked on #73). Merge those first; this PR's base then moves along.
I profiled one full compaction with JFR on top of #74. It allocated about 1 GiB, and almost none of it was copying:
mapRefswith a do-nothing mapper, which still built a new record per value (361 MiB)HashMap<Long, Integer>Changes:
AtomCodec,PostingsCodec, the property index root) return the same record frommapRefswhen nothing changed, and get aforEachRefthat visits references without building anythingVisitedPages: a small primitive open-addressing map from page id to size, pre-sized from the segments' page counts, used by the liveness walk and by relocation's "pages reached" setProfile of one full compaction (warm cache):
Benchmark, HStore only, 3 alternating runs each against #74, all checksums identical:
mixed.writep99Default segments improve clearly; 16 MiB segments are unchanged within noise (the ranges overlap). The reason: in the benchmark compaction runs right after a reopen, so the node cache is cold. What the compaction thread still allocates (965 MiB in a JFR of that window) is page read buffers.
PageStore.readallocates one per page: 344 MiB for the walk's cache misses and 267 MiB for reading victim pages to copy. Reusing read buffers is the next step, and it would help every cold read too.Tested:
./mvnw installpasses (109 tests); each commit compiles on its ownVisitedPagesTest: 200,000 random ids against aHashMap, and id 0 rejectedAtomCodecTestandPostingsCodecTest: identity mapping keeps the same object,forEachRefsees the same references asmapRefs, a real change still creates a new recordCompactionTest.livenessCountsEveryPageOfEveryRetainedRootOnce:engine.liveness()equals the old Map-based walk over every retained root after churnCompactionTestcases pass