perf(cache): hot-path wins in CacheAndBufferLayer (2/3 of #991)

[SamTV12345]

perf(cache): hot-path wins in CacheAndBufferLayer

Part 2 of 3 from splitting #991. Scope: lib/CacheAndBufferLayer.ts only — no public API change, no default change, no behavioral regression (including toJSON semantics). This layer sits on the hot path of every operation regardless of backend.

Four compounding wins plus one safety fix:

1. structuredClone on the read path

clone() was a recursive deep-copy run on every read and write. Split into:

• cloneIn (the existing toJSON-aware walker) for the write direction — keeps documented toJSON behavior.
• cloneOut using the V8-native structuredClone for the read direction, where the cached value is already JSON-safe.

2. Dirty-key Set

flush() scanned the entire LRU buffer (default cap 10 000) every interval just to find dirty entries. Now a Set<string> of dirty keys is maintained, so flush() iterates only what's actually dirty. Invariant + markDone ordering documented in code.

3. Lazy flush scheduling

Replaces the always-on setInterval with an on-demand, unref'd setTimeout armed when the first entry goes dirty and cleared after flushing. Idle databases do zero timer/scan work.

4. Lock-free fast path in get()

On a guaranteed cache hit with no in-flight writer, get() now returns the cloned value without acquiring/releasing the per-key lock (saving a promise allocation + two Map mutations per call). The fast path is fully synchronous; correctness argument is in the design doc and a code comment marks the no-await region.

Safety fix

cloneOut falls back to cloneIn for values structuredClone can't handle (e.g. a stored {toJSON: fn}), so the read path never throws DataCloneError.

Tests

New, under test/mock/: test_dirty_set, test_lazy_flush, test_lock_fast_path; plus test_metrics updates and a test_tojson case for the fallback. All existing cache/metrics/LRU/bulk tests stay green.

Verification

• tsc --noEmit passes.
• vitest run test/mock test/memory → 185 passing.

Independence

Touches only CacheAndBufferLayer.ts + tests + design docs — no overlap with the drivers PR, and the only overlap with the formatting PR is one test file (test_tojson.spec.ts). Can be reviewed in parallel.

Includes the design spec + implementation plan under docs/superpowers/. Happy to drop those from the public history if you'd rather not carry them.

🤖 Generated with Claude Code

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[qodo-free-for-open-source-projects]

Review Summary by Qodo

Performance optimization of CacheAndBufferLayer with four compounding wins

✨ Enhancement 🧪 Tests

Walkthroughs

Description

  Performance optimization of CacheAndBufferLayer.ts with four compounding wins and comprehensive
  test coverage:
• **Split clone() into cloneIn and cloneOut**: cloneIn preserves toJSON semantics for
  writes; cloneOut uses V8-native structuredClone for reads with fallback to cloneIn for
  non-serializable values
• **Dirty-key Set tracking**: Replaced full LRU buffer scan in flush() with a Set of dirty keys,
  reducing iteration overhead from O(buffer size) to O(dirty entries)
• **Lazy flush scheduling**: Replaced always-on setInterval with on-demand setTimeout armed only
  when entries go dirty; idle databases perform zero timer/scan work
• **Lock-free fast path in get()**: Cache hits with no concurrent writer skip per-key lock
  acquisition/release, eliminating promise allocation and Map mutations
• **Safety fallback**: cloneOut gracefully handles non-serializable values to prevent
  DataCloneError on read path
• **Comprehensive test coverage**: New test files for dirty-key invariant, lazy flush scheduling,
  lock-free fast path, and toJSON fallback behavior
• **Design documentation**: Includes implementation plan and design specification with correctness
  arguments and edge case analysis

Diagram

flowchart LR
  A["Read Path"] -->|"uses structuredClone"| B["cloneOut"]
  B -->|"fallback for non-serializable"| C["cloneIn"]
  D["Write Path"] -->|"preserves toJSON"| C
  E["set() call"] -->|"adds to"| F["_dirtyKeys Set"]
  F -->|"replaces full scan"| G["flush()"]
  H["First dirty entry"] -->|"arms"| I["_flushTimer"]
  I -->|"cleared after"| G
  J["get() cache hit"] -->|"no concurrent writer"| K["Lock-free fast path"]
  K -->|"skips lock acquire/release"| L["Synchronous return"]

Loading

File Changes

1. lib/CacheAndBufferLayer.ts ✨ Enhancement +137/-69

Four compounding performance wins in CacheAndBufferLayer

• Split clone() into cloneIn (write path with toJSON support) and cloneOut (read path using
 V8-native structuredClone)
• Added _dirtyKeys: Set to track dirty entries, replacing full LRU scan in flush()
• Replaced constructor's setInterval with on-demand _flushTimer (setTimeout) armed by
 _scheduleFlush() when entries go dirty
• Implemented lock-free fast path in get() for cache hits with no concurrent writer, skipping
 per-key lock acquisition

lib/CacheAndBufferLayer.ts

---

2. test/mock/test_metrics.spec.ts 🧪 Tests +181/-155

Metrics test adjustments for get() fast-path behavior

• Adjusted expected metrics for get() cache-hit cases to exclude lockAcquires and lockReleases
 (no longer incremented by fast path)
• Updated "read of in-progress write" test to conditionally expect lock counters only for
 getSub(), not get()
• Formatting changes (quote style, spacing) applied throughout

test/mock/test_metrics.spec.ts

---

3. test/mock/test_dirty_set.spec.ts 🧪 Tests +98/-0

New tests for dirty-key Set tracking

• New test file verifying _dirtyKeys Set invariant: entries added on set(), drained on flush()
• Tests re-set during in-flight write (key remains dirty when entry is replaced)
• Tests failed write path (key removed from _dirtyKeys on error)

test/mock/test_dirty_set.spec.ts

---

View more (5)

4. test/mock/test_lazy_flush.spec.ts 🧪 Tests +77/-0

New tests for on-demand flush timer

• New test file verifying lazy flush scheduling: idle database has no timer, set() arms it,
 flush() clears it
• Tests that close() cancels pending timer without leaking handles
• Tests that writeInterval=0 mode never arms the timer

test/mock/test_lazy_flush.spec.ts

---

5. test/mock/test_lock_fast_path.spec.ts 🧪 Tests +120/-0

New tests for get() lock-free fast path

• New test file verifying lock-free fast path: cache-hit get() does not acquire per-key lock
• Tests cache-miss get() still acquires lock (slow path)
• Tests get() during in-flight write returns buffered value via fast path
• Tests get() contention forces slow path when writer holds lock

test/mock/test_lock_fast_path.spec.ts

---

6. test/memory/test_tojson.spec.ts 🧪 Tests +11/-0

New test for function-containing object cache round-trip

• Added new test case verifying that object properties containing functions survive round-trip via
 cache
• Tests that cloneOut falls back to cloneIn when structuredClone would throw DataCloneError

test/memory/test_tojson.spec.ts

---

7. docs/superpowers/plans/2026-05-28-cache-buffer-perf.md 📝 Documentation +1113/-0

Implementation plan for cache-buffer performance wins

• Comprehensive implementation plan with 5 tasks (baseline, clone split, dirty-key set, lazy flush,
 lock fast-path)
• Each task broken into numbered steps with exact code snippets, test expectations, and commit
 messages
• Verification section with speed benchmark capture and PR readiness checklist

docs/superpowers/plans/2026-05-28-cache-buffer-perf.md

---

8. docs/superpowers/specs/2026-05-28-cache-buffer-perf-design.md 📝 Documentation +253/-0

Cache buffer layer performance optimization design specification

• Added comprehensive design specification for four performance optimizations to
 CacheAndBufferLayer.ts
• Documents split of clone() into cloneIn (write path) and cloneOut (read path using
 structuredClone)
• Specifies dirty-key Set maintenance to replace full-buffer iteration in flush()
• Details lazy flush scheduling with on-demand setTimeout replacing always-on setInterval
• Describes lock-free fast path in get() for cache hits with no concurrent writers
• Includes correctness arguments, edge case analysis, testing strategy, and risk assessment

docs/superpowers/specs/2026-05-28-cache-buffer-perf-design.md

---

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0)

1. Timer armed during flush 🐞 Bug ➹ Performance

Description

_setLocked() can arm _flushTimer via _scheduleFlush() while flush() is already running and will
drain the dirty keys, leaving a redundant timer that later fires on an idle DB. This defeats part of
the lazy-flush optimization and can introduce extra wakeups/active handles after a concurrent set
during an in-flight flush.

Code

lib/CacheAndBufferLayer.ts[R243-252]

Evidence

The code shows _scheduleFlush() can arm a timer without checking whether a flush is already
running; _setLocked() calls it for every dirty write; and flush() yields while awaiting
_write(), allowing a concurrent set() to arm a redundant timer that flush() does not clear before
returning.

lib/CacheAndBufferLayer.ts[243-252]
lib/CacheAndBufferLayer.ts[449-465]
lib/CacheAndBufferLayer.ts[561-583]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`_setLocked()` always calls `_scheduleFlush()`, which can arm a new `_flushTimer` even if `flush()` is already in progress (`_flushDone != null`). Because `flush()` awaits `_write()` (yields to the event loop), a concurrent `set()` can run mid-flush and schedule a timer that becomes unnecessary if the in-progress flush drains all dirty keys. This results in a redundant timer firing later on an otherwise idle DB.

### Issue Context
- `_scheduleFlush()` currently checks only `_flushTimer`, `writeInterval`, and `_dirtyKeys.size`.
- `_setLocked()` calls `_scheduleFlush()` unconditionally after `_dirtyKeys.add(key)`.
- `flush()` cancels a pending timer only at entry; it does not prevent a timer from being armed mid-flush.

### Fix Focus Areas
- lib/CacheAndBufferLayer.ts[243-252]
- lib/CacheAndBufferLayer.ts[449-465]
- lib/CacheAndBufferLayer.ts[561-583]

### Suggested implementation
Option A (preferred): In `_scheduleFlush()`, add `if (this._flushDone != null) return;` so no timer is armed while a flush is running. The existing `flush()` postlude already re-arms if dirty keys remain.

Option B: At the end of `flush()`, if `_dirtyKeys.size === 0`, call `_cancelFlushTimer()` to clear any timer armed mid-flush.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---