mirror of
https://github.com/safishamsi/graphify.git
synced 2026-10-13 15:40:05 +00:00
perf(extract): resolve and parse each path once in the id remap and call tie-break
extract()'s id remap called Path.resolve() on the same file paths once per input, once per target_file-stamped edge and once per node: ~24k calls for ~1k distinct paths on graphify's own repo. Resolve each distinct path once per extract() call through a local memo; a hit returns the same Path the call would, and a path that raises is never stored. _path_proximity_winner (the #1553 god-node tie-break) re-parsed every candidate's parent directory with PurePosixPath on every ambiguous call site (~227k Path objects per update on graphify's repo), although the same candidate files come back for every site of a common name. Memoize the parent-directory parts per path string; equal tuples are exactly equal PurePosixPath parents, so every tier decides as before. graphify's own repo (18,830 nodes), `graphify update --force`, alternated: cold 28.1 -> 26.3 s, no-change update 24.8 -> 22.5 s and 20.9 -> 18.0 s. graph.json identical (all attributes) cold and warm; old vs new tie-break identical on 200,000 random cases. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
f765dcb341
commit
d5966eda8a
3 files changed
+107
-14
No files matched your search
+24
-9
@@ -7993,10 +7993,24 @@ def extract(
|
||||
# stem_forms for those in-root files as well lets the edge remap and the
|
||||
# target_file-guided repoint pass fix them exactly as on a full scan.
|
||||
remap_paths: list[Path] = list(paths)
|
||||
# The remap below resolves the same few hundred file paths once per input,
|
||||
# once per stamped edge and once per node (tens of thousands of calls on a
|
||||
# mid-size repo). Resolve each distinct path once; a hit returns the same
|
||||
# Path the call would, and a path that raises is never stored.
|
||||
_resolve_memo: dict[str, Path] = {}
|
||||
|
||||
def _resolved(p: "str | Path") -> Path:
|
||||
key = str(p)
|
||||
hit = _resolve_memo.get(key)
|
||||
if hit is None:
|
||||
hit = Path(p).resolve()
|
||||
_resolve_memo[key] = hit
|
||||
return hit
|
||||
|
||||
_remap_seen: set[Path] = set()
|
||||
for _p in paths:
|
||||
try:
|
||||
_remap_seen.add(_p.resolve())
|
||||
_remap_seen.add(_resolved(_p))
|
||||
except (OSError, RuntimeError):
|
||||
pass
|
||||
for _e in all_edges:
|
||||
@@ -8005,7 +8019,7 @@ def extract(
|
||||
continue
|
||||
_raw_tp = Path(_tf)
|
||||
try:
|
||||
_tp = _raw_tp.resolve()
|
||||
_tp = _resolved(_raw_tp)
|
||||
except (OSError, RuntimeError):
|
||||
continue
|
||||
if _tp in _remap_seen:
|
||||
@@ -8076,7 +8090,7 @@ def extract(
|
||||
rel = path.relative_to(root)
|
||||
except ValueError:
|
||||
try:
|
||||
rel = path.resolve().relative_to(root)
|
||||
rel = _resolved(path).relative_to(root)
|
||||
except ValueError:
|
||||
continue
|
||||
new_id = _file_node_id(rel)
|
||||
@@ -8085,14 +8099,15 @@ def extract(
|
||||
# Also register the absolute-resolved form of the file-level id so
|
||||
# alias/workspace import targets (resolved via .resolve()) remap to
|
||||
# canonical instead of orphaning (#1529).
|
||||
old_id_abs = _make_id(str(path.resolve()))
|
||||
path_abs = _resolved(path)
|
||||
old_id_abs = _make_id(str(path_abs))
|
||||
if old_id_abs != new_id:
|
||||
id_remap[old_id_abs] = new_id
|
||||
old_prefs: list[tuple[str, str]] = []
|
||||
old_pref = _file_node_id(path)
|
||||
if old_pref != new_id:
|
||||
old_prefs.append((old_pref, new_id))
|
||||
old_pref_abs = _file_node_id(path.resolve())
|
||||
old_pref_abs = _file_node_id(path_abs)
|
||||
if old_pref_abs != new_id and old_pref_abs != old_pref:
|
||||
old_prefs.append((old_pref_abs, new_id))
|
||||
# Bash entrypoint node ids append "__entry" to the file-level id
|
||||
@@ -8111,10 +8126,10 @@ def extract(
|
||||
if _entry_old != _entry_new:
|
||||
id_remap.setdefault(_entry_old, _entry_new)
|
||||
if old_prefs:
|
||||
prefix_remap[path.resolve()] = old_prefs
|
||||
prefix_remap[path_abs] = old_prefs
|
||||
# Absolute form first: it is the longest, so prefix decomposition can
|
||||
# try forms in order without a shorter form shadowing it.
|
||||
stem_forms[path.resolve()] = (
|
||||
stem_forms[path_abs] = (
|
||||
new_id, [old_pref_abs, old_pref, new_id]
|
||||
)
|
||||
if id_remap:
|
||||
@@ -8166,7 +8181,7 @@ def extract(
|
||||
if n.get("type") == "package":
|
||||
continue
|
||||
try:
|
||||
entry = prefix_remap.get(Path(sf).resolve())
|
||||
entry = prefix_remap.get(_resolved(sf))
|
||||
except Exception:
|
||||
continue
|
||||
if entry is None:
|
||||
@@ -8272,7 +8287,7 @@ def extract(
|
||||
|
||||
def _decompose(target: str, tf: str) -> "tuple[str, str] | None":
|
||||
try:
|
||||
forms = stem_forms.get(Path(tf).resolve())
|
||||
forms = stem_forms.get(_resolved(tf))
|
||||
except (OSError, RuntimeError):
|
||||
return None
|
||||
if not forms:
|
||||
|
||||
+16
-5
@@ -16,6 +16,7 @@ flow) and every reader honours it.
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import functools
|
||||
import json
|
||||
import os
|
||||
import re
|
||||
@@ -253,6 +254,18 @@ def _is_test_path(path: str) -> bool:
|
||||
return False
|
||||
|
||||
|
||||
@functools.lru_cache(maxsize=65536)
|
||||
def _parent_parts(source_file: str) -> tuple[str, ...]:
|
||||
"""Segments of ``source_file``'s parent directory, POSIX-normalized.
|
||||
|
||||
`_path_proximity_winner` compares every candidate's directory against the
|
||||
call site's, and an ambiguous name (``run``, ``get``) brings the same
|
||||
candidate files back on every call site, so the parse is memoized. Equal
|
||||
tuples here mean equal ``PurePosixPath(...).parent`` values.
|
||||
"""
|
||||
return PurePosixPath(source_file.replace("\\", "/")).parent.parts
|
||||
|
||||
|
||||
def _path_proximity_winner(call_site_file: str, candidate_files: dict[str, str]) -> str | None:
|
||||
"""Pick the candidate whose source file is closest to the call site.
|
||||
|
||||
@@ -270,7 +283,7 @@ def _path_proximity_winner(call_site_file: str, candidate_files: dict[str, str])
|
||||
if not call_site_file:
|
||||
return None
|
||||
call_norm = str(call_site_file).replace("\\", "/")
|
||||
call_dir = PurePosixPath(call_norm).parent
|
||||
call_parts = _parent_parts(call_norm)
|
||||
|
||||
# Tier 1: exact same file.
|
||||
same_file = [cid for cid, f in candidate_files.items()
|
||||
@@ -282,7 +295,7 @@ def _path_proximity_winner(call_site_file: str, candidate_files: dict[str, str])
|
||||
|
||||
# Tier 2: same directory.
|
||||
same_dir = [cid for cid, f in candidate_files.items()
|
||||
if PurePosixPath(str(f).replace("\\", "/")).parent == call_dir]
|
||||
if _parent_parts(str(f)) == call_parts]
|
||||
if len(same_dir) == 1:
|
||||
return same_dir[0]
|
||||
if len(same_dir) > 1:
|
||||
@@ -290,10 +303,8 @@ def _path_proximity_winner(call_site_file: str, candidate_files: dict[str, str])
|
||||
|
||||
# Tier 3: longest common path-prefix, computed over path segments. The
|
||||
# winner must be a strict unique maximum, else we bail (guard holds).
|
||||
call_parts = call_dir.parts
|
||||
|
||||
def _common_prefix_len(f: str) -> int:
|
||||
parts = PurePosixPath(str(f).replace("\\", "/")).parent.parts
|
||||
parts = _parent_parts(str(f))
|
||||
n = 0
|
||||
for a, b in zip(call_parts, parts):
|
||||
if a != b:
|
||||
|
||||
@@ -0,0 +1,67 @@
|
||||
"""extract()'s id remap and the call tie-break parse each distinct path once.
|
||||
|
||||
The remap resolved the same file paths once per input, once per stamped edge
|
||||
and once per node, and `_path_proximity_winner` re-parsed every candidate's
|
||||
directory on every ambiguous call site. Both now memoize; answers are unchanged.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import sys
|
||||
from pathlib import Path, PurePosixPath
|
||||
|
||||
import pytest
|
||||
|
||||
from graphify import paths as paths_mod
|
||||
from graphify.extract import extract
|
||||
|
||||
|
||||
def _corpus(root: Path) -> list[Path]:
|
||||
(root / "pkg").mkdir(parents=True)
|
||||
files = []
|
||||
for i in range(4):
|
||||
body = "".join(f"def f{i}_{j}():\n return f{i}_{j + 1}()\n\n\n" for j in range(40))
|
||||
body += "".join(f"from pkg.m{k} import f{k}_0\n" for k in range(4) if k != i)
|
||||
p = root / "pkg" / f"m{i}.py"
|
||||
p.write_text(body, encoding="utf-8")
|
||||
files.append(p)
|
||||
(root / "pkg" / "__init__.py").write_text("", encoding="utf-8")
|
||||
return files + [root / "pkg" / "__init__.py"]
|
||||
|
||||
|
||||
def test_extract_resolves_each_distinct_path_once(tmp_path: Path, monkeypatch):
|
||||
files = _corpus(tmp_path / "proj")
|
||||
calls: list[str] = []
|
||||
real_resolve = Path.resolve
|
||||
remap_frames = {"extract", "_decompose", "_resolved"} # the id-remap code in extract()
|
||||
|
||||
def counting_resolve(self, *args, **kwargs):
|
||||
if sys._getframe(1).f_code.co_name in remap_frames:
|
||||
calls.append(str(self))
|
||||
return real_resolve(self, *args, **kwargs)
|
||||
|
||||
monkeypatch.setattr(Path, "resolve", counting_resolve)
|
||||
result = extract(files, root=tmp_path / "proj", cache_root=tmp_path / "cache", parallel=False)
|
||||
assert len(result["nodes"]) > 160 # every function became a node, so the per-node remap ran
|
||||
# Without the memo the remap resolved once per node and per stamped edge
|
||||
# (~200 calls here); with it, once per distinct path.
|
||||
assert len(calls) == len(set(calls)), f"{len(calls)} remap resolve calls for {len(set(calls))} paths"
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"site, candidates, expected",
|
||||
[
|
||||
("pkg/a/x.py", {"1": "pkg/a/y.py", "2": "pkg/b/y.py"}, "1"), # same dir
|
||||
("pkg\\a\\x.py", {"1": "pkg/a/y.py", "2": "pkg/b/y.py"}, "1"), # backslashes
|
||||
("pkg/a/b/x.py", {"1": "pkg/a/y.py", "2": "other/y.py"}, "1"), # longest prefix
|
||||
("pkg/a/x.py", {"1": "pkg/b/y.py", "2": "pkg/c/y.py"}, None), # tied prefix
|
||||
("x.py", {"1": "y.py", "2": "z/y.py"}, "1"), # root-level file
|
||||
("pkg/a/x.py", {"1": "pkg/a/x.py", "2": "pkg/a/y.py"}, "1"), # same file
|
||||
],
|
||||
)
|
||||
def test_proximity_winner_matches_a_fresh_parse(site, candidates, expected):
|
||||
paths_mod._parent_parts.cache_clear()
|
||||
assert paths_mod._path_proximity_winner(site, candidates) == expected
|
||||
# second call is served from the memo and must agree
|
||||
assert paths_mod._path_proximity_winner(site, candidates) == expected
|
||||
for f in candidates.values():
|
||||
assert paths_mod._parent_parts(f) == PurePosixPath(f.replace("\\", "/")).parent.parts
|
||||
Reference in new issue
Block a user