mirror of
https://github.com/safishamsi/graphify.git
synced 2026-11-08 15:07:55 +00:00
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.
This commit is contained in:
1 parent
3b12d5ce68
commit
258ab11db2
2 files changed
+327
-25
No files matched your search
+139
-25
@@ -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
|
||||
|
||||
@@ -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"))
|
||||
|
||||
Reference in new issue
Block a user