Skip to content

perf: cache list_files/branch walks that ran on every request#1331

Open
unsupo wants to merge 1 commit into
developmentfrom
feat/list-and-branch-perf-caches
Open

perf: cache list_files/branch walks that ran on every request#1331
unsupo wants to merge 1 commit into
developmentfrom
feat/list-and-branch-perf-caches

Conversation

@unsupo

@unsupo unsupo commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

What

Two read-only server hot paths re-ran expensive filesystem/git work on every request, dominating latency on large repos:

  • FileListingService._collect_files walked the full repo tree (rglob + a stat and an is_indexed check per file) on every list_files/browse call. Split into a short-TTL per-repo_path cache + _collect_files_uncached, and hoisted the indexable-extensions lookup out of the per-file loop (it was one get_config() call per file via _is_file_indexed()).
  • BranchService.list_branches read .commit for every local and remote ref (GitPython loads each commit object) on every get_branches call. Split into a short-TTL cache keyed on (codebase_dir, include_remote) + _list_branches_uncached.

Why

On a large monolith these listings ran in the tens of seconds and were recomputed per request, so N clients/agents hitting the same repo each paid the full walk. Read-only listings where a short staleness window is acceptable.

Behavior / safety

  • Both caches are eventually-consistent within a 30s TTL and bounded (LRU: 64 repos / 128 branch-keys).
  • Keyed so immutable versioned golden-repo snapshots miss naturally on refresh (path changes).
  • No API change; the cold call is unchanged (still the full walk) — this optimizes repeated access.

Tests

Regression tests for both caches (cache hit avoids re-walk, key isolation, non-git repo, is_indexed hoist correctness). Existing file/branch suites pass unchanged. ruff/ruff format/mypy clean.


🤖 Generated with Claude Code

Two read-only server hot paths re-did expensive filesystem/git work on every
call, dominating latency on large repos:

- FileListingService._collect_files walked the FULL repo tree (rglob + a stat
  and an is_indexed check per file) on every list_files/browse request. Split
  into a short-TTL per-repo_path cache wrapper + _collect_files_uncached, and
  hoisted the indexable-extensions lookup out of the per-file loop (it was one
  get_config() call PER FILE via _is_file_indexed()).

- BranchService.list_branches read .commit for every local and remote ref
  (GitPython loads each commit object) on every get_branches call. Split into a
  short-TTL cache keyed on (codebase_dir, include_remote) + _list_branches_uncached.

Both caches are eventually-consistent within a 30s TTL and bounded (LRU). Keyed
so immutable versioned golden-repo snapshots miss naturally on refresh. Repeated
calls (a client paging a repo, or many agents hitting the same repo) reuse the
walk instead of re-running it.

Adds regression tests for both caches; existing file/branch suites unchanged.
@unsupo unsupo requested a review from jsbattig July 8, 2026 23:25
@jsbattig

jsbattig commented Jul 9, 2026

Copy link
Copy Markdown

Reviewed via /implement-backlog and integrated into development (merge commit b782bf77, preserving your original commit + a review-fix commit 4ff8075f). Thanks for the contribution!

Code review confirmed the cluster-invariant is satisfied (read-only listings, bounded 30s staleness, no auth-bearing data — the same carve-out as query_path_cache) and immutable-snapshot keying is correct. Three findings were addressed before merge:

  1. TTL anchored pre-walk (HIGH): expiry was stamped from a timestamp taken before the walk, so a repo whose walk exceeds the TTL stored an already-expired entry — zero benefit for exactly the slow repos this targets. Fixed: expiry now computed post-walk in both _collect_files and list_branches.
  2. Branch cache not invalidated on mutation (MEDIUM): added BranchService.invalidate(codebase_dir) wired into git_branch_create/switch/delete so a branch mutation is immediately visible on get_branches (was up to 30s stale).
  3. Tests (MEDIUM): added slow-walk-still-cached (guards finding 1), TTL-expiry, LRU-eviction, invalidate-scoped, and distinct-key isolation.

Gates: fast-automation 12389/0; server-fast clean (2 failures were pre-existing flakes unrelated to this PR — test_coalescer_joiner_live_batch_id_1293_a3 and test_sqlite_log_handler_batched_1241, both confirmed by isolation re-runs); ./lint.sh 0.

Follow-up note (LOW, not blocking): for -global golden repos the branch-cache key differs between the read path (mutable base) and the write handlers (versioned snapshot), so a global-repo branch mutation's invalidate could miss (bounded, <=30s stale read-only listing; switch is already blocked for -global). Worth a small follow-up.

This will show as merged once development is pushed.

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.

2 participants