From 258ab11db273bae55714dec48554b5b3b0d1aba8 Mon Sep 17 00:00:00 2001 From: Azeem1985 <90791197+Azeem1985@users.noreply.github.com> Date: Tue, 25 Aug 2026 15:46:44 +0100 Subject: [PATCH] perf(detect): stop building pathlib objects per pattern per file in ignore evaluation (#2226) Ignore evaluation built a Path and called relative_to (with its CPython quadratic parents cost) per pattern per file, and re-parsed each pattern's flags every call. Replace it with a lexical string-space relative computation (no Path construction, no exception control flow), parse each pattern once into a process-wide cache, and memoize per-anchor relatives, segment splits and is_dir per evaluation. Ignore decisions are unchanged (differential-fuzz verified); ~50x faster on a pattern-heavy monorepo. --- graphify/detect.py | 164 +++++++++++++++++++++++++++++++------ tests/test_detect.py | 188 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 327 insertions(+), 25 deletions(-) diff --git a/graphify/detect.py b/graphify/detect.py index d16b5800c..648612d07 100644 --- a/graphify/detect.py +++ b/graphify/detect.py @@ -1249,6 +1249,73 @@ def _load_graphifyignore(root: Path, *, gitignore: bool = True) -> list[tuple[Pa return patterns +# Parsed-pattern cache: raw pattern string -> (negated, directory_only, +# path_relative, stripped_pattern). Ignore patterns are re-evaluated for every +# walked entry; parsing the same strings per entry per scan was pure waste. +# Plain dict (no LRU): the universe of keys is the distinct pattern lines in +# the corpus's ignore files, which is small and bounded per process. +_PARSED_PATTERN_CACHE: dict[str, tuple[bool, bool, bool, str]] = {} + + +def _parse_ignore_pattern(pattern: str) -> tuple[bool, bool, bool, str]: + """Split one gitignore-style pattern into its matching flags, cached. + + Returns (negated, directory_only, path_relative, stripped). ``stripped`` + is empty for patterns that match nothing (bare "!", bare "/"). + """ + got = _PARSED_PATTERN_CACHE.get(pattern) + if got is None: + negated = pattern.startswith("!") + raw = pattern[1:] if negated else pattern + directory_only = raw.endswith("/") + path_relative = "/" in raw.rstrip("/") + got = (negated, directory_only, path_relative, raw.strip("/")) + _PARSED_PATTERN_CACHE[pattern] = got + return got + + +# PurePath.relative_to compares casefolded parts on Windows only; mirror that +# exactly, but skip the normcase pass entirely where it is a no-op (POSIX). +_CASEFOLD_PATHS = os.path.normcase("Aa") != "Aa" + + +def _lexical_relative( + target: Path, target_parts: tuple[str, ...], anchor: Path +) -> str | None: + """String-space equivalent of ``_nfc(str(target.relative_to(anchor)).replace(os.sep, "/"))``. + + Returns None where ``relative_to`` raises ValueError (target not under + anchor). ``relative_to`` is purely lexical — a parts-prefix check — so this + performs the same comparison on the already-parsed ``parts`` tuples instead + of constructing a Path object (and, on 3.12+, walking ``parents`` + quadratically) per pattern per scanned entry. That construction was the + dominant cost of large scans (see CHANGELOG: 76k-file vault, 50+ min). + """ + anchor_parts = anchor.parts + if not anchor_parts: + # Anchor without parts (Path(".")): defer to pathlib for its exact + # "." / ValueError semantics. Real anchors are directories with parts, + # so the hot path never lands here. + try: + return _nfc(str(target.relative_to(anchor)).replace(os.sep, "/")) + except ValueError: + return None + n = len(anchor_parts) + if len(target_parts) < n: + return None + head = target_parts[:n] + if head != anchor_parts and ( + not _CASEFOLD_PATHS + or tuple(map(os.path.normcase, head)) + != tuple(map(os.path.normcase, anchor_parts)) + ): + return None + tail = target_parts[n:] + if not tail: + return "." + return _nfc("/".join(tail)) + + def _match_anchored_ignore_pattern(path: str, pattern: str) -> bool: """Match an anchored gitignore pattern without letting ``*`` cross ``/``.""" path_parts = tuple(path.split("/")) @@ -1300,32 +1367,67 @@ def _is_ignored( if not patterns: return False + root_nparts = len(root.parts) + def _eval(target: Path) -> bool: - """Apply last-match-wins to a single target path.""" + """Apply last-match-wins to a single target path. + + Everything derivable from ``target`` alone — its parts, its relative + path per anchor, the root-relative path, the split segments and their + "/"-joined prefixes, the NFC name, the is_dir() stat — is computed at + most ONCE per call, no matter how many patterns are evaluated. The + previous shape rebuilt pathlib objects (``relative_to``) and re-split + strings per PATTERN per entry, which pinned real scans for tens of + minutes (see CHANGELOG). + """ if _cache is not None and target in _cache: return _cache[target] + + target_parts = target.parts + target_name_nfc: str | None = None + target_is_dir: bool | None = None + rel_root_known = False + rel_root: str | None = None + # rel string (or None = outside anchor) and part-count per distinct + # anchor; patterns overwhelmingly share a handful of anchors. + rel_by_anchor: dict[Path, tuple[str | None, int]] = {} + # split segments + accumulated "/" prefixes per distinct rel string. + segs_by_rel: dict[str, tuple[list[str], list[str]]] = {} + + def _segments(rel: str) -> tuple[list[str], list[str]]: + got = segs_by_rel.get(rel) + if got is None: + parts = rel.split("/") + prefixes: list[str] = [] + acc = "" + for part in parts: + acc = part if not acc else acc + "/" + part + prefixes.append(acc) + got = (parts, prefixes) + segs_by_rel[rel] = got + return got + def _matches(rel: str, p: str, path_relative: bool) -> bool: + nonlocal target_name_nfc if path_relative: return _match_anchored_ignore_pattern(rel, p) - parts = rel.split("/") if fnmatch.fnmatch(rel, p): return True - if fnmatch.fnmatch(_nfc(target.name), p): + if target_name_nfc is None: + target_name_nfc = _nfc(target.name) + if fnmatch.fnmatch(target_name_nfc, p): return True - for i, part in enumerate(parts): + parts, prefixes = _segments(rel) + for part, prefix in zip(parts, prefixes): if fnmatch.fnmatch(part, p): return True - if fnmatch.fnmatch("/".join(parts[:i + 1]), p): + if fnmatch.fnmatch(prefix, p): return True return False result = False for anchor, pattern in patterns: - negated = pattern.startswith("!") - raw = pattern[1:] if negated else pattern - directory_only = raw.endswith("/") - path_relative = "/" in raw.rstrip("/") - p = raw.strip("/") + negated, directory_only, path_relative, p = _parse_ignore_pattern(pattern) if not p: continue @@ -1334,22 +1436,31 @@ def _is_ignored( # 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. - matched = False - try: - rel_anchor = _nfc(str(target.relative_to(anchor)).replace(os.sep, "/")) - except ValueError: + cached_rel = rel_by_anchor.get(anchor) + if cached_rel is None: + cached_rel = ( + _lexical_relative(target, target_parts, anchor), + len(anchor.parts), + ) + rel_by_anchor[anchor] = cached_rel + rel_anchor, anchor_nparts = cached_rel + if rel_anchor is None: continue # target outside this pattern's anchor: cannot match + matched = False if rel_anchor != ".": rel = rel_anchor - if not path_relative: - try: - if len(root.parts) > len(anchor.parts): - rel = _nfc(str(target.relative_to(root)).replace(os.sep, "/")) - except ValueError: - pass + if not path_relative and root_nparts > anchor_nparts: + if not rel_root_known: + rel_root_known = True + rel_root = _lexical_relative(target, target_parts, root) + if rel_root is not None: + rel = rel_root matched = _matches(rel, p, path_relative=path_relative) - if matched and directory_only and not target.is_dir(): - matched = False + if matched and directory_only: + if target_is_dir is None: + target_is_dir = target.is_dir() + if not target_is_dir: + matched = False if matched: result = not negated # last match wins; ! flips to un-ignore @@ -1673,10 +1784,13 @@ def detect(root: Path, *, follow_symlinks: bool | None = None, google_workspace: # Record pruned-as-noise dirs so a wrongly-pruned real # source dir is at least traceable in the output rather # than vanishing silently (#2058). - pruned_noise.append(str(dp / d) + os.sep) + pruned_noise.append(str(child) + os.sep) continue - if _ignored_for_scan(dp / d): - ignored.append(str(dp / d) + os.sep) + # Directory-level pruning: ONE ignore evaluation excludes the + # whole subtree — os.walk never descends, so a 29k-file + # ignored dir costs one check, not one per contained file. + if _ignored_for_scan(child): + ignored.append(str(child) + os.sep) continue kept_dirs.append(d) dirnames[:] = kept_dirs diff --git a/tests/test_detect.py b/tests/test_detect.py index 94642424b..e74da8481 100644 --- a/tests/test_detect.py +++ b/tests/test_detect.py @@ -1523,6 +1523,194 @@ def test_is_ignored_cache_evaluates_each_dir_once(): assert eval_counts[f] == 1 +# Regression tests for the per-file pathlib ignore-evaluation defect: a scan of +# a 76k-file vault with a 29k-file directory listed in .graphifyignore pinned a +# CPU for 50+ minutes inside _eval's per-pattern Path.relative_to calls. The +# walk must pay ONE evaluation per ignored directory (pruning), and each +# evaluated entry must pay string matching, not per-pattern Path construction. + +def test_ignored_dir_pruned_walk_never_lists_contents(tmp_path, monkeypatch): + """A dir matching an ignore pattern is pruned from os.walk in one evaluation. + + The walk must never descend into it (its files are never listed), while + non-ignored siblings are still detected. Mirrors the real defect shape: + a regenerated notes/ dir listed in .graphifyignore. + """ + import graphify.detect as det + + (tmp_path / ".graphifyignore").write_text("graphify-notes/\n") + for d in range(5): + sub = tmp_path / "graphify-notes" / f"note-dir-{d}" + sub.mkdir(parents=True) + for f in range(4): + (sub / f"note-{f}.md").write_text("# note") + (tmp_path / "src").mkdir() + (tmp_path / "src" / "app.py").write_text("y = 2") + (tmp_path / "notes.md").write_text("# kept") + + visited: list[str] = [] + real_walk = os.walk + + def tracking_walk(top, *args, **kwargs): + for dirpath, dirnames, filenames in real_walk(top, *args, **kwargs): + visited.append(dirpath) + yield dirpath, dirnames, filenames + + monkeypatch.setattr(det.os, "walk", tracking_walk) + result = det.detect(tmp_path) + + assert not any("graphify-notes" in Path(v).parts for v in visited), ( + "walk descended into the ignored dir — directory pruning regressed" + ) + all_files = as_posix_list(p for cat in result["files"].values() for p in cat) + assert not any("graphify-notes" in p for p in all_files) + assert any(p.endswith("src/app.py") for p in all_files) + assert any(p.endswith("notes.md") for p in all_files) + # The pruned dir is reported once, as a directory entry. + ignored = as_posix_list(result["ignored"]) + assert sum("graphify-notes" in p for p in ignored) == 1 + + +def test_scan_ignore_cost_is_per_directory_not_per_file(tmp_path, monkeypatch): + """On a 2,000-file tree under an ignored dir, ignore evaluation is O(dirs). + + Counts _is_scan_ignored and _is_ignored invocations during detect(): the + ignored subtree must cost exactly one directory-level check — never one + per contained file (the 29k-file defect shape). + """ + import graphify.detect as det + + n_ignored_dirs, n_ignored_files_per_dir = 100, 20 # 2,000 ignored files + (tmp_path / ".graphifyignore").write_text("graphify-notes/\n") + for d in range(n_ignored_dirs): + sub = tmp_path / "graphify-notes" / f"note-dir-{d:03d}" + sub.mkdir(parents=True) + for f in range(n_ignored_files_per_dir): + (sub / f"note-{f:02d}.md").write_text("# note") + kept = 0 + for d in range(5): + sub = tmp_path / "src" / f"pkg{d}" + sub.mkdir(parents=True) + for f in range(10): + (sub / f"mod{f}.py").write_text("x = 1") + kept += 1 + + scan_calls = {"total": 0, "under_ignored": 0} + real_scan = det._is_scan_ignored + + def counting_scan(path, *args, **kwargs): + scan_calls["total"] += 1 + if "graphify-notes" in path.parts: + scan_calls["under_ignored"] += 1 + return real_scan(path, *args, **kwargs) + + ignored_calls = {"total": 0} + real_is_ignored = det._is_ignored + + def counting_is_ignored(*args, **kwargs): + ignored_calls["total"] += 1 + return real_is_ignored(*args, **kwargs) + + monkeypatch.setattr(det, "_is_scan_ignored", counting_scan) + monkeypatch.setattr(det, "_is_ignored", counting_is_ignored) + result = det.detect(tmp_path) + + assert result["total_files"] == kept + # Exactly ONE ignore evaluation for the whole ignored subtree. + assert scan_calls["under_ignored"] == 1, ( + f"{scan_calls['under_ignored']} evaluations under the ignored dir — " + "expected 1 (the directory itself); per-file evaluation regressed" + ) + # Overall evaluation count scales with kept entries + dirs, not with the + # 2,000 files inside the ignored dir. + budget = 4 * (kept + n_ignored_dirs) + assert scan_calls["total"] < budget, ( + f"{scan_calls['total']} _is_scan_ignored calls, expected < {budget}" + ) + assert ignored_calls["total"] < 2 * budget, ( + f"{ignored_calls['total']} _is_ignored calls, expected < {2 * budget}" + ) + + +def test_is_ignored_no_per_pattern_path_construction(monkeypatch): + """_eval must not build pathlib objects per pattern. + + The old shape called target.relative_to(anchor) for EVERY pattern for + every entry (pathlib construction dominated a 52-minute scan, caught by + py-spy at detect.py's _eval). With N patterns, one _is_ignored call must + make O(1) relative_to calls — not O(N). + """ + import pathlib + + from graphify.detect import _is_ignored + + root = Path("/repo") + patterns = [(root, f"*.zzz{i}") for i in range(200)] + patterns.append((root, "never-matches-dir/")) + target = root / "a" / "b" / "c" / "leaf.py" + + counts = {"relative_to": 0} + real_relative_to = pathlib.PurePath.relative_to + + def counting_relative_to(self, *args, **kwargs): + counts["relative_to"] += 1 + return real_relative_to(self, *args, **kwargs) + + monkeypatch.setattr(pathlib.PurePath, "relative_to", counting_relative_to) + assert not _is_ignored(target, root, patterns) + + # One rel_parts computation for the ancestor walk; _eval itself must add + # none. Old code: ~200+ (one per pattern, per evaluated ancestor + leaf). + assert counts["relative_to"] <= 6, ( + f"{counts['relative_to']} relative_to calls for 201 patterns — " + "per-pattern pathlib construction is back" + ) + + +def test_string_matcher_preserves_pattern_forms(tmp_path): + """Each supported pattern form matches/misses exactly as before the + string-space rewrite of _eval (rel computed once per entry, patterns + matched with fnmatch/segment logic on strings).""" + (tmp_path / "build").mkdir() + (tmp_path / "build" / "x.py").write_text("x=1") + (tmp_path / "notbuild").mkdir() + (tmp_path / "notbuild" / "build").write_text("plain file named build") + (tmp_path / "logs").mkdir() + (tmp_path / "logs" / "drop.log").write_text("x") + (tmp_path / "logs" / "keep.log").write_text("x") + (tmp_path / "anchored.md").write_text("x") + (tmp_path / "sub").mkdir() + (tmp_path / "sub" / "anchored.md").write_text("x") + (tmp_path / "docs" / "deep").mkdir(parents=True) + (tmp_path / "docs" / "deep" / "guide.md").write_text("x") + (tmp_path / "x" / "cache" / "y").mkdir(parents=True) + (tmp_path / "x" / "cache" / "y" / "f.py").write_text("x=1") + (tmp_path / ".graphifyignore").write_text( + "build/\n*.log\n!keep.log\n/anchored.md\ndocs/**\ncache\n" + ) + patterns = _load_graphifyignore(tmp_path) + + # dir-only pattern: matches the dir and everything under it... + assert _is_ignored(tmp_path / "build", tmp_path, patterns) + assert _is_ignored(tmp_path / "build" / "x.py", tmp_path, patterns) + # ...but never a plain FILE of the same name + assert not _is_ignored(tmp_path / "notbuild" / "build", tmp_path, patterns) + # glob + basename negation, last match wins + assert _is_ignored(tmp_path / "logs" / "drop.log", tmp_path, patterns) + assert not _is_ignored(tmp_path / "logs" / "keep.log", tmp_path, patterns) + # anchored file pattern: root level only + assert _is_ignored(tmp_path / "anchored.md", tmp_path, patterns) + assert not _is_ignored(tmp_path / "sub" / "anchored.md", tmp_path, patterns) + # ** crosses path segments under its anchor + assert _is_ignored(tmp_path / "docs" / "deep" / "guide.md", tmp_path, patterns) + # bare segment pattern matches an intermediate directory anywhere + assert _is_ignored(tmp_path / "x" / "cache", tmp_path, patterns) + assert _is_ignored(tmp_path / "x" / "cache" / "y" / "f.py", tmp_path, patterns) + # unrelated paths untouched + assert not _is_ignored(tmp_path / "notbuild", tmp_path, patterns) + assert not _is_ignored(tmp_path / "sub", tmp_path, patterns) + + # Regression tests for #920 - sensitive pattern misses underscore-prefixed names def test_sensitive_flags_api_token_txt(): assert _is_sensitive(Path("api_token.txt"))