From cce13fad205d93f444dd0afea5f2a5cc55a5be15 Mon Sep 17 00:00:00 2001 From: abhay-codes07 Date: Sun, 13 Sep 2026 23:07:04 +0530 Subject: [PATCH] perf(cache): memoize source-path (de)normalization per distinct string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both directions of the cache source_file round-trip did per-ITEM path work, though every node in a file's payload — and most of its edges/raw_calls — carries the same one or two source strings, and each mapping is a pure function of (source, root): - _relativize_source_files_in (save): an os.path.abspath, an on-disk exists() stat, and an os.path.relpath per item -> ~2000ms to ~64ms on an 800-item payload sharing one source_file (~31x), almost all of it eliminated stats. - _absolutize_source_files_in (warm load / `graphify update`): a Path build + join + str per item -> ~1150ms to ~65ms (~18x). Each now computes once per distinct string per call. Output is byte-identical (verified against the pre-memo implementation across relative / absolute / out-of-root / missing / duplicate paths, and via a relativize->absolutize round-trip). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01JJbLfztSxm2tH5cJwBbe9q --- graphify/cache.py | 81 +++++++++++----- tests/test_relativize_source_memo.py | 138 +++++++++++++++++++++++++++ 2 files changed, 194 insertions(+), 25 deletions(-) create mode 100644 tests/test_relativize_source_memo.py diff --git a/graphify/cache.py b/graphify/cache.py index a70cff03dc..6926e236ba 100644 --- a/graphify/cache.py +++ b/graphify/cache.py @@ -610,6 +610,37 @@ def _relativize_source_files_in(payload: dict, root: Path) -> None: # definition_file (#2990) is a path into the scanned tree exactly like # source_file; a cache entry keeping it absolute replayed the build host's # layout on every warm hit (#3223). + # Every node in a file's payload — and most of its edges/raw_calls — carries + # the SAME one or two source-path strings (the file being extracted, plus a + # handful of import target files). Relativizing is a pure function of + # (source string, root), yet it was recomputed per item: an os.path.abspath, + # an on-disk exists() stat, and an os.path.relpath for every one of a file's + # hundreds of nodes. Memoize the mapping per distinct source string for this + # call, so the path work (and the stat) runs once per distinct path rather + # than once per item. Output is byte-identical — the value depends only on + # the string and the fixed root. ``None`` records "leave unchanged". + rel_cache: dict[str, "str | None"] = {} + + def _relativized(source: str) -> "str | None": + sp = Path(source) + if not sp.is_absolute(): + # os.path.abspath is lexical (no symlink resolution), + # matching the symbolic relativization below. + cwd_form = Path(os.path.abspath(sp)) + try: + if cwd_form == root_resolved / sp or not cwd_form.exists(): + return None # already root-relative, or a ghost path + except OSError: + return None + sp = cwd_form + try: + rel = os.path.relpath(sp, root_resolved) + except (ValueError, OSError): + return None # out-of-root (e.g. Windows cross-drive) + if rel == ".." or rel.startswith(".." + os.sep) or rel.startswith("../"): + return None # escaped root — keep absolute + return rel.replace(os.sep, "/") + for bucket in ("nodes", "edges", "hyperedges", "raw_calls"): for item in payload.get(bucket, []): if not isinstance(item, dict): @@ -618,24 +649,11 @@ def _relativize_source_files_in(payload: dict, root: Path) -> None: source = item.get(key) if not source: continue - sp = Path(source) - if not sp.is_absolute(): - # os.path.abspath is lexical (no symlink resolution), - # matching the symbolic relativization below. - cwd_form = Path(os.path.abspath(sp)) - try: - if cwd_form == root_resolved / sp or not cwd_form.exists(): - continue # already root-relative, or a ghost path - except OSError: - continue - sp = cwd_form - try: - rel = os.path.relpath(sp, root_resolved) - except (ValueError, OSError): - continue # out-of-root (e.g. Windows cross-drive) - if rel == ".." or rel.startswith(".." + os.sep) or rel.startswith("../"): - continue # escaped root — keep absolute - item[key] = rel.replace(os.sep, "/") + if source not in rel_cache: + rel_cache[source] = _relativized(source) + rel = rel_cache[source] + if rel is not None: + item[key] = rel def _normalize_source_file_value(src: "str | Path", root_resolved: Path) -> str: @@ -912,6 +930,21 @@ def _absolutize_source_files_in(payload: dict, root: Path) -> None: root_resolved = Path(root).resolve() except OSError: return + # As in _relativize_source_files_in: a file's whole payload shares one or two + # source strings, and the re-anchoring is a pure function of (source, root). + # Memoize per distinct string so the Path construction and join run once per + # path rather than once per item — this is the warm/`graphify update` path. + abs_cache: dict[str, "str | None"] = {} + + def _absolutized(source: str) -> "str | None": + sp = Path(source) + if sp.is_absolute(): + return None # legacy absolute entry — leave unchanged + try: + return str(root_resolved / sp) + except (TypeError, OSError): + return None + for bucket in ("nodes", "edges", "hyperedges", "raw_calls"): for item in payload.get(bucket, []): if not isinstance(item, dict): @@ -921,13 +954,11 @@ def _absolutize_source_files_in(payload: dict, root: Path) -> None: source = item.get(key) if not source: continue - sp = Path(source) - if sp.is_absolute(): - continue - try: - item[key] = str(root_resolved / sp) - except (TypeError, OSError): - continue + if source not in abs_cache: + abs_cache[source] = _absolutized(source) + new = abs_cache[source] + if new is not None: + item[key] = new def cache_dir(root: Path = Path("."), kind: str = "ast", diff --git a/tests/test_relativize_source_memo.py b/tests/test_relativize_source_memo.py new file mode 100644 index 0000000000..e61752bdda --- /dev/null +++ b/tests/test_relativize_source_memo.py @@ -0,0 +1,138 @@ +"""_relativize_source_files_in memoizes per distinct source path (#perf). + +Every node in a file's cache payload — and most of its edges/raw_calls — carries +the same one or two source-path strings, yet each item triggered an +os.path.abspath, an on-disk exists() stat, and an os.path.relpath. The result +is a pure function of (source string, root), so it is now computed once per +distinct string: same output, a fraction of the path work and stats. +""" + +import os +from pathlib import Path + +from graphify.cache import ( + _absolutize_source_files_in, + _relativize_source_files_in, +) + + +def _reference(payload, root): + """The pre-memo implementation, verbatim, as an equivalence oracle.""" + root_resolved = Path(root).resolve() + for bucket in ("nodes", "edges", "hyperedges", "raw_calls"): + for item in payload.get(bucket, []): + if not isinstance(item, dict): + continue + for key in ("source_file", "definition_file"): + source = item.get(key) + if not source: + continue + sp = Path(source) + if not sp.is_absolute(): + cwd_form = Path(os.path.abspath(sp)) + try: + if cwd_form == root_resolved / sp or not cwd_form.exists(): + continue + except OSError: + continue + sp = cwd_form + try: + rel = os.path.relpath(sp, root_resolved) + except (ValueError, OSError): + continue + if rel == ".." or rel.startswith(".." + os.sep) or rel.startswith("../"): + continue + item[key] = rel.replace(os.sep, "/") + + +def _corpus(root): + (root / "src").mkdir(parents=True, exist_ok=True) + (root / "src" / "m.py").write_text("x=1\n", encoding="utf-8") + (root / "a.py").write_text("y=1\n", encoding="utf-8") + outside = str(root.parent / "other.py") + return { + "nodes": [ + {"id": "1", "source_file": str(root / "src" / "m.py"), + "definition_file": str(root / "a.py")}, + {"id": "2", "source_file": "src/m.py"}, # already relative + {"id": "3", "source_file": outside}, # out-of-root + {"id": "4", "source_file": str(root / "ghost.py")}, # missing (abs) + {"id": "5"}, # no source_file + {"id": "6", "source_file": str(root / "src" / "m.py")}, # dup + ], + "edges": [{"source": "1", "target": "2", + "source_file": str(root / "a.py")}], + "hyperedges": [], + "raw_calls": [{"caller_nid": "1", + "source_file": str(root / "src" / "m.py")}], + } + + +def test_output_matches_the_unmemoized_reference(tmp_path): + root = tmp_path.resolve() + got = _corpus(root) + _relativize_source_files_in(got, root) + expected = _corpus(root) + _reference(expected, root) + assert got == expected + + +def test_stats_each_distinct_path_once(tmp_path, monkeypatch): + """A file's hundreds of same-source nodes must not each stat the disk.""" + root = tmp_path.resolve() + (root / "src").mkdir(parents=True) + (root / "src" / "m.py").write_text("x=1\n", encoding="utf-8") + # 500 nodes, all the SAME relative source string (the only path needing a + # stat — an absolute in-root path skips the exists() check). + payload = {"nodes": [{"id": str(i), "source_file": "src/m.py"} + for i in range(500)], + "edges": [], "hyperedges": [], "raw_calls": []} + + calls = {"n": 0} + real_exists = Path.exists + + def counting_exists(self): + calls["n"] += 1 + return real_exists(self) + + monkeypatch.setattr(Path, "exists", counting_exists) + _relativize_source_files_in(payload, root) + # One distinct source string -> at most one exists() probe, not 500. + assert calls["n"] <= 1, calls["n"] + # And every node was still rewritten identically. + assert all(n["source_file"] == "src/m.py" for n in payload["nodes"]) + + +def test_duplicate_source_strings_all_rewritten(tmp_path): + root = tmp_path.resolve() + (root / "m.py").write_text("x=1\n", encoding="utf-8") + abs_sf = str(root / "m.py") + payload = {"nodes": [{"id": str(i), "source_file": abs_sf} for i in range(10)], + "edges": [], "hyperedges": [], "raw_calls": []} + _relativize_source_files_in(payload, root) + assert all(n["source_file"] == "m.py" for n in payload["nodes"]) + + +def test_absolutize_round_trips_relativize(tmp_path): + """relativize then absolutize recovers the original absolute source_file, + across many items sharing one string (the warm-load / update path).""" + root = tmp_path.resolve() + (root / "src").mkdir(parents=True) + (root / "src" / "m.py").write_text("x=1\n", encoding="utf-8") + orig = str(root / "src" / "m.py") + payload = {"nodes": [{"id": str(i), "source_file": orig} for i in range(50)], + "edges": [], "hyperedges": [], "raw_calls": []} + _relativize_source_files_in(payload, root) + assert all(n["source_file"] == "src/m.py" for n in payload["nodes"]) + _absolutize_source_files_in(payload, root) + assert all(n["source_file"] == orig for n in payload["nodes"]) + + +def test_absolutize_leaves_legacy_absolute_entries(tmp_path): + """A legacy cache entry storing an absolute source_file is left unchanged.""" + root = tmp_path.resolve() + abs_sf = str(root / "already" / "abs.py") + payload = {"nodes": [{"id": "1", "source_file": abs_sf}], + "edges": [], "hyperedges": [], "raw_calls": []} + _absolutize_source_files_in(payload, root) + assert payload["nodes"][0]["source_file"] == abs_sf