refactor: extract SlotPacker (R1 live-slot packing) — extraction PR 2

src/defrag.rs

@@ -406,8 +406,8 @@ mod tests {
         // Run defrag with a generous sparse threshold (0.5) so the source pages
         // definitely qualify.  Steps 5 and 7 execute inside the SAME transaction.
         //
-        // Step 5 triggers at least one `allocate_data_page` → `allocate_first`
-        // call, which COWs a freemap leaf and pushes its old id into
+        // Step 5 triggers at least one data-page allocation → `cow_alloc` →
+        // `allocate_first` call, which COWs a freemap leaf and pushes its old id into
         // `structural_superseded`.  Step 7 must then exclude that id so only the
         // forged orphan is reclaimed.
         tm.begin().unwrap();

src/page_cache.rs

@@ -624,7 +624,7 @@ impl PageCache {
     ///     directly; any dirty entries at id >= watermark are pages
     ///     allocated AFTER the savepoint, exactly the ones we want
     ///     gone. Savepoint-bearing transactions disable freemap reuse
-    ///     (see `allocate_data_page`) so there are no dirty reused-id
+    ///     (see `cow_alloc`) so there are no dirty reused-id
     ///     pages to worry about.
     ///
     /// If a future caller needs "truncate without dropping any dirty
@@ -736,7 +736,7 @@ impl PageCache {
     pub fn claim_page(&mut self, page_id: u64) -> Result<()> {
         // ISSUES.md I20: enforce the "freemap never returns an already-dirty
         // id" invariant in debug builds. The only legitimate caller is
-        // `allocate_data_page` via the freemap, which — post-I18 — keeps the
+        // `cow_alloc` (the shared freemap-aware allocator), which — post-I18 — keeps the
         // at-risk id sets out of the in-commit free pool. A violation here
         // would silently drop the caller's pending writes on `page_id`; an
         // assertion surfaces the bug at its source rather than hours later
@@ -1325,8 +1325,8 @@ mod tests {
     // Regression test for ISSUES.md I20. claim_page previously silently
     // dropped any prior dirty writes on the claimed id: it unconditionally
     // removed the existing cache entry and inserted a fresh zeroed one.
-    // The only legitimate caller is `allocate_data_page` via the freemap,
-    // which must never return an id already dirty in the current txn —
+    // The only legitimate caller is `cow_alloc` (the shared freemap-aware
+    // allocator), which must never return an id already dirty in the current txn —
     // but the invariant was unenforced. I20 adds a debug_assert so the
     // rule is checked in debug builds; a violation fires immediately
     // rather than surfacing hours later as silent data loss.

src/transaction/freemap.rs

@@ -123,9 +123,12 @@ impl TransactionManager {
 
     // --- Freemap-aware page allocation (ISSUES.md R2) ---
     //
-    // `allocate_data_page` is the single entry point for allocating a
-    // fresh data page during a transaction. It first tries to reuse an
-    // id from `current_freemap` and falls back to extending the file.
+    // `cow_alloc` (free function above) is the shared freemap-aware allocator
+    // for a fresh page during a transaction: it first tries to reuse an id from
+    // the committed freemap tree and falls back to extending the file. Data-page
+    // allocation reaches it through the freemap dance hoisted into
+    // `SlotPacker::insert`'s caller (`insert_into_data_page` in packing.rs); the
+    // handle-table / membership COW paths reach it through `ht_insert` below.
     //
     // Two important scoping rules:
     //
@@ -183,27 +186,6 @@ impl TransactionManager {
         self.freemap_session_owned = std::mem::take(&mut tree.session_owned);
     }
 
-    pub(super) fn allocate_data_page(&mut self) -> Result<u64> {
-        let reuse = self.savepoints.is_empty();
-        let mut tree = self.take_freemap_tree();
-        let id = {
-            let mut cache = self.cache.borrow_mut();
-            cow_alloc(
-                &mut cache,
-                &mut tree,
-                &mut self.freemap_hint,
-                &mut self.structural_reuse,
-                reuse,
-            )
-        };
-        // Write back tree growth + drain supersedes even on error: the freemap
-        // pages were extended (never freed), so on a non-fatal failure they are
-        // harmless above-watermark scratch, and the session set must still be
-        // returned so a retry/commit in the same transaction stays consistent.
-        self.put_freemap_tree(tree);
-        id
-    }
-
     /// COW `handle`'s handle-table entry to `entry`, installing the new root
     /// and queuing the superseded spine pages for freemap reclamation at
     /// commit. Shared by `allocate`, `update`, and `set_client_byte`.

src/transaction/lifecycle.rs

@@ -109,8 +109,7 @@ impl TransactionManager {
         // R1: clone the live-slot counts and reset the insert cursor.
         // The cursor is always None at begin — it only tracks pages
         // allocated during the current transaction.
-        self.current_live_slots = self.committed_live_slots.clone();
-        self.insert_cursor = None;
+        self.packer.begin();
         self.active_txn = true;
         self.savepoints.clear();
         self.txn_freed_pages.clear();
@@ -230,7 +229,8 @@ impl TransactionManager {
         }
 
         // I28: drain the page cache BEFORE persist_freemap runs. Without
-        // this, `persist_freemap`'s own `allocate_data_page` can trip
+        // this, `persist_freemap`'s own freemap-page allocation
+        // (`structural_extend` → `new_page`) can trip
         // `maybe_evict`'s spill-or-CacheFull decision (every existing entry
         // dirty, nothing evictable, and either spillway disabled or full)
         // and return `ChiselError::CacheFull` or `ChiselError::SpillwayFull`.
@@ -331,8 +331,7 @@ impl TransactionManager {
         // separate in-memory freemap copy to advance.
         // R1: promote the live-slot counts. The cursor is per-transaction
         // and gets reset for the next begin().
-        self.committed_live_slots = self.current_live_slots.clone();
-        self.insert_cursor = None;
+        self.packer.commit();
         self.active_txn = false;
         self.savepoints.clear();
         // txn_freed_pages were already marked free in the new committed freemap
@@ -446,8 +445,7 @@ impl TransactionManager {
         self.structural_reuse.clear();
         self.freemap_session_owned.clear();
         // R1: revert the live-slot counts and drop the insert cursor.
-        self.current_live_slots = self.committed_live_slots.clone();
-        self.insert_cursor = None;
+        self.packer.rollback();
         self.active_txn = false;
         self.savepoints.clear();
         self.txn_freed_pages.clear();

src/transaction/mod.rs

@@ -29,8 +29,9 @@
 //     only in memory. A crash mid-transaction discards all dirty pages from cache
 //     and the on-disk superblock still references the prior committed snapshot.
 //   - NOTE: `new_page()` (file extension) extends the underlying file immediately;
-//     `allocate_data_page` prefers reuse from `current_freemap` but also calls
-//     through to `new_page()` when the freemap is empty. Either way, any pages
+//     data-page allocation (via `cow_alloc`) prefers reuse from the committed
+//     freemap tree but also calls through to `new_page()` when the freemap is
+//     empty. Either way, any pages
 //     extended-but-uncommitted before a crash are harmless because nothing in the
 //     committed superblock references them, and the rollback path
 //     (`cache.truncate(committed_roots.total_pages)` — I3) actively shrinks the
@@ -46,7 +47,8 @@
 
 use std::cell::{Cell, RefCell};
 // I127 (ISSUES.md, 2026-06-21): FxHashMap (not std SipHash) for the per-op
-// slot-accounting maps below (current/committed_live_slots, Savepoint.live_slots).
+// slot-accounting maps (the SlotPacker live-slot maps in packing.rs,
+// Savepoint.live_slots below, and the open-time scan map in recovery.rs).
 // Keys are trusted local u64 page ids — no DoS surface — so SipHash is pure cost,
 // exactly the I77 rationale; that pass converted the page cache/LRU but missed
 // these. FxHashMap is a drop-in std HashMap with a faster non-DoS-resistant hasher.
@@ -242,34 +244,13 @@ pub struct TransactionManager {
     // COW and mutate a live committed page in place), which is exactly why it is
     // transaction-scoped, not cross-transaction.
     freemap_session_owned: FxHashSet<u64>,
-    // Live-slot count per data page (ISSUES.md R1). Tracks how many
-    // handle-table entries currently point at each data page — this
-    // is the information needed to decide when a page is fully empty
-    // and can be returned to the freemap. `committed_live_slots` is
-    // the durable state (rebuilt at open time by scanning the handle
-    // table); `current_live_slots` is the in-transaction working copy.
-    //
-    // Kept in memory rather than on disk because updating a slot count
-    // on a committed data page would require COW, and COWing a data
-    // page would require rewriting every handle_table entry that
-    // points into it — an O(live-slots-in-page) amplification per
-    // delete that shadow paging does not handle well.
-    committed_live_slots: FxHashMap<u64, u32>,
-    current_live_slots: FxHashMap<u64, u32>,
-    // Per-transaction "insert cursor" (ISSUES.md R1). The id of a data
-    // page allocated earlier in the current transaction that still has
-    // free space. New values pack into it until it fills, at which
-    // point a new page is allocated and becomes the new cursor.
-    //
-    // `None` at the start of each transaction. Only set for pages
-    // allocated during THIS transaction (so they're dirty in the cache
-    // and safe to modify). A committed data page is never the cursor —
-    // that would require COW, which is prohibitively expensive for data
-    // pages (every handle_table entry pointing at the page would need
-    // to be rewritten). Disabled entirely when savepoints are active,
-    // same as freemap reuse (R2): the savepoint-snapshot cost becomes
-    // manageable when only one code path interacts with packing state.
-    insert_cursor: Option<u64>,
+    // R1 slot-packing state (live-slot counts per data page + the insert
+    // cursor), extracted into its own owned unit. No code outside `packing.rs`
+    // touches the inner fields; `TransactionManager` reaches packing through
+    // the narrow `SlotPacker` interface (the `insert_into_data_page` /
+    // `release_data_slot` wrappers plus the lifecycle/savepoint/stats hooks).
+    // See the `packing` module header for the live-slot and cursor models.
+    packer: packing::SlotPacker,
     // Poison flag (ISSUES.md I1). Once set, every public entry point returns
     // ChiselError::Poisoned until the manager is dropped. Set by commit() on
     // any error in the commit protocol, and by `poison_on_fatal()` for any