perf: shard pool guard to improve performance

docs/backlogs/000175-scalable-shared-resource-lifetime-management.md → docs/backlogs/closed/000175-scalable-shared-resource-lifetime-management.md

@@ -10,7 +10,7 @@ Task 000247 performance work exposed the issue after boxing `TrxInner` removed l
 
 A 50-million-operation 4-thread/16-session perf profile attributed 49.86% of candidate aggregate CPU time to `__aarch64_ldadd8_acq_rel`, versus 25.04% on `origin/main`. Candidate caller attribution was 16.59% through `WeakEngineRef::upgrade -> EngineRef::new -> retain_runtime_ref`, 13.74% through `EngineAdmission::drop -> release_admission`, and 19.54% through `TrxAttachment::drop -> EngineRef::drop -> release_runtime_ref`. At the same time, inline-core `memcpy` fell from 23.06% to 0.67%.
 
-Static review found that session operation entries already block component teardown for active transaction work, while `runtime_refs` remains necessary in the current design for detached pins such as `SessionObserverPin` and as a waitable notification layer over `Arc` ownership. The current counter is therefore broader than the hot-path lifetime proof requires.
+At the task 000247 revision, static review found that session operation entries already blocked component teardown for active transaction work, while `runtime_refs` still covered detached pins such as `SessionObserverPin` and supplied waitable notification over `Arc` ownership. That counter was therefore broader than the hot-path lifetime proof required; task 000254 later removed it after moving observer authority into session lifecycle state.
 
 The fresh task-resolution matrix reproduced the contended result. Against
 `origin/main` `768842e8e8c1`, the candidate reduced median `stmt-noop` latency
@@ -51,26 +51,91 @@ session-registry lookup, or attachment guard-bundle clone remained in the
 candidate stack. This evidence identifies the contended domain but does not
 yet prove why the ownership-path speedup changes page-frame contention.
 
+An exact-revision reproduction on 2026-08-08 isolated the cause. Task 000255
+moved `PoolGuards` construction from `SessionState::new` to one canonical
+`EnginePools` bundle. `PoolGuard` does not retain one `Arc` per page frame; it
+retains a `SyncQuiescentGuard<()>`, and every page lookup clones that wrapper
+into `PageLatchGuard`. The canonical bundle therefore made metadata- and
+row-pool page accesses from every session update the same two `Arc` strong-count
+cache lines. The underlying arena `QuiescentGuardCount` remained pool-global
+but was not touched by those clones.
+
+Seven alternating release samples reproduced unique `index-stream` medians of
+73,046 ns for the pre-task baseline and 105,969 ns for task 000255 at 4/16. A
+controlled candidate that restored fresh guard roots per session reduced the
+median to 77,109 ns without changing cache-hit or row counts; its 1/1 median
+was unchanged within noise. Relaxed and release `Arc` helpers fell from 28.96%
+of profiled samples to 5.90%, matching the pre-task profile. Non-unique streams
+showed the same result. This proves the regression is cross-session contention
+on the canonical `PoolGuard` roots, not additional page operations, statement
+synchronization, or index scheduling.
+
+The implemented session-root correction was then measured against exact
+`HEAD` `916471d9c3cb`. Seven alternating release samples reduced the unique
+4/16 median from 101,072 ns to 73,938 ns (-26.85%) and the non-unique median
+from 101,336 ns to 77,525 ns (-23.50%). Unique and non-unique 1/1 medians
+changed by -0.05% and +0.16%, respectively. A final candidate profile
+attributed 5.83% of samples to relaxed/release atomic helpers, below the 10%
+acceptance ceiling and consistent with session-local rather than cross-session
+refcount traffic.
+
+The follow-up API audit renamed the misleading `BufferPool::pool_guard()`
+accessor to `create_base_guard()` and made its construction cost explicit.
+Production root creation is now limited to engine/session owners, catalog
+bootstrap, and detached eviction workers. Readonly cache misses and
+invalidation, CoW writes, DDL, recovery, and checkpoint work receive the
+session- or operation-scoped guard instead, so those paths cannot silently
+acquire another pool-global quiescent keepalive.
+
+After rebasing onto `e5152e8`, a fresh-root current-working-tree smoke used
+100 streams of 1,000 rows at 4 threads/16 sessions after one warmup. Unique
+and non-unique average latency was 77,752.510 ns and 77,550.420 ns per stream,
+respectively, with 100,000 rows returned and zero failures in each run. This
+bounded check is consistent with the earlier seven-sample medians; it is not a
+replacement for that paired matrix.
+
+## Resolution
+
+The implemented result uses a measured hybrid lifetime policy rather than one
+universal counter. Task 000254 removed engine-global runtime reference
+accounting in favor of registered session operations, session-local observers,
+mandatory permits, and component-worker ownership. Task 000255 removed the
+engine weak upgrade, registry lookup, and guard-bundle clone from transaction
+checkout. The final buffer follow-up shards the high-frequency outer
+`PoolGuard` `Arc` roots per session while retaining the pool-global
+`QuiescentGuardCount` only for deliberate base-root acquisition. The packed
+`EngineAdmission` counter remains solely as the operation-start versus shutdown
+race gate and is released before effectful work.
+
+The repository-wide ownership inventory is maintained in
+`docs/engine-component-lifetime.md`: it records admission and shutdown
+authorities, session and observer handles, mandatory work, component workers,
+quiescent ownership, pool-root provenance, and teardown order. Production
+`create_base_guard()` sites were audited down to engine/session owners, catalog
+bootstrap, and detached eviction workers; page access, invalidation, COW, DDL,
+recovery, and checkpoint paths receive an existing owner-scoped guard.
+
 ## Deferred From (Optional)
 
 docs/tasks/000247-statement-public-transaction-cancellation-ownership.md; docs/rfcs/0025-session-coordinated-cancellation-cleanup-ownership.md Phase 2; docs/tasks/000255-session-local-runtime-reachability.md
 
 ## Deferral Context (Optional)
 
 - Defer Reason: Task 000247 is scoped to statement cancellation ownership and its bounded performance work. Redesigning lifetime and destruction policy across the engine, pools, transaction system, and other component resources materially broadens both architecture and shutdown proof obligations, so it should be planned and reviewed independently rather than folded into Phase 2. Task 000255 is scoped to session-local runtime reachability; changing buffer-frame ownership or index-stream scheduling to address the newly measured contention would cross that boundary without a proven cause.
-- Findings: The session coordinator is already an authoritative teardown blocker for active public/private transactions and foreground operations, making the custom counted `EngineRef` pin redundant for much of that hot path. It is not yet authoritative for every runtime user: detached observer pins do not occupy the active operation slot, and standalone/internal strong pins rely on `runtime_refs` for efficient shutdown notification. `Arc::strong_count` remains the final ownership backstop, but it has no drop notification, explaining why the separate counter exists. Removing local `TrxInner` copy work exposed contention on shared lifecycle cache lines rather than adding new lifecycle operations. Task 000255 confirmed that session-local weak reachability materially improves statement and transaction no-op paths. Its contended index-stream profiles show increased time in existing page-frame `Arc` increments and decrements, while the new session runtime access is negligible; the relationship between faster statement boundaries and buffer-page contention remains unproven.
+- Findings: At deferral time, the session coordinator was already an authoritative teardown blocker for active public/private transactions and foreground operations, making the custom counted `EngineRef` pin redundant for much of that hot path. Detached observer pins did not occupy the active operation slot, so the then-current design still relied on `runtime_refs` for efficient shutdown notification. Task 000254 subsequently replaced that dependency with session-local observer accounting. Removing local `TrxInner` copy work exposed contention on shared lifecycle cache lines rather than adding new lifecycle operations. Task 000255 confirmed that session-local weak reachability materially improves statement and transaction no-op paths. Its canonical pool-guard bundle accidentally changed the sharing domain of the outer `SyncQuiescentGuard` `Arc` from one root per session to one root per engine pool. Restoring one fresh bundle per `SessionState` preserves the existing lifetime proof while sharding page-guard clone/drop traffic.
 - Direction Hint:
   Start with the narrow performance result: make session-coordinated transaction and foreground-operation access use the session/component lifecycle proof instead of globally counted runtime pins, while retaining admission for the operation-start versus shutdown race. Explicitly account for detached observers, terminal-publication gaps, stale cleanup jobs, and worker-owned work before narrowing or removing `runtime_refs`.
 
   Then evaluate the general resource-lifetime policy. Compare sharded counters with centralized arena/owner destruction rather than assuming one universal mechanism. Prefer centralized destruction when resource lifetime is already bounded by an engine/component owner and individual early reclamation is unnecessary; prefer sharding only where independent lifetime and thread mobility still require counting. Avoid weakening memory ordering or deleting counters without a replacement shutdown and destruction proof.
 
-  Reproduce the task 000255 index-stream profile independently before changing
-  buffer or index code. Separate page-frame atomic operation count from
-  per-operation contention latency, and test whether statement-boundary
-  synchronization, shared root traversal, range overlap, or object/cache-line
-  placement explains the 4/16-only result. Do not optimize index-stream by
-  adding duplicate runtime capabilities or unsafe cached pointers unless a
-  profile-backed design proves that session runtime access is causal.
+  Preserve one fresh `PoolGuards` root bundle per session while continuing to
+  borrow it from transaction attachments and operation pins. Keep canonical
+  engine roots for non-session work, and test both pool identity and outer
+  `Arc` root identity so provenance-preserving centralization cannot silently
+  recreate the contention. Removing the remaining per-page `Arc` operations
+  would require scoped page guards plus owned promotion for detached I/O and
+  should remain separate unless its measured incremental benefit justifies the
+  broader lifetime/API change.
 
 ## Scope Hint
 
@@ -97,3 +162,11 @@ When a backlog item is moved to `docs/backlogs/closed/`, append:
 - Reference: <task/issue/pr reference>
 - Closed At: <YYYY-MM-DD>
 ```
+
+## Close Reason
+
+- Type: implemented
+- Detail: Tasks 000254 and 000255 removed engine-global runtime accounting and statement lookup traffic. The follow-up sharded pool-guard roots per session, audited base-root creation boundaries, preserved lifecycle authority, and restored contended index-stream performance without a repeatable 1/1 regression.
+- Closed By: backlog close
+- Reference: docs/tasks/000254-remove-engine-runtime-reference-accounting.md; docs/tasks/000255-session-local-runtime-reachability.md; docs/engine-component-lifetime.md; doradb-storage/src/session.rs; doradb-storage/src/buffer/mod.rs
+- Closed At: 2026-08-09

docs/engine-component-lifetime.md

@@ -578,11 +578,28 @@ That provenance rule gives three guarantees:
 - stable owner identity survives cloning because guards keep the owner alive
 - page guards and arena state can rely on one exact pool provenance source
 
-`EngineCore` owns one canonical `EnginePools` capability containing the four
-typed pool handles and one prebuilt `PoolGuards` bundle. Session-coordinated
-operations borrow that bundle through `SessionRuntime`; transaction attachments
-do not clone it. `PoolGuards` remains only a named bundle of individually
-branded guards and does not weaken the single-owner provenance rule.
+`EngineCore` owns one `EnginePools` capability containing the four typed pool
+handles and a prebuilt `PoolGuards` bundle for engine-owned work. Each
+`SessionState` constructs one fresh bundle from those same handles and
+session-coordinated operations borrow it through `SessionRuntime`; transaction
+attachments do not clone it. The engine and session bundles carry identical
+`PoolIdentity` values but distinct outer `Arc` roots.
+
+This distinction is intentional. The arena's `QuiescentGuardCount` remains
+pool-global and is acquired once for each long-lived root. Page guards clone
+the outer `SyncQuiescentGuard` on every retained page access, so allocating one
+root per session keeps that high-frequency `Arc` traffic on session-local cache
+lines. Moving session paths back to the canonical engine bundle would preserve
+provenance and memory safety but reintroduce cross-session refcount contention.
+`PoolGuards` remains only a named bundle of individually branded guards and
+does not weaken the single-owner provenance rule.
+
+`BufferPool::create_base_guard()` names the exceptional root-construction
+operation explicitly. It is appropriate at engine, session, component
+bootstrap, detached-worker, and isolated test-fixture ownership boundaries.
+Page access, cache-miss retries, invalidation, CoW mutation, DDL, recovery, and
+checkpoint helpers must instead accept an owner-scoped guard and clone it only
+when an owned sub-operation can outlive the borrow.
 
 ## Arena And Page-Guard Lifetime Rules
 

docs/rfcs/0025-session-coordinated-cancellation-cleanup-ownership.md

@@ -119,7 +119,7 @@ RFC-0026. [D5] [D6] [D7]
 
 - [B1] `docs/backlogs/closed/000170-session-coordinated-cancellation-cleanup.md`
 - [B2] `docs/backlogs/closed/000124-statement-execution-cancellation-safety.md`
-- [B3] `docs/backlogs/000175-scalable-shared-resource-lifetime-management.md`
+- [B3] `docs/backlogs/closed/000175-scalable-shared-resource-lifetime-management.md`
 - [B4] `docs/backlogs/000171-exact-family-lock-system-redesign.md`
 
 ## Decision
@@ -223,7 +223,8 @@ work. Lifecycle events are created only for an actual close or shutdown waiter.
 Phase 2 improved uncontended statement and transaction boundaries but exposed
 repeatable contended statement-boundary and stream regressions. The
 cancellation result was accepted with that fixed overhead recorded as explicit
-debt in backlog 000175; detailed samples and flamegraphs remain in task 000247.
+debt in backlog 000175; the debt was subsequently resolved, while detailed
+original samples and flamegraphs remain in task 000247.
 [D6] [B3] [U4]
 
 ### 5. RFC-0026 owns all post-Phase-2 execution design
@@ -319,7 +320,8 @@ numbered phase. [D8]
   - Implementation Summary: Implemented cancellation-safe public statement
     ownership, synchronous residual-effect settlement, whole-transaction
     cleanup, boxed transaction cores, and a reusable public-session core cache;
-    accepted measured contention debt is tracked by backlog 000175.
+    accepted measured contention debt was later resolved through backlog
+    000175.
 
 ### Superseded Remainder
 
@@ -348,8 +350,8 @@ RFC-0026 defines the replacement five-phase runtime-first program. [D7] [U5]
   000246 and 000247 or the current code.
 - Existing transitional state names may remain in the implementation until
   RFC-0026 migrates them.
-- The measured shared-resource lifetime contention remains open in backlog
-  000175.
+- The measured shared-resource lifetime contention remained open when this RFC
+  was superseded and was subsequently resolved through backlog 000175.
 
 ## Open Questions
 
@@ -362,8 +364,9 @@ deadlock and mutation policy remains separate follow-up work under backlog
 
 - Implement the RFC-0026 mandatory runtime and migrate DDL, maintenance, and
   transaction cleanup through its phases. [D7]
-- Remove unnecessary hot-path shared-resource lifetime traffic and reassess
-  long-lived resource ownership under backlog 000175. [B3]
+- Completed after this RFC closed: remove unnecessary hot-path shared-resource
+  lifetime traffic and document long-lived resource ownership under backlog
+  000175. [B3]
 - Revisit exact-family lock-system policy independently under backlog 000171.
   [B4]
 
@@ -376,5 +379,5 @@ deadlock and mutation policy remains separate follow-up work under backlog
 - `docs/tasks/000247-statement-public-transaction-cancellation-ownership.md`
 - `docs/backlogs/closed/000170-session-coordinated-cancellation-cleanup.md`
 - `docs/backlogs/closed/000124-statement-execution-cancellation-safety.md`
-- `docs/backlogs/000175-scalable-shared-resource-lifetime-management.md`
+- `docs/backlogs/closed/000175-scalable-shared-resource-lifetime-management.md`
 - `docs/backlogs/000171-exact-family-lock-system-redesign.md`

docs/tasks/000102-separate-readonly-pool-interface-and-simplify-ownership.md

@@ -147,7 +147,7 @@ Reference:
      - `read_validated_block(&self, guard: &PoolGuard, block_id: PageID, validator: ReadonlyPageValidator) -> Result<ReadonlyBlockGuard>`
    - Keep `persisted_file_kind()`, `invalidate_block_id()`, and
      `invalidate_block_id_strict()` as readonly-specific metadata/invalidation
-     operations, and keep `pool_guard()` available for outer callers that need
+     operations, and keep `create_base_guard()` available for outer callers that need
      an explicit readonly guard source.
    - Implement the new methods on top of the existing internal
      load/dedup/validation machinery so task scope stays narrow.
@@ -164,7 +164,7 @@ Reference:
      - readonly page tests in `doradb-storage/src/file/table_file.rs`
      to use `read_validated_block()` and the returned immutable guard rather
      than `PageSharedGuard<Page>`.
-   - Thread `PoolGuards::disk_guard()` or `ReadonlyBufferPool::pool_guard()`
+   - Thread `PoolGuards::disk_guard()` or `ReadonlyBufferPool::create_base_guard()`
      from outer call sites instead of reacquiring readonly guards inside the
      read helpers.
    - Remove readonly-only `BufferPool` imports from modules that no longer need
@@ -224,7 +224,7 @@ Reference:
    - removed `impl BufferPool for ReadonlyBufferPool`;
    - added `ReadonlyBlockGuard` and readonly-specific `read_block(...)` /
      `read_validated_block(...)` entrypoints;
-   - kept `ReadonlyBufferPool::pool_guard()` public, and the final shipped API
+   - kept `ReadonlyBufferPool::create_base_guard()` public, and the final shipped API
      requires callers to pass `&PoolGuard` explicitly for readonly reads.
 2. Simplified readonly ownership and runtime structure:
    - `GlobalReadonlyBufferPool` now owns `mappings`, `inflight_loads`, and
@@ -241,7 +241,7 @@ Reference:
      explicit disk-pool guard threading.
 4. Implementation review adjusted the final public shape:
    - readonly reads keep caller-owned guard provenance instead of reacquiring
-     `global.pool_guard()` internally;
+     `global.create_base_guard()` internally;
    - raw `read_block()` is now documented as a narrow COW root/meta-page helper
      and future expansion is explicitly cautioned;
    - the resident-hit validation-failure invalidation path keeps synchronous