Speed up cache freshness checks on large archives#266
Merged
Conversation
Even with a fully warm cache, launching the TUI on a multi-thousand- file archive spent seconds re-verifying freshness. Profiling against a real 2GB / 5,381-item ~/.claude/projects copy showed the file stats were never the problem (~5ms for 1,830 files); the cost was per-file SQLite connections, per-file sidecar dir probes, and a dead query. Four changes, all in cache.py: - get_modified_files(): batched equivalent of per-file is_file_cached() — one query fetches every cached row for the project (one connection open instead of one per file, composing with batch() from #251), and one scandir per parent directory rules out subagent-sidecar fingerprints (#213) so the fingerprint scan only runs for files that actually have a <stem>/ dir (~154 of 1,830 in the test archive). Semantics verified identical to the per-file path on real data. - get_cached_project_data(): drop the per-file "SELECT DISTINCT session_id FROM messages" loop feeding CachedFileInfo.session_ids — the field was written but never read anywhere (production, tests, or docs), and cost N queries over the largest table on every call. Field removed from the model. - get_cached_project_data(): fetch working_directories on the same connection instead of opening a second one per call. - _init_database(): memoise the migration check per DB path per process — the TUI project selector constructs a CacheManager per row on every resize, each re-paying ~2ms of migration checks. An exists() guard keeps --clear-cache's DB deletion re-migrating. Measured on the 2GB archive (63 projects, warm cache, best of 3), before -> after on current main: TUI launch, one project (453 files): 646ms -> 12ms (54x) TUI project selector (63 projects): 1190ms -> 276ms (4x) Full-archive freshness scan: 4652ms -> 358ms (13x) Approaches considered and rejected: directory-mtime fast path (unsound — appends don't touch dir mtime; and stats were never the bottleneck), hash/probabilistic checks (same reason), parallelism (unnecessary at these timings). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRefactors the SQLite cache module to memoize migration checks per database path, centralize freshness validation, batch cached-file metadata queries for freshness scanning, remove ChangesCache freshness and migration memoization
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
claude_code_log/cache.py (1)
912-990: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFreshness logic duplicated between
is_file_cached()andget_modified_files().Both methods independently re-implement the same mtime-tolerance check and the NULL-fingerprint acceptance rule (Lines 616-629 vs. 955-989). The batched version is a legitimate optimization (scandir pre-filter, single query), but any future change to the freshness rules now needs to be applied in two places, risking drift.
Consider extracting the pure decision (
row,source_mtime,current_fp) →boolinto a small shared helper that both methods call, keeping the batching/scandir optimization only inget_modified_files().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@claude_code_log/cache.py` around lines 912 - 990, The freshness decision logic is duplicated between is_file_cached() and get_modified_files(), so extract the pure cache-validity check into a shared helper that takes the cached row, current source_mtime, and current subagents fingerprint and returns whether the file is fresh. Update both is_file_cached() and get_modified_files() to call that helper, while keeping get_modified_files()’s batching and scandir optimization intact; use the existing symbols is_file_cached, get_modified_files, and subagents_fingerprint to locate the shared rule.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@claude_code_log/cache.py`:
- Around line 912-990: The freshness decision logic is duplicated between
is_file_cached() and get_modified_files(), so extract the pure cache-validity
check into a shared helper that takes the cached row, current source_mtime, and
current subagents fingerprint and returns whether the file is fresh. Update both
is_file_cached() and get_modified_files() to call that helper, while keeping
get_modified_files()’s batching and scandir optimization intact; use the
existing symbols is_file_cached, get_modified_files, and subagents_fingerprint
to locate the shared rule.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d629b424-72d0-4446-8587-ec40ccb935e0
📒 Files selected for processing (2)
claude_code_log/cache.pydev-docs/application_model.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Speed up cache freshness checks 13–54× on large archives (#12)
Even with a fully warm cache, launching the TUI on a big archive spent seconds
re-verifying freshness. This PR attacks the check itself, benchmarked against a
real 2 GB / 5,381-item copy of
~/.claude/projects(63 projects, ~1,830session JSONLs, 1,603 sessions).
Results
Warm cache, best of 3, before → after on current
main(which alreadyincludes #251):
The
--tuilaunch path runs the freshness check twice (once in_launch_tui_with_cache_check, again inSessionBrowser.load_sessions), sothe user-visible launch delay was ~1.3 s and is now ~25 ms.
What was actually slow
The file stats were never the problem — stat'ing all 1,830 files takes ~5 ms,
and even a full recursive walk of the 2 GB tree is ~150 ms. The measured costs:
get_modified_files()calledis_file_cached()per file, each opening a fresh connection (~40× the costof the query it runs). Speed up the SQLite metadata-cache build (connection reuse + synchronous=NORMAL) #251 fixed this pattern for cache builds via
batch(), but the freshness-check paths don't run inside a batch.subagents_fingerprint()(Support hierarchies of agents #213) does anis_dir()probe + glob per file, ~160 ms across the archive — almost allfor files that can't have sidecars.
get_cached_project_data()ranSELECT DISTINCT session_id FROM messages WHERE file_id = ?per cachedfile to fill
CachedFileInfo.session_ids— a field nothing reads(production code, tests, or docs). This is what made the project selector
slow.
CacheManagerconstruction re-ran themigration check (~2 ms); the TUI selector constructs one per project row,
on every resize.
Changes (all in
cache.py)get_modified_files()— one query fetches every cached row forthe project (one connection open; zero extra inside a
batch()scope), andone
os.scandirper parent directory determines which files even canhave sidecars, so the fingerprint scan only runs for those (~154 of 1,830
here). Semantics are identical to the per-file path — verified below.
CachedFileInfo.session_idsand its N-queries-per-call.(Measured alternatives: single
GROUP BYquery = 209 ms, per-file lookupson a shared connection = 131 ms, not doing it = 4 ms.)
get_cached_project_data()fetchesworking_directorieson the sameconnection instead of opening a second one per call.
exists()guard so--clear-cache's DB deletion still re-migrates.Connection lifecycle is untouched: connection-per-call stays the default
(the Windows-safe behaviour pinned by #251's integrity tests), and the new
code composes with
batch().Approaches considered and rejected
check — appending to a JSONL doesn't update the directory mtime, so it can
only detect create/delete/rename. And since per-file stats cost ~5 ms
total, it would save nothing meaningful.
were never the bottleneck; probabilistic is also non-deterministic.
could still help the one-time initial parse (~400 s for the full 2 GB),
but that's a rare event and separate from this issue.
Verification
new sidecar detected (including for a session with no pre-existing
<stem>/dir — the case the scandir shortcut could plausibly havebroken), new file detected, and full parity with per-file
is_file_cached()both outside and inside abatch()scope.lifecycle integrity tests);
ruff checkandty checkclean.Numbers above are from a Linux dev container; absolute values will differ on
macOS but the ratios should hold, since the fix removes work rather than
speeding it up.
Summary by CodeRabbit