From d5966eda8abb47d12d93e047daf79e23d500d659 Mon Sep 17 00:00:00 2001 From: rohit-jsfreaky Date: Wed, 7 Oct 2026 17:15:00 +0530 Subject: [PATCH] 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 --- graphify/extract.py | 33 +++++++++++----- graphify/paths.py | 21 ++++++++--- tests/test_extract_path_memo.py | 67 +++++++++++++++++++++++++++++++++ 3 files changed, 107 insertions(+), 14 deletions(-) create mode 100644 tests/test_extract_path_memo.py diff --git a/graphify/extract.py b/graphify/extract.py index db4fd1bad6..e1d1746952 100644 --- a/graphify/extract.py +++ b/graphify/extract.py @@ -7993,10 +7993,24 @@ def _portable_out_of_root_sf(p: Path) -> str: # 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 _portable_out_of_root_sf(p: Path) -> str: 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 _portable_out_of_root_sf(p: Path) -> str: 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 _portable_out_of_root_sf(p: Path) -> str: # 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 _portable_out_of_root_sf(p: Path) -> str: 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 _portable_out_of_root_sf(p: Path) -> str: 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 _edge_key(edge: dict) -> str: 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: diff --git a/graphify/paths.py b/graphify/paths.py index 24bb4751f7..1bd1a613e9 100644 --- a/graphify/paths.py +++ b/graphify/paths.py @@ -16,6 +16,7 @@ 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: diff --git a/tests/test_extract_path_memo.py b/tests/test_extract_path_memo.py new file mode 100644 index 0000000000..f14c5e23ba --- /dev/null +++ b/tests/test_extract_path_memo.py @@ -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