From 83fa987985d76fb865463ec0ae41c03451032d1b Mon Sep 17 00:00:00 2001 From: zhanghui Date: Thu, 24 Sep 2026 13:24:23 +0800 Subject: [PATCH 1/2] perf(cascade): relativise watcher paths without a resolve() per event MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_relative_to_root` called `Path(raw).resolve()` on every watchdog event to defend against an external symlink surfacing inside the tree. watchdog already reports absolute paths inside the watched root, so for the common case that is a filesystem round trip per path component for nothing — on Windows a `GetFinalPathNameByHandle` call that goes through Defender. py-spy on the 2-hour soak under raised write load: about 40 % of the worker threads' samples sat in `realpath` / `_getfinalpathname` / `stat` under this one function, GIL held, while the event loop waited. The textual `relative_to` now comes first; `resolve()` runs only when the path is not under the root as written or carries `..` segments, so the symlink case still maps and outside paths still return None. Two tests: an in-root path is relativised with `Path.resolve` patched to raise, and a path through an external symlink still resolves inside. Co-Authored-By: Claude Fable 5.1 --- src/everos/memory/cascade/watcher.py | 22 +++++++++--- .../test_cascade/test_watcher_helpers.py | 35 +++++++++++++++++++ 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/src/everos/memory/cascade/watcher.py b/src/everos/memory/cascade/watcher.py index 9c30032b9..ef0eb3db9 100644 --- a/src/everos/memory/cascade/watcher.py +++ b/src/everos/memory/cascade/watcher.py @@ -152,15 +152,27 @@ async def _enqueue_async( def _relative_to_root(root: Path, raw: str) -> str | None: """Return ``raw`` relative to ``root`` using POSIX separators. - ``None`` when the path is outside the memory root (defensive — the - watcher only watches inside ``root``, but external symlinks could - surface). + watchdog reports absolute paths inside the watched root, so the common + case is a pure string operation. ``resolve()`` is a filesystem round + trip per path component — on Windows a ``GetFinalPathNameByHandle`` + call that goes through Defender — and doing it on every event was about + 40 % of the worker threads' CPU under write load (py-spy on the 2 h + soak). It is kept for the defensive case only: a textual path that is + not under ``root`` (an external symlink surfacing inside the tree) or + one carrying ``..`` segments. + + ``None`` when the path is outside the memory root even after resolving. """ + path = Path(raw) + if ".." not in path.parts: + try: + return path.relative_to(root).as_posix() + except ValueError: + pass try: - rel = Path(raw).resolve().relative_to(root) + return path.resolve().relative_to(root).as_posix() except ValueError: return None - return rel.as_posix() def _safe_mtime(raw: str) -> float: diff --git a/tests/unit/test_memory/test_cascade/test_watcher_helpers.py b/tests/unit/test_memory/test_cascade/test_watcher_helpers.py index 772e247ed..e2dfad6bd 100644 --- a/tests/unit/test_memory/test_cascade/test_watcher_helpers.py +++ b/tests/unit/test_memory/test_cascade/test_watcher_helpers.py @@ -7,8 +7,11 @@ from __future__ import annotations +import sys from pathlib import Path +import pytest + from everos.memory.cascade.watcher import _relative_to_root, _safe_mtime @@ -34,3 +37,35 @@ def test_safe_mtime_existing_path_returns_positive(tmp_path: Path) -> None: f = tmp_path / "f.md" f.write_text("ok") assert _safe_mtime(str(f)) > 0 + + +def test_relative_to_root_within_does_not_touch_the_filesystem( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """An in-root path is relativised textually — no ``resolve()``. + + ``resolve()`` is a filesystem round trip per component (through + Defender on Windows) and the watcher calls this on every event. + """ + + def _boom(self: Path, strict: bool = False) -> Path: + raise AssertionError("resolve() must not run for an in-root path") + + monkeypatch.setattr(Path, "resolve", _boom) + target = tmp_path / "users" / "u1" / "x.md" # need not exist + assert _relative_to_root(tmp_path, str(target)) == "users/u1/x.md" + + +@pytest.mark.skipif(sys.platform == "win32", reason="symlinks need a privilege") +def test_relative_to_root_via_symlink_still_resolves( + tmp_path: Path, tmp_path_factory: pytest.TempPathFactory +) -> None: + """A path that reaches the root through an external symlink still maps.""" + root = tmp_path.resolve() + (root / "users" / "u1").mkdir(parents=True) + (root / "users" / "u1" / "x.md").write_text("x", encoding="utf-8") + link = tmp_path_factory.mktemp("elsewhere") / "link_to_root" + link.symlink_to(root, target_is_directory=True) + assert ( + _relative_to_root(root, str(link / "users" / "u1" / "x.md")) == "users/u1/x.md" + ) From da15afa5e50e8abba5646e68612ad0c4fee0cb45 Mon Sep 17 00:00:00 2001 From: zhanghui Date: Thu, 24 Sep 2026 13:47:53 +0800 Subject: [PATCH 2/2] perf(cascade): pin the dotdot fallback and state the real per-event cost Review of the previous commit: the `..` guard was untested (a copy without it passed the whole file), and the docstring described the cost as "a round trip per path component" when on an existing file it is two GetFinalPathNameByHandle opens plus a stat(); the per-component walk only happens for paths that are already gone. One test pins the guard; the docstring says what the soak profile actually showed and names the 8.3 short-name residue the scanner's sweep reconciles. Co-Authored-By: Claude Fable 5.1 --- src/everos/memory/cascade/watcher.py | 13 ++++++++----- .../test_cascade/test_watcher_helpers.py | 8 ++++++++ 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/everos/memory/cascade/watcher.py b/src/everos/memory/cascade/watcher.py index ef0eb3db9..b4a406f5e 100644 --- a/src/everos/memory/cascade/watcher.py +++ b/src/everos/memory/cascade/watcher.py @@ -152,14 +152,17 @@ async def _enqueue_async( def _relative_to_root(root: Path, raw: str) -> str | None: """Return ``raw`` relative to ``root`` using POSIX separators. - watchdog reports absolute paths inside the watched root, so the common - case is a pure string operation. ``resolve()`` is a filesystem round - trip per path component — on Windows a ``GetFinalPathNameByHandle`` - call that goes through Defender — and doing it on every event was about + watchdog reports absolute paths inside the watched root (``root`` is + already resolved by ``MemoryRoot``), so the common case is a pure string + operation and matches the scanner's key byte for byte. ``resolve()`` on + Windows is two ``GetFinalPathNameByHandle`` opens plus a ``stat()`` per + event, and a per-component walk when the path is already gone — about 40 % of the worker threads' CPU under write load (py-spy on the 2 h soak). It is kept for the defensive case only: a textual path that is not under ``root`` (an external symlink surfacing inside the tree) or - one carrying ``..`` segments. + one carrying ``..`` segments. Known residue: if ``ReadDirectoryChangesW`` + reports a file by its 8.3 short name, the key differs from the + scanner's long-name key until the next sweep reconciles it. ``None`` when the path is outside the memory root even after resolving. """ diff --git a/tests/unit/test_memory/test_cascade/test_watcher_helpers.py b/tests/unit/test_memory/test_cascade/test_watcher_helpers.py index e2dfad6bd..22faa078c 100644 --- a/tests/unit/test_memory/test_cascade/test_watcher_helpers.py +++ b/tests/unit/test_memory/test_cascade/test_watcher_helpers.py @@ -69,3 +69,11 @@ def test_relative_to_root_via_symlink_still_resolves( assert ( _relative_to_root(root, str(link / "users" / "u1" / "x.md")) == "users/u1/x.md" ) + + +def test_relative_to_root_normalises_dotdot_segments(tmp_path: Path) -> None: + """A ``..`` segment takes the resolving path so the key stays canonical.""" + root = tmp_path.resolve() + (root / "users" / "u1").mkdir(parents=True) + raw = str(root / "users" / "u2" / ".." / "u1" / "x.md") + assert _relative_to_root(root, raw) == "users/u1/x.md"