From 79832c8736abf22ae8cead82e3b203bcfaecdaab Mon Sep 17 00:00:00 2001 From: Stephan Garland Date: Mon, 17 Aug 2026 20:18:25 -0400 Subject: [PATCH] perf(detect): index ignore patterns by anchor; drop the nested lru_cache (#1958) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit detect() accumulates ignore patterns into one flat [(anchor_dir, pattern)] list and _is_ignored() rescanned all of it per path. Two costs fell out of that on a 356k-file corpus with 1,697 .gitignore files (57,286 patterns): Anchor selection. The scan tested every anchor with target.relative_to(anchor). On CPython 3.12+ that scans target.parents linearly (_PathParents is a Sequence with no __contains__ override), minting a Path per element whose __eq__ raises and catches an AttributeError off an unset __slots__ member, then throws ValueError away for the ~97% of anchors that are not ancestors. Measured 813ms per path. _AnchorIndex buckets the list by anchor so a lookup walks the target's own ancestor chain instead — the same candidate set relative_to accepted, in list order so last-match-wins and ! negations are unaffected. The list is append-only during a walk, so the index refreshes over the new tail only. Cyclic garbage. _match_anchored_ignore_pattern wrapped a recursive closure in its own @lru_cache, so every call built a reference cycle only the GC could reclaim (14 cyclic objects per call), and the cache was discarded before it could serve a second call. It is now a module-level function taking an explicit dict memo, with patterns lacking ** skipping the recursion entirely (54 of 57,283 patterns contain **). Behavior is unchanged: differentially fuzzed against the previous implementations — 200k random plus exhaustive small-space pattern/path pairs for the matcher, and 3,000 random pattern sets x 40 files for _is_ignored, covering sibling anchors, negations and directory-only rules — 0 mismatches. On a synthetic 1,500-anchor / 30k-pattern corpus, 807ms -> 0.089ms per path, and 500 globstar matches now leave 0 cyclic objects. Tests: 18 new in test_detect.py. Suite: 4,621 passed, 48 skipped, no regressions (test_labeling.py::test_label_communities_batches_when_over_batch_size and the four in test_ollama_retry_cap.py fail identically before this change). Co-Authored-By: Claude Opus 5 (1M context) --- graphify/detect.py | 199 ++++++++++++++++++++++++++++++++++++------- tests/test_detect.py | 148 ++++++++++++++++++++++++++++++++ 2 files changed, 317 insertions(+), 30 deletions(-) diff --git a/graphify/detect.py b/graphify/detect.py index 4cb123104c..327b5699e4 100644 --- a/graphify/detect.py +++ b/graphify/detect.py @@ -11,7 +11,6 @@ import unicodedata from concurrent.futures import ThreadPoolExecutor from enum import Enum -from functools import lru_cache from pathlib import Path from typing import Callable @@ -1249,32 +1248,165 @@ def _load_graphifyignore(root: Path, *, gitignore: bool = True) -> list[tuple[Pa return patterns -def _match_anchored_ignore_pattern(path: str, pattern: str) -> bool: - """Match an anchored gitignore pattern without letting ``*`` cross ``/``.""" - path_parts = tuple(path.split("/")) - pattern_parts = tuple(pattern.split("/")) +def _match_globstar_parts( + path_parts: tuple[str, ...], + pattern_parts: tuple[str, ...], + path_idx: int, + pattern_idx: int, + memo: dict[tuple[int, int], bool], +) -> bool: + """Component-wise match where ``**`` spans zero or more path components. - @lru_cache(maxsize=None) - def _matches(path_idx: int, pattern_idx: int) -> bool: - if pattern_idx == len(pattern_parts): - return path_idx == len(path_parts) + Module-level with a passed-in memo, not a nested ``lru_cache``: a recursive + closure wrapped in its own cache references itself through that cache, so + every call leaked a reference cycle for the GC to reclaim — and the cache + died before it could serve a second call. This memo dies by refcount. + A trailing ``**`` still requires at least one remaining path component. + """ + key = (path_idx, pattern_idx) + hit = memo.get(key) + if hit is not None: + return hit + + n_path = len(path_parts) + n_pattern = len(pattern_parts) + if pattern_idx == n_pattern: + result = path_idx == n_path + else: part = pattern_parts[pattern_idx] if part == "**": - if pattern_idx == len(pattern_parts) - 1: - return path_idx < len(path_parts) - return _matches(path_idx, pattern_idx + 1) or ( - path_idx < len(path_parts) - and _matches(path_idx + 1, pattern_idx) + if pattern_idx == n_pattern - 1: + result = path_idx < n_path + else: + result = _match_globstar_parts( + path_parts, pattern_parts, path_idx, pattern_idx + 1, memo + ) or ( + path_idx < n_path + and _match_globstar_parts( + path_parts, pattern_parts, path_idx + 1, pattern_idx, memo + ) + ) + else: + result = ( + path_idx < n_path + and fnmatch.fnmatchcase(path_parts[path_idx], part) + and _match_globstar_parts( + path_parts, pattern_parts, path_idx + 1, pattern_idx + 1, memo + ) ) - return ( - path_idx < len(path_parts) - and fnmatch.fnmatchcase(path_parts[path_idx], part) - and _matches(path_idx + 1, pattern_idx + 1) + memo[key] = result + return result + + +def _match_anchored_ignore_pattern(path: str, pattern: str) -> bool: + """Match an anchored gitignore pattern without letting ``*`` cross ``/``.""" + path_parts = tuple(path.split("/")) + pattern_parts = tuple(pattern.split("/")) + + if "**" not in pattern_parts: + # Without ``**`` each pattern component eats exactly one path component, + # so this is a straight zip and the recursion below would never reuse a + # memo entry. Also the common shape: 54 of 57,283 patterns in a large + # monorepo contain ``**``. + if len(path_parts) != len(pattern_parts): + return False + return all( + fnmatch.fnmatchcase(part, pat) + for part, pat in zip(path_parts, pattern_parts) ) - return _matches(0, 0) + return _match_globstar_parts(path_parts, pattern_parts, 0, 0, {}) + + +class _AnchorIndex: + """Anchor-bucketed view over a flat ``[(anchor_dir, pattern), ...]`` list. + + ``_is_ignored`` only needs patterns whose anchor is an ancestor of (or equal + to) the target; the rest are inert, per the invariant ``ignored_predicate`` + documents. Bucketing by anchor turns the per-path cost from O(all patterns) + into O(path depth) dict lookups. + + The flat scan it replaces tested each anchor with + ``target.relative_to(anchor)``, which on CPython 3.12+ scans + ``target.parents`` linearly, minting a ``Path`` per element, then raises + ``ValueError`` for the ~97% of anchors that are not ancestors. On a monorepo + with 1,697 ``.gitignore`` files (57k patterns) that was 813ms per path. + + ``detect()`` extends the list as its walk finds nested ignore files, so + ``_refresh`` indexes only the new tail. + """ + + __slots__ = ("_patterns", "_by_anchor", "_indexed") + + def __init__(self, patterns: list[tuple[Path, str]]) -> None: + self._patterns = patterns + self._by_anchor: dict[Path, list[int]] = {} + self._indexed = 0 + self._refresh() + + def _refresh(self) -> None: + n = len(self._patterns) + if n == self._indexed: + return + if n < self._indexed: + # Shrunk: the list was truncated or reused for a different scan. + self._by_anchor.clear() + self._indexed = 0 + by_anchor = self._by_anchor + for i in range(self._indexed, n): + by_anchor.setdefault(self._patterns[i][0], []).append(i) + self._indexed = n + + def candidates(self, target: Path) -> list[int]: + """Indices of the patterns that can match *target*, in original order. + + Order matters: ``_eval`` is last-match-wins, so a later ``!`` negation + (or a CLI ``--exclude``) must still override an earlier rule. The + candidate set is exactly ``{target} | set(target.parents)`` — the anchors + ``relative_to`` accepted — so this matches the scan it replaces. + """ + self._refresh() + by_anchor = self._by_anchor + if not by_anchor: + return [] + buckets: list[list[int]] = [] + anchor = target + while True: + bucket = by_anchor.get(anchor) + if bucket is not None: + buckets.append(bucket) + parent = anchor.parent + if parent == anchor: # reached the filesystem/drive root + break + anchor = parent + if not buckets: + return [] + if len(buckets) == 1: + return buckets[0] # already ascending + return sorted(i for bucket in buckets for i in bucket) + + +# Identity-keyed memo so every _is_ignored caller gets the index without a +# signature change. The entry holds the list, so its id() cannot be recycled +# while live. Cap covers the lists one path can interleave: _is_scan_ignored +# alternates two per scan, and a `watch` rebuild has its own plus two +# predicates' pairs — evicting below that rebuilds per call, not per scan. +_ANCHOR_INDEX_MEMO_MAX = 8 +_ANCHOR_INDEX_MEMO: dict[int, tuple[list[tuple[Path, str]], _AnchorIndex]] = {} + + +def _anchor_index(patterns: list[tuple[Path, str]]) -> _AnchorIndex: + key = id(patterns) + hit = _ANCHOR_INDEX_MEMO.get(key) + if hit is not None and hit[0] is patterns: + return hit[1] + if len(_ANCHOR_INDEX_MEMO) >= _ANCHOR_INDEX_MEMO_MAX: + _ANCHOR_INDEX_MEMO.clear() + index = _AnchorIndex(patterns) + _ANCHOR_INDEX_MEMO[key] = (patterns, index) + return index def _is_ignored( @@ -1300,6 +1432,8 @@ def _is_ignored( if not patterns: return False + index = _anchor_index(patterns) + def _eval(target: Path) -> bool: """Apply last-match-wins to a single target path.""" if _cache is not None and target in _cache: @@ -1320,7 +1454,13 @@ def _matches(rel: str, p: str, path_relative: bool) -> bool: return False result = False - for anchor, pattern in patterns: + # gitignore semantics: patterns from A/.gitignore apply ONLY to paths + # under A, so only anchors on the target's ancestor chain can match. The + # index yields those in list order, replacing a full-list scan that + # rediscovered the same fact one raised ValueError at a time. + target_parts = target.parts + n_target = len(target_parts) + for anchor, pattern in (patterns[i] for i in index.candidates(target)): negated = pattern.startswith("!") raw = pattern[1:] if negated else pattern directory_only = raw.endswith("/") @@ -1329,17 +1469,16 @@ def _matches(rel: str, p: str, path_relative: bool) -> bool: if not p: continue - # gitignore semantics: patterns from A/.gitignore apply ONLY to paths - # under A. Matching non-anchored patterns against root-relative paths - # let e.g. .hypothesis/.gitignore's bare "*" ignore the ENTIRE repo - # (detect() returned 0 files). The anchor dir itself is exempt — an - # ignore file governs its directory's contents, not the directory. + # Matching non-anchored patterns against root-relative paths let e.g. + # .hypothesis/.gitignore's bare "*" ignore the ENTIRE repo (detect() + # returned 0 files). The anchor dir itself is exempt — an ignore file + # governs its contents, not itself. anchor is a known ancestor here, + # so the relative path is a part-slice and equal depth means + # anchor == target, i.e. the old relative_to's ".". matched = False - try: - rel_anchor = _nfc(str(target.relative_to(anchor)).replace(os.sep, "/")) - except ValueError: - continue # target outside this pattern's anchor: cannot match - if rel_anchor != ".": + n_anchor = len(anchor.parts) + if n_anchor < n_target: + rel_anchor = _nfc("/".join(target_parts[n_anchor:])) rel = rel_anchor if not path_relative: try: diff --git a/tests/test_detect.py b/tests/test_detect.py index 5be697ce72..6cacd2eda8 100644 --- a/tests/test_detect.py +++ b/tests/test_detect.py @@ -3066,3 +3066,151 @@ def test_sensitive_env_template_inside_secrets_dir_still_dropped(path): """Stage 1 dir guard runs before the Stage 2 template exemption: anything under a secrets/credentials dir stays excluded, template suffix or not.""" assert _is_sensitive(Path(path)), f"{path} is under a secrets dir, must stay excluded (#2184)" + + +# --- #1958: ignore-matcher cost (anchor index + memo-free globstar matcher) --- + +from graphify.detect import ( + _AnchorIndex, + _anchor_index, + _match_anchored_ignore_pattern, + _ANCHOR_INDEX_MEMO, +) + + +def _flat_candidates(patterns, target): + """What the pre-#1958 flat scan kept: the anchors relative_to() accepted.""" + out = [] + for i, (anchor, _pattern) in enumerate(patterns): + try: + target.relative_to(anchor) + except ValueError: + continue + out.append(i) + return out + + +def test_anchor_index_selects_exactly_the_relative_to_survivors(tmp_path): + """The index must select the same set the relative_to() scan did.""" + a = tmp_path / "a" / "b" + sibling = tmp_path / "other" + a.mkdir(parents=True) + sibling.mkdir() + patterns = [ + (tmp_path, "*.log"), + (sibling, "*.py"), # sibling subtree: inert for targets under a/b + (tmp_path / "a", "gen/"), + (a, "*.tmp"), + ] + target = a / "x.py" + index = _anchor_index(patterns) + assert index.candidates(target) == _flat_candidates(patterns, target) + + +def test_anchor_index_preserves_original_order_for_last_match_wins(tmp_path): + """_eval is last-match-wins, so a later ! must still override an earlier + rule even when the two live in different anchor buckets.""" + sub = tmp_path / "sub" + sub.mkdir() + patterns = [ + (sub, "keep.py"), # nested rule ... + (tmp_path, "!keep.py"), # ... overridden by a later root-anchored one + ] + target = sub / "keep.py" + assert _anchor_index(patterns).candidates(target) == [0, 1] + assert _is_ignored(target, tmp_path, patterns) is False + + patterns.reverse() # flip the order: the nested rule now wins + _ANCHOR_INDEX_MEMO.clear() + assert _is_ignored(target, tmp_path, patterns) is True + + +def test_anchor_index_indexes_appended_patterns(tmp_path): + """detect() extends the list mid-walk; the index must see the new tail.""" + sub = tmp_path / "sub" + sub.mkdir() + target = sub / "x.py" + target.write_text("x") + patterns = [(tmp_path, "*.log")] + assert _is_ignored(target, tmp_path, patterns) is False + patterns.append((sub, "*.py")) # a nested .gitignore discovered mid-walk + assert _is_ignored(target, tmp_path, patterns) is True + + +def test_anchor_index_rebuilds_when_the_pattern_list_shrinks(tmp_path): + sub = tmp_path / "sub" + sub.mkdir() + target = sub / "x.py" + target.write_text("x") + patterns = [(tmp_path, "*.log"), (sub, "*.py")] + index = _AnchorIndex(patterns) + assert index.candidates(target) == [0, 1] + del patterns[1] + assert index.candidates(target) == [0] + + +def test_anchor_index_memo_survives_a_reused_list_identity(tmp_path): + """The memo is id()-keyed, so it must hold the list: a freed one's address + could be recycled and hand a later scan somebody else's index.""" + patterns = [(tmp_path, "*.py")] + first = _anchor_index(patterns) + assert _anchor_index(patterns) is first + other = [(tmp_path, "*.log")] + assert _anchor_index(other) is not first + + +@pytest.mark.parametrize("path,pattern,expected", [ + ("a/b/c.py", "a/b/c.py", True), + ("a/b/c.py", "a/*/c.py", True), + ("a/b/c.py", "a/*.py", False), # * must not cross a / + ("a/b/c.py", "a/**/c.py", True), + ("a/b/c/d.py", "a/**/d.py", True), + ("a/c.py", "a/**/c.py", True), # ** spans zero components + ("a/b/c.py", "a/b", False), # pattern shorter than the path + ("a", "a/**", False), # trailing ** needs a component to eat + ("a/b", "a/**", True), + ("a/b/c/d/e.py", "**/c/**/*.py", True), + ("a/b/x.txt", "**/*.py", False), +]) +def test_match_anchored_ignore_pattern_semantics(path, pattern, expected): + """Semantics must survive the move off the nested lru_cache.""" + assert _match_anchored_ignore_pattern(path, pattern) is expected + + +def test_globstar_match_leaves_no_cyclic_garbage(): + """The old nested lru_cache closure referenced itself through its own + cache, leaking a cycle per call for the GC to reclaim.""" + import gc + + gc.collect() + gc.set_debug(gc.DEBUG_SAVEALL) + try: + before = len(gc.garbage) + for _ in range(500): + _match_anchored_ignore_pattern("a/b/c/d/e.py", "**/c/**/*.py") + gc.collect() + created = len(gc.garbage) - before + finally: + gc.set_debug(0) + del gc.garbage[:] + gc.collect() + assert created == 0, f"{created} cyclic objects from 500 globstar matches" + + +def test_sibling_anchored_patterns_do_not_cost_per_path_work(tmp_path): + """Sibling-anchored patterns are inert, so a path's candidate set stays + O(its own depth), not O(all patterns).""" + target_dir = tmp_path / "pkg0" / "src" + target_dir.mkdir(parents=True) + target = target_dir / "main.py" + target.write_text("x") + patterns = [(tmp_path, "*.log")] + for i in range(1, 200): + sibling = tmp_path / f"pkg{i}" + patterns.extend((sibling, f"*.gen{j}") for j in range(10)) + patterns.append((target_dir, "*.tmp")) + + candidates = _anchor_index(patterns).candidates(target) + assert candidates == _flat_candidates(patterns, target) + assert len(candidates) == 2 # only the root and the file's own dir + assert _is_ignored(target, tmp_path, patterns) is False