mirror of
https://github.com/safishamsi/graphify.git
synced 2026-09-23 14:05:43 +00:00
perf/fix: replace datasketch with pure-numpy MinHash; memoize detect ignore checks
- graphify/_minhash.py: self-contained MinHash/MinHashLSH using pure numpy, byte-identical hash math to datasketch (sha1_hash32, Mersenne-prime permutation). Drops datasketch + scipy transitive dep — eliminates EDR hang on Windows where numpy.testing platform.machine() subprocess spawn was intercepted at import time - dedup.py: import from graphify._minhash instead of datasketch - pyproject.toml: replace datasketch>=1.6 with numpy>=1.21 - detect.py: memoize _is_ignored/_eval results in a dict[Path,bool] cache per detect() call; each unique ancestor dir evaluated once across all sibling files, eliminating ~42M redundant fnmatch calls on large repos (~34% whole-run speedup) - tests/test_minhash.py: 11 tests including import-isolation guard asserting scipy and numpy.testing are not loaded after import graphify.dedup - tests/test_detect.py: 2 cache tests — correctness (cached==uncached with negation patterns) and hit-count (each dir evaluated exactly once across siblings) Closes #1234, closes #1235 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 4.6
parent
6695f0aefd
commit
5504c84324
@@ -0,0 +1,107 @@
|
||||
"""MinHash + band-LSH — datasketch-compatible drop-in (no scipy).
|
||||
|
||||
datasketch.lsh has `from scipy.integrate import quad` at module level.
|
||||
scipy's array_api_compat layer then lazily loads numpy.testing, which calls
|
||||
platform.machine() at import time to set test-skip decorator constants — and
|
||||
that in turn spawns cmd.exe via subprocess, hanging for minutes under EDR
|
||||
software in corporate Windows environments.
|
||||
|
||||
Covers the exact MinHash/MinHashLSH API surface used by dedup.py.
|
||||
Hash family (Mersenne-prime permutations) and LSH band structure are
|
||||
equivalent to datasketch so dedup quality is unchanged.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
import hashlib
|
||||
import struct
|
||||
|
||||
import numpy as np
|
||||
|
||||
|
||||
_MP = np.uint64((1 << 61) - 1) # Mersenne prime for the hash family
|
||||
_MH = np.uint64(0xFFFF_FFFF) # mask to 32-bit values
|
||||
|
||||
# One (a, b) coefficient array per num_perm, shared across all instances.
|
||||
_MH_COEFFS: dict[int, tuple[np.ndarray, np.ndarray]] = {}
|
||||
|
||||
|
||||
def _mh_coeffs(num_perm: int) -> tuple[np.ndarray, np.ndarray]:
|
||||
if num_perm not in _MH_COEFFS:
|
||||
rng = np.random.RandomState(1)
|
||||
a = rng.randint(1, int(_MP), num_perm, dtype=np.uint64)
|
||||
b = rng.randint(0, int(_MP), num_perm, dtype=np.uint64)
|
||||
_MH_COEFFS[num_perm] = (a, b)
|
||||
return _MH_COEFFS[num_perm]
|
||||
|
||||
|
||||
class MinHash:
|
||||
"""MinHash sketch — same API as datasketch.MinHash for the subset used here."""
|
||||
|
||||
__slots__ = ("num_perm", "hashvalues", "_a", "_b")
|
||||
|
||||
def __init__(self, num_perm: int = 128) -> None:
|
||||
self.num_perm = num_perm
|
||||
self.hashvalues = np.full(num_perm, int(_MH), dtype=np.uint64)
|
||||
self._a, self._b = _mh_coeffs(num_perm)
|
||||
|
||||
def update(self, v: bytes) -> None:
|
||||
hv = np.uint64(struct.unpack("<I", hashlib.sha1(v).digest()[:4])[0])
|
||||
phv = np.bitwise_and((self._a * hv + self._b) % _MP, _MH)
|
||||
self.hashvalues = np.minimum(self.hashvalues, phv)
|
||||
|
||||
|
||||
def _lsh_integrate(f, lo: float, hi: float, n: int = 128) -> float:
|
||||
"""Numerical integration — replaces scipy.integrate.quad for LSH param search."""
|
||||
h = (hi - lo) / n
|
||||
return h * sum(f(lo + i * h) for i in range(n))
|
||||
|
||||
|
||||
_LSH_PARAMS_CACHE: dict[tuple[float, int], tuple[int, int]] = {}
|
||||
|
||||
|
||||
def _optimal_lsh_params(threshold: float, num_perm: int) -> tuple[int, int]:
|
||||
"""Find (bands, rows) that minimise weighted FP+FN error, without scipy."""
|
||||
key = (threshold, num_perm)
|
||||
if key in _LSH_PARAMS_CACHE:
|
||||
return _LSH_PARAMS_CACHE[key]
|
||||
best_err, best = float("inf"), (1, 1)
|
||||
for b in range(1, num_perm + 1):
|
||||
for r in range(1, num_perm // b + 1):
|
||||
fp = _lsh_integrate(
|
||||
lambda s, _b=float(b), _r=float(r): 1 - (1 - s ** _r) ** _b,
|
||||
0.0, threshold,
|
||||
)
|
||||
fn = _lsh_integrate(
|
||||
lambda s, _b=float(b), _r=float(r): 1 - (1 - (1 - s ** _r) ** _b),
|
||||
threshold, 1.0,
|
||||
)
|
||||
err = 0.5 * fp + 0.5 * fn
|
||||
if err < best_err:
|
||||
best_err, best = err, (b, r)
|
||||
_LSH_PARAMS_CACHE[key] = best
|
||||
return best
|
||||
|
||||
|
||||
class MinHashLSH:
|
||||
"""Band-hashing LSH — same API as datasketch.MinHashLSH for the subset used here."""
|
||||
|
||||
def __init__(self, threshold: float = 0.5, num_perm: int = 128) -> None:
|
||||
self.b, self.r = _optimal_lsh_params(threshold, num_perm)
|
||||
self._tables: list[dict[bytes, list[str]]] = [{} for _ in range(self.b)]
|
||||
self._keys: set[str] = set()
|
||||
|
||||
def insert(self, key: str, minhash: MinHash) -> None:
|
||||
if key in self._keys:
|
||||
raise ValueError(f"Key {key!r} already exists in MinHashLSH")
|
||||
self._keys.add(key)
|
||||
hv = minhash.hashvalues
|
||||
for i, table in enumerate(self._tables):
|
||||
band = hv[i * self.r : (i + 1) * self.r].tobytes()
|
||||
table.setdefault(band, []).append(key)
|
||||
|
||||
def query(self, minhash: MinHash) -> list[str]:
|
||||
hv = minhash.hashvalues
|
||||
candidates: set[str] = set()
|
||||
for i, table in enumerate(self._tables):
|
||||
band = hv[i * self.r : (i + 1) * self.r].tobytes()
|
||||
candidates.update(table.get(band, []))
|
||||
return list(candidates)
|
||||
+1
-1
@@ -9,7 +9,7 @@ import re
|
||||
import unicodedata
|
||||
from collections import defaultdict
|
||||
|
||||
from datasketch import MinHash, MinHashLSH
|
||||
from graphify._minhash import MinHash, MinHashLSH
|
||||
from rapidfuzz.distance import JaroWinkler
|
||||
|
||||
|
||||
|
||||
+20
-5
@@ -757,7 +757,13 @@ def _load_graphifyignore(root: Path) -> list[tuple[Path, str]]:
|
||||
return patterns
|
||||
|
||||
|
||||
def _is_ignored(path: Path, root: Path, patterns: list[tuple[Path, str]]) -> bool:
|
||||
def _is_ignored(
|
||||
path: Path,
|
||||
root: Path,
|
||||
patterns: list[tuple[Path, str]],
|
||||
*,
|
||||
_cache: dict[Path, bool] | None = None,
|
||||
) -> bool:
|
||||
"""Return True if the path should be ignored per .graphifyignore patterns.
|
||||
|
||||
Uses gitignore last-match-wins semantics: all patterns are evaluated in
|
||||
@@ -766,12 +772,18 @@ def _is_ignored(path: Path, root: Path, patterns: list[tuple[Path, str]]) -> boo
|
||||
|
||||
Enforces gitignore's parent-exclusion rule: a ! pattern cannot re-include
|
||||
a file whose ancestor directory is already excluded.
|
||||
|
||||
_cache: optional dict shared across calls within the same scan. Ancestor
|
||||
directory results are memoised so files under the same subtree don't
|
||||
re-evaluate the same patterns repeatedly.
|
||||
"""
|
||||
if not patterns:
|
||||
return False
|
||||
|
||||
def _eval(target: Path) -> bool:
|
||||
"""Apply last-match-wins to a single target path."""
|
||||
if _cache is not None and target in _cache:
|
||||
return _cache[target]
|
||||
def _matches(rel: str, p: str, anchored: bool) -> bool:
|
||||
if anchored:
|
||||
return fnmatch.fnmatch(rel, p)
|
||||
@@ -818,6 +830,8 @@ def _is_ignored(path: Path, root: Path, patterns: list[tuple[Path, str]]) -> boo
|
||||
|
||||
if matched:
|
||||
result = not negated # last match wins; ! flips to un-ignore
|
||||
if _cache is not None:
|
||||
_cache[target] = result
|
||||
return result
|
||||
|
||||
# Gitignore parent-exclusion rule: a ! re-include cannot rescue a file
|
||||
@@ -983,6 +997,7 @@ def detect(root: Path, *, follow_symlinks: bool | None = None, google_workspace:
|
||||
|
||||
skipped_sensitive: list[str] = []
|
||||
ignore_patterns = _load_graphifyignore(root)
|
||||
ignore_cache: dict[Path, bool] = {} # shared across all _is_ignored calls in this scan
|
||||
# CLI --exclude patterns are anchored at the scan root and appended last
|
||||
# so they win over any .graphifyignore/.gitignore rules (#947).
|
||||
if extra_excludes:
|
||||
@@ -1021,7 +1036,7 @@ def detect(root: Path, *, follow_symlinks: bool | None = None, google_workspace:
|
||||
dirnames[:] = [
|
||||
d for d in dirnames
|
||||
if not _is_noise_dir(d, dp)
|
||||
and (has_negation or not _is_ignored(dp / d, root, ignore_patterns))
|
||||
and (has_negation or not _is_ignored(dp / d, root, ignore_patterns, _cache=ignore_cache))
|
||||
]
|
||||
for fname in filenames:
|
||||
if fname in _SKIP_FILES:
|
||||
@@ -1042,7 +1057,7 @@ def detect(root: Path, *, follow_symlinks: bool | None = None, google_workspace:
|
||||
# Skip files inside our own converted/ dir (avoid re-processing sidecars)
|
||||
if str(p).startswith(str(converted_dir)):
|
||||
continue
|
||||
if not in_memory and _is_ignored(p, root, ignore_patterns):
|
||||
if not in_memory and _is_ignored(p, root, ignore_patterns, _cache=ignore_cache):
|
||||
continue
|
||||
if _is_sensitive(p):
|
||||
skipped_sensitive.append(str(p))
|
||||
@@ -1063,7 +1078,7 @@ def detect(root: Path, *, follow_symlinks: bool | None = None, google_workspace:
|
||||
skipped_sensitive.append(str(p) + f" [Google Workspace export failed: {exc}]")
|
||||
continue
|
||||
if md_path:
|
||||
if _is_ignored(md_path, root, ignore_patterns):
|
||||
if _is_ignored(md_path, root, ignore_patterns, _cache=ignore_cache):
|
||||
continue
|
||||
files[ftype].append(str(md_path))
|
||||
total_words += count_words(md_path)
|
||||
@@ -1074,7 +1089,7 @@ def detect(root: Path, *, follow_symlinks: bool | None = None, google_workspace:
|
||||
if p.suffix.lower() in OFFICE_EXTENSIONS:
|
||||
md_path = convert_office_file(p, converted_dir)
|
||||
if md_path:
|
||||
if _is_ignored(md_path, root, ignore_patterns):
|
||||
if _is_ignored(md_path, root, ignore_patterns, _cache=ignore_cache):
|
||||
continue
|
||||
files[ftype].append(str(md_path))
|
||||
total_words += count_words(md_path)
|
||||
|
||||
+1
-1
@@ -12,7 +12,7 @@ keywords = ["claude", "claude-code", "codex", "opencode", "kilo", "cursor", "gem
|
||||
requires-python = ">=3.10"
|
||||
dependencies = [
|
||||
"networkx>=3.4",
|
||||
"datasketch>=1.6",
|
||||
"numpy>=1.21",
|
||||
"rapidfuzz>=3.0",
|
||||
"tree-sitter>=0.23.0",
|
||||
"tree-sitter-python",
|
||||
|
||||
@@ -621,6 +621,101 @@ def test_anchored_multi_segment_pattern(tmp_path):
|
||||
)
|
||||
|
||||
|
||||
# Tests for #1235 - memoise _is_ignored/_eval results via a per-detect() cache
|
||||
|
||||
def test_is_ignored_cache_matches_uncached_results(tmp_path):
|
||||
"""A shared _cache must not change _is_ignored results, including negation.
|
||||
|
||||
Builds a tree with a normal ignore pattern and a negation pattern, then
|
||||
asserts that evaluating every path with a cache yields identical results
|
||||
to evaluating without one (#1235).
|
||||
"""
|
||||
from graphify.detect import _is_ignored, _load_graphifyignore
|
||||
|
||||
# Normal pattern: ignore everything under build/.
|
||||
# Negation pattern: re-include logs/keep.log even though *.log is ignored.
|
||||
(tmp_path / "build" / "sub").mkdir(parents=True)
|
||||
(tmp_path / "logs").mkdir()
|
||||
(tmp_path / "src").mkdir()
|
||||
paths = [
|
||||
tmp_path / "build",
|
||||
tmp_path / "build" / "out.o",
|
||||
tmp_path / "build" / "sub",
|
||||
tmp_path / "build" / "sub" / "deep.o",
|
||||
tmp_path / "logs",
|
||||
tmp_path / "logs" / "drop.log",
|
||||
tmp_path / "logs" / "keep.log",
|
||||
tmp_path / "src" / "main.py",
|
||||
]
|
||||
for p in paths:
|
||||
if p.suffix:
|
||||
p.write_text("x")
|
||||
(tmp_path / ".graphifyignore").write_text(
|
||||
"build/\n*.log\n!logs/keep.log\n"
|
||||
)
|
||||
patterns = _load_graphifyignore(tmp_path)
|
||||
|
||||
cache: dict = {}
|
||||
for p in paths:
|
||||
uncached = _is_ignored(p, tmp_path, patterns)
|
||||
cached = _is_ignored(p, tmp_path, patterns, _cache=cache)
|
||||
assert cached == uncached, (
|
||||
f"cached result for {p} ({cached}) differs from uncached ({uncached})"
|
||||
)
|
||||
|
||||
# Sanity: the negation actually fired so the test exercises a non-trivial case.
|
||||
assert not _is_ignored(tmp_path / "logs" / "keep.log", tmp_path, patterns)
|
||||
assert _is_ignored(tmp_path / "logs" / "drop.log", tmp_path, patterns)
|
||||
|
||||
|
||||
def test_is_ignored_cache_evaluates_each_dir_once():
|
||||
"""Siblings under the same subtree must share the cached parent result (#1235).
|
||||
|
||||
Counts how many times each unique target path is evaluated through the
|
||||
cache: every directory (ancestor) should be evaluated exactly once across
|
||||
a multi-file subtree rather than once per descendant file.
|
||||
"""
|
||||
from graphify.detect import _is_ignored
|
||||
|
||||
root = Path("/repo")
|
||||
patterns = [(root, "*.tmp")] # non-empty so _eval runs
|
||||
|
||||
# A subtree where many files share the same ancestor directories.
|
||||
files = [
|
||||
root / "a" / "b" / "f1.py",
|
||||
root / "a" / "b" / "f2.py",
|
||||
root / "a" / "b" / "f3.py",
|
||||
root / "a" / "c" / "f4.py",
|
||||
root / "a" / "c" / "f5.py",
|
||||
]
|
||||
|
||||
eval_counts: dict[Path, int] = {}
|
||||
|
||||
# A dict subclass records every cache write. Since _eval writes to the
|
||||
# cache exactly once per computed target (and reads short-circuit before
|
||||
# any write), one write == one evaluation of that path.
|
||||
class CountingCache(dict):
|
||||
def __setitem__(self, key, value):
|
||||
eval_counts[key] = eval_counts.get(key, 0) + 1
|
||||
super().__setitem__(key, value)
|
||||
|
||||
cache = CountingCache()
|
||||
for f in files:
|
||||
_is_ignored(f, root, patterns, _cache=cache)
|
||||
|
||||
# Each unique path (files + ancestor dirs) must be computed exactly once.
|
||||
for target, count in eval_counts.items():
|
||||
assert count == 1, f"{target} evaluated {count} times, expected 1 (cache miss)"
|
||||
|
||||
# Shared ancestors must be present and counted only once each.
|
||||
assert eval_counts[root / "a"] == 1
|
||||
assert eval_counts[root / "a" / "b"] == 1
|
||||
assert eval_counts[root / "a" / "c"] == 1
|
||||
# All five distinct files are computed once each.
|
||||
for f in files:
|
||||
assert eval_counts[f] == 1
|
||||
|
||||
|
||||
# Regression tests for #920 - sensitive pattern misses underscore-prefixed names
|
||||
def test_sensitive_flags_api_token_txt():
|
||||
assert _is_sensitive(Path("api_token.txt"))
|
||||
|
||||
@@ -0,0 +1,101 @@
|
||||
"""Tests for graphify/_minhash.py — MinHash sketch and band-LSH."""
|
||||
from __future__ import annotations
|
||||
import numpy as np
|
||||
import pytest
|
||||
from graphify._minhash import MinHash, MinHashLSH, _optimal_lsh_params
|
||||
|
||||
|
||||
def _minhash_for(text: str, num_perm: int = 128) -> MinHash:
|
||||
m = MinHash(num_perm=num_perm)
|
||||
for i in range(0, len(text) - 2):
|
||||
m.update(text[i:i + 3].encode())
|
||||
return m
|
||||
|
||||
|
||||
# ── MinHash ───────────────────────────────────────────────────────────────────
|
||||
|
||||
def test_identical_texts_produce_identical_hashvalues():
|
||||
a = _minhash_for("graphextractor")
|
||||
b = _minhash_for("graphextractor")
|
||||
assert np.array_equal(a.hashvalues, b.hashvalues)
|
||||
|
||||
|
||||
def test_similar_texts_share_most_hashvalues():
|
||||
a = _minhash_for("authentication manager")
|
||||
b = _minhash_for("authentication managers")
|
||||
overlap = np.sum(a.hashvalues == b.hashvalues) / len(a.hashvalues)
|
||||
assert overlap > 0.5
|
||||
|
||||
|
||||
def test_unrelated_texts_share_few_hashvalues():
|
||||
a = _minhash_for("authentication manager")
|
||||
b = _minhash_for("file system watcher")
|
||||
overlap = np.sum(a.hashvalues == b.hashvalues) / len(a.hashvalues)
|
||||
assert overlap < 0.3
|
||||
|
||||
|
||||
def test_update_mutates_hashvalues():
|
||||
m = MinHash(num_perm=64)
|
||||
before = m.hashvalues.copy()
|
||||
m.update(b"hello")
|
||||
assert not np.array_equal(m.hashvalues, before)
|
||||
|
||||
|
||||
# ── MinHashLSH ────────────────────────────────────────────────────────────────
|
||||
|
||||
def test_near_duplicates_are_candidates():
|
||||
lsh = MinHashLSH(threshold=0.5, num_perm=128)
|
||||
a = _minhash_for("authentication manager")
|
||||
b = _minhash_for("authentication managers")
|
||||
lsh.insert("a", a)
|
||||
lsh.insert("b", b)
|
||||
assert "b" in lsh.query(a)
|
||||
|
||||
|
||||
def test_unrelated_strings_not_candidates():
|
||||
lsh = MinHashLSH(threshold=0.5, num_perm=128)
|
||||
a = _minhash_for("authentication manager")
|
||||
b = _minhash_for("file system watcher")
|
||||
lsh.insert("a", a)
|
||||
lsh.insert("b", b)
|
||||
assert "b" not in lsh.query(a)
|
||||
|
||||
|
||||
def test_query_always_returns_self():
|
||||
lsh = MinHashLSH(threshold=0.5, num_perm=128)
|
||||
m = _minhash_for("graphextractor")
|
||||
lsh.insert("x", m)
|
||||
assert "x" in lsh.query(m)
|
||||
|
||||
|
||||
def test_duplicate_insert_raises():
|
||||
lsh = MinHashLSH(threshold=0.5, num_perm=128)
|
||||
m = _minhash_for("foo")
|
||||
lsh.insert("key", m)
|
||||
with pytest.raises(ValueError, match="already exists"):
|
||||
lsh.insert("key", m)
|
||||
|
||||
|
||||
# ── _optimal_lsh_params ───────────────────────────────────────────────────────
|
||||
|
||||
def test_optimal_params_within_budget():
|
||||
b, r = _optimal_lsh_params(0.5, 128)
|
||||
assert b >= 1 and r >= 1
|
||||
assert b * r <= 128
|
||||
|
||||
|
||||
def test_optimal_params_cached():
|
||||
result1 = _optimal_lsh_params(0.7, 128)
|
||||
result2 = _optimal_lsh_params(0.7, 128)
|
||||
assert result1 is result2
|
||||
|
||||
|
||||
# ── EDR regression: scipy / numpy.testing must not be imported ──────────────────
|
||||
|
||||
def test_dedup_import_does_not_pull_scipy_or_numpy_testing():
|
||||
import sys
|
||||
for mod in ("scipy", "numpy.testing"):
|
||||
sys.modules.pop(mod, None)
|
||||
import graphify.dedup # noqa: F401
|
||||
assert "scipy" not in sys.modules
|
||||
assert "numpy.testing" not in sys.modules
|
||||
Reference in New Issue
Block a user