mirror of
https://github.com/safishamsi/graphify.git
synced 2026-08-28 01:06:36 +00:00
perf(detect): kill the globstar matcher's per-call cycle, bound the pattern cache (#2226)
Senior tweaks on the landed #3074: - Lift the **-aware matcher out of the per-call @lru_cache closure (which referenced itself and leaked a reference cycle every call) into a module-level function with an explicit dict memo, plus a no-** fast path; port of the fix from the alternative #2845. - Cap the process-global parsed-pattern cache so a long-lived watch across many repos cannot grow it without bound. - Add a differential test pinning _lexical_relative against pathlib relative_to, and a gc test asserting the matcher leaves no cyclic garbage.
This commit is contained in:
+62
-23
@@ -11,7 +11,6 @@ import time
|
||||
import unicodedata
|
||||
from concurrent.futures import ThreadPoolExecutor
|
||||
from enum import Enum
|
||||
from functools import lru_cache
|
||||
from pathlib import Path
|
||||
from typing import Callable
|
||||
|
||||
@@ -1253,8 +1252,12 @@ def _load_graphifyignore(root: Path, *, gitignore: bool = True) -> list[tuple[Pa
|
||||
# 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.
|
||||
# the corpus's ignore files, which is small and bounded per scan. A long-lived
|
||||
# `graphify watch` process spanning many repos could still accumulate keys over
|
||||
# time, so cap it and clear wholesale on overflow (parsing is cheap, so a rare
|
||||
# full re-fill costs nothing that matters).
|
||||
_PARSED_PATTERN_CACHE: dict[str, tuple[bool, bool, bool, str]] = {}
|
||||
_PARSED_PATTERN_CACHE_MAX = 100_000
|
||||
|
||||
|
||||
def _parse_ignore_pattern(pattern: str) -> tuple[bool, bool, bool, str]:
|
||||
@@ -1270,6 +1273,8 @@ def _parse_ignore_pattern(pattern: str) -> tuple[bool, bool, bool, str]:
|
||||
directory_only = raw.endswith("/")
|
||||
path_relative = "/" in raw.rstrip("/")
|
||||
got = (negated, directory_only, path_relative, raw.strip("/"))
|
||||
if len(_PARSED_PATTERN_CACHE) >= _PARSED_PATTERN_CACHE_MAX:
|
||||
_PARSED_PATTERN_CACHE.clear()
|
||||
_PARSED_PATTERN_CACHE[pattern] = got
|
||||
return got
|
||||
|
||||
@@ -1316,32 +1321,66 @@ def _lexical_relative(
|
||||
return _nfc("/".join(tail))
|
||||
|
||||
|
||||
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:
|
||||
"""Recursive ``**``-aware segment match, memoized via an explicit dict.
|
||||
|
||||
Lifted out of ``_match_anchored_ignore_pattern`` (was a per-call
|
||||
``@lru_cache`` closure): the decorated inner closure referenced itself, so
|
||||
every call leaked a reference cycle for the GC to reclaim on this hot path.
|
||||
A plain dict passed in avoids both the cycle and the per-call cache setup.
|
||||
"""
|
||||
key = (path_idx, pattern_idx)
|
||||
cached = memo.get(key)
|
||||
if cached is not None:
|
||||
return cached
|
||||
|
||||
if pattern_idx == len(pattern_parts):
|
||||
result = path_idx == len(path_parts)
|
||||
else:
|
||||
part = pattern_parts[pattern_idx]
|
||||
if part == "**":
|
||||
if pattern_idx == len(pattern_parts) - 1:
|
||||
result = path_idx < len(path_parts)
|
||||
else:
|
||||
result = _match_globstar_parts(
|
||||
path_parts, pattern_parts, path_idx, pattern_idx + 1, memo
|
||||
) or (
|
||||
path_idx < len(path_parts)
|
||||
and _match_globstar_parts(
|
||||
path_parts, pattern_parts, path_idx + 1, pattern_idx, memo
|
||||
)
|
||||
)
|
||||
else:
|
||||
result = (
|
||||
path_idx < len(path_parts)
|
||||
and fnmatch.fnmatchcase(path_parts[path_idx], part)
|
||||
and _match_globstar_parts(
|
||||
path_parts, pattern_parts, path_idx + 1, pattern_idx + 1, memo
|
||||
)
|
||||
)
|
||||
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("/"))
|
||||
|
||||
@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)
|
||||
|
||||
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)
|
||||
)
|
||||
|
||||
return (
|
||||
path_idx < len(path_parts)
|
||||
and fnmatch.fnmatchcase(path_parts[path_idx], part)
|
||||
and _matches(path_idx + 1, pattern_idx + 1)
|
||||
# Fast path: with no ``**`` the match is a straight segment-wise fnmatch of
|
||||
# equal-length paths, so skip the recursive matcher and its memo entirely.
|
||||
if "**" not in pattern_parts:
|
||||
if len(path_parts) != len(pattern_parts):
|
||||
return False
|
||||
return all(
|
||||
fnmatch.fnmatchcase(pp, qp) for pp, qp in zip(path_parts, pattern_parts)
|
||||
)
|
||||
|
||||
return _matches(0, 0)
|
||||
return _match_globstar_parts(path_parts, pattern_parts, 0, 0, {})
|
||||
|
||||
|
||||
def _is_ignored(
|
||||
|
||||
@@ -3397,3 +3397,47 @@ 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)"
|
||||
|
||||
|
||||
def test_lexical_relative_matches_pathlib_relative_to():
|
||||
"""The string-space `_lexical_relative` must return exactly what
|
||||
`_nfc(str(target.relative_to(anchor)).replace(os.sep, "/"))` would — including
|
||||
None where relative_to raises (target not under anchor). Guards the
|
||||
reimplemented relative_to against silent drift (#2226)."""
|
||||
from pathlib import Path
|
||||
from graphify.detect import _lexical_relative, _nfc
|
||||
|
||||
anchors = ["/a/b", "/a", "/a/b/c", "/x", "/"]
|
||||
targets = [
|
||||
"/a/b/c/d.py", "/a/b", "/a/b/c", "/a/x.py", "/a/b/c/d/e.py",
|
||||
"/x/y.py", "/other/z.py", "/a/bb/c.py", "/a/b/c",
|
||||
]
|
||||
for r in anchors:
|
||||
anchor = Path(r)
|
||||
for t in targets:
|
||||
target = Path(t)
|
||||
try:
|
||||
expected = _nfc(str(target.relative_to(anchor)).replace(os.sep, "/"))
|
||||
except ValueError:
|
||||
expected = None
|
||||
assert _lexical_relative(target, target.parts, anchor) == expected, (t, r)
|
||||
|
||||
|
||||
def test_globstar_matcher_leaves_no_reference_cycle():
|
||||
"""`_match_anchored_ignore_pattern` must not leak a reference cycle per call,
|
||||
as the old per-call `@lru_cache` closure did (it referenced itself). With gc
|
||||
disabled, a run of the matcher must leave nothing for the collector."""
|
||||
import gc
|
||||
from graphify.detect import _match_anchored_ignore_pattern
|
||||
|
||||
gc.collect()
|
||||
gc.disable()
|
||||
try:
|
||||
for _ in range(500):
|
||||
assert _match_anchored_ignore_pattern("docs/deep/guide.md", "docs/**")
|
||||
assert not _match_anchored_ignore_pattern("src/app.py", "docs/**")
|
||||
assert _match_anchored_ignore_pattern("a/b/c.py", "a/*/c.py")
|
||||
collected = gc.collect()
|
||||
finally:
|
||||
gc.enable()
|
||||
assert collected == 0, f"globstar matcher leaked {collected} cyclic objects per run"
|
||||
|
||||
Reference in New Issue
Block a user