Skip to content

treedb: retain more leaf-ref build pages - #3568

Draft
snissn wants to merge 1 commit into
mainfrom
codex/3537-leaf-ref-cache-buildpage-reuse
Draft

treedb: retain more leaf-ref build pages#3568
snissn wants to merge 1 commit into
mainfrom
codex/3537-leaf-ref-cache-buildpage-reuse

Conversation

@snissn

@snissn snissn commented Jul 6, 2026

Copy link
Copy Markdown
Owner

Objective

Prepare the focused #3537 leaf-ref cache slice by reducing persisted leaf-ref build-page churn in the zipper scratch cache while preserving the cache-owned byte lifetime invariant.

Fixes #3537 after the deferred benchmark gate passes.

Context

Post-#3561/#3565 main evidence reported TreeDB/zipper.(*mergeScratch).acquireLeafRefCacheBuildPage as a current full-run allocation surface in /tmp/gomap_alloc_loop_main_post3561_all_20260706_060625 at commit 6f2d94def23551ccb1c6bdb35514bc72733d88e0.

This PR intentionally does not claim performance validation yet. A local 10M gate attempt was stopped because another worker was running a concurrent 10M gate on the same machine, contaminating benchmark evidence. Those partial artifacts must not be used for merge decisions.

Non-goals

  • No changes to TreeDB memtable append-only arenas.
  • No changes to TreeDB caching batch helper ownership.
  • No changes to durable persistence, value-log pointer semantics, WAL behavior, or GC/rewrite policy.
  • No weakening of the documented rule that cached leaf page bytes must not alias scratch buffers that can be mutated later.

Design

  • Increase mergeLeafRefCachePageKeep from 4096 to 8192.
  • Reuse remains scratch-local and lifetime-gated: cache-owned build pages are returned to scratch reuse only after endLeafRefCache clears the active leaf-ref cache map.
  • No global hot-path pool is introduced.

Scope

Includes:

  • TreeDB/zipper/zipper.go: leaf-ref cache build-page retention cap.
  • TreeDB/zipper/zipper_test.go: overflow lifetime regression coverage.

Excludes:

  • TreeDB/internal/memtable
  • TreeDB/caching
  • Benchmark harness changes

Correctness

Tests run locally:

GOWORK=off go test ./TreeDB/zipper -count=1
# pass: ok github.com/snissn/gomap/TreeDB/zipper 1.071s

GOWORK=off go test ./TreeDB -run 'TestReopenVerify_WALOn_Checkpoint|TestReopenVerify_WALOn_WriteSync' -count=1
# pass: ok github.com/snissn/gomap/TreeDB 4.847s

GOWORK=off go test ./TreeDB/db -run 'TestValueLogGC_RemovesUnreferencedSegment' -count=1
# pass: ok github.com/snissn/gomap/TreeDB/db 0.537s

git diff --check
# pass

Focused coverage added:

  • TestZipperLeafRefCacheOverflowBuildPagesStayPinnedUntilClose forces more active cache build pages than the retention cap, persists both retained and overflow pages through leafPageCacheAdoptOwned, mutates later active build buffers, and verifies cached leaf-log loads still return the original leaf entries without using the leaf page reader.

Performance

Benchmarks run:

  • Deferred. The required clean 10M sequential paired gate is delegated to the main orchestrator.

Invalidated local attempt:

  • A local 10M gate was stopped after cross-load was detected from another worker running a concurrent 10M gate on the same machine. Do not use those partial artifacts for merge decisions.

Required before merge:

GOWORK=off GOMAXPROCS=8 ./bin/unified-bench \
  -profile durable -dbs treedb -keys 10000000 -valsize 128 -batchsize 8000 \
  -test batch_delete,random_delete,batch_random -checkpoint-between-tests \
  -treedb-journal-lanes=1 -progress=false \
  -profile-dir "$OUT" -path-label native-fastpath
GOWORK=off ./bin/benchprof -profiles-dir "$OUT"

Gate status:

  • Not merge-ready until the main orchestrator records clean before/after throughput, total allocation totals, and acquireLeafRefCacheBuildPage flat bytes/objects with no material throughput regression.

Rollout / Toggle Plan

  • No runtime toggle. The change is a bounded scratch-local retention cap.
  • Revert path is the single constant change plus the test.

AI Review Loop

  • Codex latest-head review: not requested yet; wait until benchmark gate evidence is attached.
  • Copilot latest-head review: not requested yet; wait until benchmark gate evidence is attached.
  • CodeRabbit latest-head review: not requested yet; wait until benchmark gate evidence is attached.

@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: fa8a5492-f5fd-46ef-be63-75d51269fd74

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/3537-leaf-ref-cache-buildpage-reuse

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@snissn

snissn commented Jul 6, 2026

Copy link
Copy Markdown
Owner Author

Clean sequential coordinator gate for #3537 / #3568:

Artifact root: /tmp/gomap_3537_leafref_clean_gate_20260706_072528
Baseline: origin/main 6f2d94def23551ccb1c6bdb35514bc72733d88e0
Candidate: 4c33610eaccc8e7fe09d57b9b0b6a207148e466c
Command shape: durable 10M TreeDB, -test batch_delete,random_delete,batch_random, -checkpoint-between-tests, -treedb-journal-lanes=1, -profile-dir, then benchprof.

Throughput:

test main ops/sec PR ops/sec delta
batch_delete 780,388 730,055 -6.5%
random_delete 579,301 571,819 -1.3%
batch_random 91,519 166,286 +81.7%

Allocation totals:

test main alloc space / objects PR alloc space / objects delta
batch_delete 3,678.14 MB / 54,024 3,690.02 MB / 120,119 +11.88 MB / +66,095
random_delete 2,487.56 MB / 54,949 4,143.97 MB / 55,540 +1,656.41 MB / +591
batch_random 6,924.98 MB / 927,409 6,683.25 MB / 878,965 -241.73 MB / -48,444

The specific #3537 target did not reproduce in this focused gate:

alloc_objects focus=acquireLeafRefCacheBuildPage
main batch_delete: 0
main random_delete: 0
main batch_random: 0
PR   batch_delete: 0
PR   random_delete: 0
PR   batch_random: 0

alloc_space focus=acquireLeafRefCacheBuildPage
main batch_delete: 0
main random_delete: 0
main batch_random: 0
PR   batch_delete: 0
PR   random_delete: 0
PR   batch_random: 0

Visible top allocators in this gate are now mostly HashSorted / hashArena / bytes.Clone; the only visible zipper rows were small batch_random entries:

main batch_random alloc_objects: mergeScratch.acquireChildRefBatch 23,767 objects (2.56%)
PR   batch_random alloc_objects: mergeScratch.acquirePendingLeafPagePersists 5,462 objects (0.62%)

Decision: leave this PR draft / gate-blocked. It has useful lifetime coverage, but the constant change is not mergeable as an allocation optimization on this evidence because the target does not reproduce and there are material allocation/throughput regressions in selected delete workloads.

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.

TreeDB alloc loop: recycle delete leaf-ref cache pages

1 participant