Skip to content

Walk trees for compaction without allocating per page - #75

Closed
venkat1701 wants to merge 2 commits into
perf/compaction-byte-copyfrom
perf/compaction-allocation
Closed

venkat1701 wants to merge 2 commits into
perf/compaction-byte-copyfrom
perf/compaction-allocation

Conversation

@venkat1701

Copy link
Copy Markdown
Collaborator

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:

  • liveness walk, 724 MiB:
    • to list a record's nested trees it called mapRefs with a do-nothing mapper, which still built a new record per value (361 MiB)
    • every visited page went into a HashMap<Long, Integer>
    • a capturing lambda was created per leaf value
  • pairing old and new pages, 266 MiB: it reloaded freshly copied leaves from disk just to learn there was nothing below them

Changes:

  • codecs that hold tree references (AtomCodec, PostingsCodec, the property index root) return the same record from mapRefs when nothing changed, and get a forEachRef that visits references without building anything
  • VisitedPages: 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" set
  • the walk uses one walker object per walk instead of a lambda per value
  • pairing skips loading the copy when the old node is a leaf with no nested trees

Profile of one full compaction (warm cache):

before after
total 1,044 MiB 173 MiB (-83%)
liveness walk 724 MiB 72 MiB
pairing 266 MiB 29 MiB
relocation, encoding, copies 54 MiB 72 MiB

Benchmark, HStore only, 3 alternating runs each against #74, all checksums identical:

during a full compaction default segments 16 MiB segments
allocated (whole process) 1,255 → 936 MiB 1,456 → 1,030 MiB
GC pauses 474 → 220 ms 317 → 379 ms
commit p99 266 → 142 ms 233 → 297 ms
mixed.write p99 119 → 62 ms 62 → 68 ms

Default 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.read allocates 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:

  • full ./mvnw install passes (109 tests); each commit compiles on its own
  • VisitedPagesTest: 200,000 random ids against a HashMap, and id 0 rejected
  • AtomCodecTest and PostingsCodecTest: identity mapping keeps the same object, forEachRef sees the same references as mapRefs, a real change still creates a new record
  • CompactionTest.livenessCountsEveryPageOfEveryRetainedRootOnce: engine.liveness() equals the old Map-based walk over every retained root after churn
  • all 12 CompactionTest cases pass

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

Copy link
Copy Markdown
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.

@venkat1701 venkat1701 closed this Oct 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant