diff --git a/src/code_indexer/server/services/branch_service.py b/src/code_indexer/server/services/branch_service.py index 6ef8302c..20c9dcb1 100644 --- a/src/code_indexer/server/services/branch_service.py +++ b/src/code_indexer/server/services/branch_service.py @@ -8,8 +8,11 @@ from code_indexer.server.middleware.correlation import get_correlation_id import logging +import threading +import time +from collections import OrderedDict from pathlib import Path -from typing import List, Optional, Protocol +from typing import List, Optional, Protocol, Tuple from git import Repo, InvalidGitRepositoryError, GitCommandError from code_indexer.services.git_topology_service import GitTopologyService @@ -23,6 +26,18 @@ logger = logging.getLogger(__name__) +# Per-request latency: list_branches reads .commit for every local and remote +# ref (GitPython loads each commit object), which cost tens of seconds on repos +# with many refs and ran on every get_branches call. Cache the result per +# (codebase_dir, include_remote) for a short TTL so repeated calls reuse the +# walk. Branch state is eventually-consistent within the TTL. +_BRANCHES_CACHE_TTL_SECONDS = 30.0 +_BRANCHES_CACHE_MAX = 128 +_branches_cache: "OrderedDict[Tuple[str, bool], Tuple[float, List[BranchInfo]]]" = ( + OrderedDict() +) +_branches_cache_lock = threading.Lock() + class IndexStatusManager(Protocol): """Protocol for index status management to avoid tight coupling.""" @@ -61,6 +76,35 @@ def __init__( self.repo = None # type: ignore[assignment] def list_branches(self, include_remote: bool = False) -> List[BranchInfo]: + """List branches, CACHED per (codebase_dir, include_remote). + + The underlying walk (:meth:`_list_branches_uncached`) reads ``.commit`` + for every local and remote ref; on repos with many refs it cost tens of + seconds and ran on every request. Results are cached for + ``_BRANCHES_CACHE_TTL_SECONDS`` so repeated calls reuse the walk. Branch + state is eventually-consistent within the TTL. + """ + if not self._is_git_repo: + return [] + + key = (str(self.git_topology_service.codebase_dir), include_remote) + now = time.monotonic() + with _branches_cache_lock: + entry = _branches_cache.get(key) + if entry is not None and now < entry[0]: + _branches_cache.move_to_end(key) + return entry[1] + + branches = self._list_branches_uncached(include_remote=include_remote) + + with _branches_cache_lock: + _branches_cache[key] = (now + _BRANCHES_CACHE_TTL_SECONDS, branches) + _branches_cache.move_to_end(key) + while len(_branches_cache) > _BRANCHES_CACHE_MAX: + _branches_cache.popitem(last=False) + return branches + + def _list_branches_uncached(self, include_remote: bool = False) -> List[BranchInfo]: """List all branches in the repository. Args: diff --git a/src/code_indexer/server/services/file_service.py b/src/code_indexer/server/services/file_service.py index 3466b8c1..0e95fb98 100644 --- a/src/code_indexer/server/services/file_service.py +++ b/src/code_indexer/server/services/file_service.py @@ -8,6 +8,9 @@ from code_indexer.server.logging_utils import format_error_log, get_log_extra import os +import threading +import time +from collections import OrderedDict from pathlib import Path from typing import List, Optional, Tuple, Dict, Any, Set, cast from datetime import datetime, timezone @@ -27,6 +30,18 @@ logger = logging.getLogger(__name__) +# Per-request latency: _collect_files walks the FULL repo tree (rglob + a stat +# and an is_indexed check per file). On large repos this dominates list_files / +# browse latency and it ran on every request. Cache the collected listing per +# repo_path for a short TTL so repeated calls (a client paging a repo) reuse the +# walk. Keyed on repo_path: immutable versioned golden-repo snapshots change path +# on refresh and miss naturally; mutable paths are eventually-consistent within +# the TTL. +_COLLECT_CACHE_TTL_SECONDS = 30.0 +_COLLECT_CACHE_MAX_REPOS = 64 +_collect_cache: "OrderedDict[str, Tuple[float, List[FileInfo]]]" = OrderedDict() +_collect_cache_lock = threading.Lock() + # Same language detection as stats service LANGUAGE_EXTENSIONS = { ".py": "python", @@ -261,6 +276,44 @@ def _get_repository_path(self, repo_id: str, username: str) -> str: raise RuntimeError(f"Unable to access repository {repo_id}: {e}") def _collect_files(self, repo_path: str) -> List[FileInfo]: + """Collect all files in the repo, CACHED per repo_path. + + The underlying walk (:meth:`_collect_files_uncached`) is O(files) with a + stat + an is_indexed check per file; on large repos it cost tens of + seconds and list_files/browse ran it on every request. Results are cached + for ``_COLLECT_CACHE_TTL_SECONDS`` so repeated calls (a client paging a + repo) reuse the walk. Keyed on repo_path; immutable versioned golden-repo + snapshots change path on refresh and miss naturally. + """ + now = time.monotonic() + with _collect_cache_lock: + entry = _collect_cache.get(repo_path) + if entry is not None and now < entry[0]: + _collect_cache.move_to_end(repo_path) + return entry[1] + + files = self._collect_files_uncached(repo_path) + + with _collect_cache_lock: + _collect_cache[repo_path] = (now + _COLLECT_CACHE_TTL_SECONDS, files) + _collect_cache.move_to_end(repo_path) + while len(_collect_cache) > _COLLECT_CACHE_MAX_REPOS: + _collect_cache.popitem(last=False) + return files + + def _get_indexable_extensions(self) -> Set[str]: + """Indexable extensions from config, fetched ONCE per collection. + + ``_is_file_indexed`` reads ``get_config_service().get_config()`` and + ``indexing_config.indexable_extensions`` once PER FILE; hoisting it here + removes O(files) config lookups from the collection loop. + """ + config = get_config_service().get_config() + if config.indexing_config is None: + return set() + return set(config.indexing_config.indexable_extensions) + + def _collect_files_uncached(self, repo_path: str) -> List[FileInfo]: """ Collect all files in repository, excluding system directories and gitignored files. @@ -281,6 +334,10 @@ def _collect_files(self, repo_path: str) -> List[FileInfo]: # Load .gitignore patterns if present gitignore_spec = self._load_gitignore_spec(repo_root) + # Hoist the indexable-extensions lookup out of the per-file loop below + # (it was one get_config() call PER FILE via _is_file_indexed()). + indexable_extensions = self._get_indexable_extensions() + try: for file_path in repo_root.rglob("*"): if file_path.is_file(): @@ -307,7 +364,7 @@ def _collect_files(self, repo_path: str) -> List[FileInfo]: stat_info.st_mtime, tz=timezone.utc ), language=self._detect_language(file_path), - is_indexed=self._is_file_indexed(file_path), + is_indexed=file_path.suffix.lower() in indexable_extensions, ) files.append(file_info) diff --git a/tests/unit/server/services/test_branch_service_cache.py b/tests/unit/server/services/test_branch_service_cache.py new file mode 100644 index 00000000..e498807a --- /dev/null +++ b/tests/unit/server/services/test_branch_service_cache.py @@ -0,0 +1,65 @@ +"""Regression: list_branches caches per (codebase_dir, include_remote). + +The underlying walk reads .commit for every local and remote ref; on repos with +many refs it cost tens of seconds and ran on every get_branches call. A +short-TTL cache makes repeated calls reuse the walk. +""" + +from unittest.mock import MagicMock, patch + +from code_indexer.server.services import branch_service as bs_mod +from code_indexer.server.services.branch_service import BranchService + + +def _svc(codebase="/repo"): + svc = BranchService.__new__(BranchService) + svc._is_git_repo = True + svc._closed = False + svc.repo = None + gt = MagicMock() + gt.codebase_dir = codebase + svc.git_topology_service = gt + return svc + + +def test_second_call_hits_cache(): + bs_mod._branches_cache.clear() + svc = _svc() + calls = {"n": 0} + + def fake_uncached(self, include_remote=False): + calls["n"] += 1 + return [MagicMock(name="main")] + + with patch.object(BranchService, "_list_branches_uncached", fake_uncached): + r1 = svc.list_branches() + r2 = svc.list_branches() + + assert calls["n"] == 1, f"expected 1 walk (cache hit on 2nd), got {calls['n']}" + assert r1 is r2 + + +def test_include_remote_is_separate_cache_key(): + bs_mod._branches_cache.clear() + svc = _svc() + calls = {"n": 0} + + def fake_uncached(self, include_remote=False): + calls["n"] += 1 + return [] + + with patch.object(BranchService, "_list_branches_uncached", fake_uncached): + svc.list_branches(include_remote=False) + svc.list_branches(include_remote=True) # different key -> separate walk + + assert calls["n"] == 2 + + +def test_non_git_repo_returns_empty_without_walk(): + bs_mod._branches_cache.clear() + svc = _svc() + svc._is_git_repo = False + with patch.object( + BranchService, "_list_branches_uncached", side_effect=AssertionError + ): + assert svc.list_branches() == [] diff --git a/tests/unit/server/services/test_file_service_collect_cache.py b/tests/unit/server/services/test_file_service_collect_cache.py new file mode 100644 index 00000000..df6dd6a6 --- /dev/null +++ b/tests/unit/server/services/test_file_service_collect_cache.py @@ -0,0 +1,78 @@ +"""Regression: _collect_files caches the full-tree walk per repo_path. + +The walk (rglob + a stat + an is_indexed check per file) ran on every +list_files / browse request; on large repos it dominated latency. A short-TTL +per-repo_path cache makes repeated calls (a client paging a repo) reuse the +walk. The indexable-extensions lookup is also hoisted out of the per-file loop. +""" + +import tempfile +from pathlib import Path +from unittest.mock import MagicMock, patch + +from code_indexer.server.services import file_service as fs_mod +from code_indexer.server.services.file_service import FileListingService + + +def _svc(): + # Skip __init__ (ActivatedRepoManager wiring) — we exercise _collect_files only. + return FileListingService.__new__(FileListingService) + + +def _make_tree(root, n=5): + for i in range(n): + (Path(root) / f"f{i}.py").write_text("x") + (Path(root) / "note.md").write_text("x") # non-indexable extension + + +def _patch_config(): + cfg = MagicMock() + cfg.indexing_config.indexable_extensions = [".py"] + svc = MagicMock() + svc.get_config.return_value = cfg + return patch.object(fs_mod, "get_config_service", return_value=svc) + + +def test_second_call_hits_cache(): + with tempfile.TemporaryDirectory() as d: + _make_tree(d) + fs_mod._collect_cache.clear() + svc = _svc() + calls = {"n": 0} + real = FileListingService._collect_files_uncached + + def counting(self, repo_path): + calls["n"] += 1 + return real(self, repo_path) + + with ( + patch.object(FileListingService, "_collect_files_uncached", counting), + _patch_config(), + ): + r1 = svc._collect_files(d) + r2 = svc._collect_files(d) + + assert calls["n"] == 1, f"expected 1 walk (cache hit on 2nd), got {calls['n']}" + assert r1 == r2 + + +def test_is_indexed_hoist_matches_extensions(): + with tempfile.TemporaryDirectory() as d: + _make_tree(d) + fs_mod._collect_cache.clear() + with _patch_config(): + files = _svc()._collect_files(d) + by_name = {f.path: f.is_indexed for f in files} + assert by_name["f0.py"] is True + assert by_name["note.md"] is False + + +def test_distinct_repo_paths_do_not_collide(): + with tempfile.TemporaryDirectory() as d1, tempfile.TemporaryDirectory() as d2: + _make_tree(d1, n=2) + _make_tree(d2, n=4) + fs_mod._collect_cache.clear() + with _patch_config(): + r1 = _svc()._collect_files(d1) + r2 = _svc()._collect_files(d2) + assert len(r1) != len(r2) # each path cached independently