Skip to content

refactor: extract FreemapRecycle (structural recycle + freemap commit/alloc/persist/reclaim) — extraction PR 4 - #78

Merged
Xof merged 1 commit into
mainfrom
feature/transaction-freemap-recycle
Jun 23, 2026
Merged

refactor: extract FreemapRecycle (structural recycle + freemap commit/alloc/persist/reclaim) — extraction PR 4#78
Xof merged 1 commit into
mainfrom
feature/transaction-freemap-recycle

Conversation

@Xof

@Xof Xof commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

Task 4 of the transaction.rs extraction (after #76, #77) — the hardest and most durability-critical step. Extracts FreemapRecycle: the structural-page recycle cluster and the freemap commit/alloc/persist/reclaim machinery, out of TransactionManager into an owned unit. Pure behavior-preserving refactor; the existing suite is the oracle.

What changed

  • FreemapRecycle (in transaction/freemap.rs) now owns the five fields hint / structural_reuse / structural_superseded / pending_structural_frees / session_owned. No code outside freemap.rs touches them directly (surface-checked).
  • Transient-tree trio take_tree / cow_alloc_into / put_tree replaces take_freemap_tree / cow_alloc / put_freemap_tree. The six allocation call sites (data-page insert, ht_insert, 3× membership staging, the update/delete path) keep the same one-tree-per-insert lifetime — the alloc closure operates on a single persistent FreeMapTree whose session_owned dedup accumulates across the multiple COWs of one structure insert. Only the five fields moved behind the interface, not the tree's lifetime.
  • Commit-path machinery (mark_free_committed_path, persist, reclaim_orphans) became FreemapRecycle methods taking cache: &mut PageCache + roots: &mut Roots. persist takes txn_freed_pages: &[u64] (the old mem::take/restore dance — only a borrow-checker dodge — is gone). reclaim_orphans takes savepoint_active + superblock_count params; the savepoint guard de-inversion is savepoint_active = !savepoints.is_empty().
  • begin/commit/rollback lifecycle hooks delegate to FreemapRecycle; the per-stream reasoning (one-commit-defer rotation, why rollback leaves pending intact, the two-free-streams) was relocated from lifecycle.rs into the FreemapRecycle docs (single source of truth).

Verification

  • cargo test578 passing, 0 failed (273 chisel-lib) — identical to the pre-refactor baseline; all named durability oracle tests green (I18 persist_freemap_does_not_reuse_committed_live_pages, the structural_recycle_* defer family, the reclaim_freemap_orphans_* sweep tests, the I28 commit-drain).
  • cargo clippy --workspace --all-targets -- -D warnings clean; cargo fmt --check clean; pytest119 passing.
  • Four-lens adversarial review (line-level equivalence, durability-invariant counterfactual hunt, tree-lifetime/borrow correctness across all six sites, reclaim/mark deep-check): all four verdicts clean. Independently confirmed cow_alloc/structural_extend byte-identical, the put_tree-before-? error-path ordering preserved at every site, and the one-continuous-cache-borrow change equivalent. Only NITs surfaced — one unobservable internal-state diff in persist's error path (behind the I1 poison wall) and one relocated comment that is more accurate than the original.

Next: Task 5 (CommitProtocol), one PR off updated main.

@Xof
Xof merged commit f8acb06 into main Jun 23, 2026
6 checks passed
@Xof
Xof deleted the feature/transaction-freemap-recycle branch June 23, 2026 03:42
@github-actions

Copy link
Copy Markdown

🚦 Bench results: PR vs main

⚠️ 1 regression(s) detected across 1 scenario/mode pair(s)

Scenario Mode Δ throughput Worst Δ
mutation-log sqlite-strict -0.7% p50 +5.6% ⚠️
document-store chisel-mem +0.1%
document-store chisel-strict +0.3%
document-store redb-strict +3.0%
document-store sqlite-strict +0.4%
mutation-log chisel-mem +1.1%
mutation-log chisel-strict -0.6%
mutation-log redb-strict -0.2%
ycsb-a chisel-mem +2.8%
ycsb-a chisel-strict -0.8%
ycsb-a redb-strict -0.1%
ycsb-a sqlite-strict +0.3%
ycsb-b chisel-mem +1.5%
ycsb-b chisel-strict +0.4%
ycsb-b redb-strict -0.3%
ycsb-b sqlite-strict -1.3%
Per-scenario detail (4 metrics × cells)

document-store

Mode Throughput p50 p95 p99
chisel-mem 19198 ops/s → 19226 ops/s (+0.1%) 14.7 µs → 14.6 µs (-0.5%) 134.8 µs → 135.2 µs (+0.4%) 648.1 µs → 652.9 µs (+0.8%)
chisel-strict 2960 ops/s → 2968 ops/s (+0.3%) 24.0 µs → 22.7 µs (-5.4%) 884.7 µs → 868.1 µs (-1.9%) 3.18 ms → 2.97 ms (-6.5%)
redb-strict 4012 ops/s → 4134 ops/s (+3.0%) 12.2 µs → 11.9 µs (-2.5%) 472.8 µs → 422.5 µs (-10.6%) 1.35 ms → 1.28 ms (-5.4%)
sqlite-strict 5125 ops/s → 5143 ops/s (+0.4%) 20.8 µs → 21.1 µs (+1.7%) 347.2 µs → 353.9 µs (+1.9%) 1.35 ms → 1.32 ms (-2.0%)

mutation-log

Mode Throughput p50 p95 p99
chisel-mem 52404 ops/s → 52959 ops/s (+1.1%) 22.9 µs → 22.7 µs (-1.1%) 26.6 µs → 26.4 µs (-1.0%) 35.6 µs → 35.2 µs (-1.0%)
chisel-strict 1778 ops/s → 1766 ops/s (-0.6%) 290.8 µs → 303.3 µs (+4.3%) 641.0 µs → 679.4 µs (+6.0%) 18.40 ms → 16.93 ms (-8.0%)
redb-strict 1805 ops/s → 1802 ops/s (-0.2%) 149.8 µs → 151.3 µs (+1.0%) 276.6 µs → 249.9 µs (-9.7%) 33.22 ms → 33.04 ms (-0.5%)
sqlite-strict 5045 ops/s → 5009 ops/s (-0.7%) 93.7 µs → 98.9 µs (+5.6%) ⚠️ 262.7 µs → 274.4 µs (+4.4%) 368.4 µs → 403.9 µs (+9.6%)

ycsb-a

Mode Throughput p50 p95 p99
chisel-mem 37458 ops/s → 38489 ops/s (+2.8%) 42.1 µs → 40.7 µs (-3.5%) 60.1 µs → 58.0 µs (-3.5%) 77.7 µs → 79.1 µs (+1.8%)
chisel-strict 2262 ops/s → 2244 ops/s (-0.8%) 257.3 µs → 253.6 µs (-1.4%) 736.5 µs → 792.7 µs (+7.6%) 1.88 ms → 2.05 ms (+9.1%)
redb-strict 2683 ops/s → 2681 ops/s (-0.1%) 116.7 µs → 117.5 µs (+0.7%) 208.1 µs → 205.5 µs (-1.2%) 432.3 µs → 354.0 µs (-18.1%)
sqlite-strict 133048 ops/s → 133473 ops/s (+0.3%) 7.7 µs → 7.7 µs (-0.1%) 10.1 µs → 10.1 µs (+0.0%) 11.7 µs → 11.7 µs (+0.3%)

ycsb-b

Mode Throughput p50 p95 p99
chisel-mem 166154 ops/s → 168648 ops/s (+1.5%) 4.5 µs → 4.6 µs (+0.5%) 43.2 µs → 41.7 µs (-3.7%) 48.9 µs → 46.9 µs (-4.1%)
chisel-strict 21908 ops/s → 21985 ops/s (+0.4%) 7.1 µs → 6.6 µs (-6.5%) 257.6 µs → 259.9 µs (+0.9%) 645.8 µs → 657.9 µs (+1.9%)
redb-strict 27218 ops/s → 27147 ops/s (-0.3%) 3.0 µs → 3.0 µs (+0.7%) 116.6 µs → 118.8 µs (+1.8%) 179.0 µs → 181.0 µs (+1.1%)
sqlite-strict 163164 ops/s → 161108 ops/s (-1.3%) 6.3 µs → 6.3 µs (+0.7%) 8.2 µs → 8.4 µs (+1.8%) 10.4 µs → 10.7 µs (+2.4%)
Generated by chisel-bench-diff at 2026-06-23T03:57:28Z. Compares PR HEAD against main. Never blocks merge — signal, not gate. Thresholds: throughput 5%, p50 5%, p95 10%, p99 10%.

Xof added a commit that referenced this pull request Jun 23, 2026
…ion deferred (I141) (#80)

The 2026-06-22 review's god-module SMELL was worked through four unit
extractions (SlotPacker #77, FreemapRecycle #78, CommitProtocol #79,
FaultInjector #76); the final StagingTxn extraction is deliberately
deferred. The candidate-prepare/install staging vocabulary is shared
across allocate_inner (staging.rs) and update_inner/delete_inner
(mutate.rs), so a context-based extraction cannot be contained to
staging.rs without dragging the delicate mutation paths through a
mechanical wrapper change. Recorded as future work to be done
incrementally if/when those paths are touched, not re-triggered from
the SMELL alone.
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