Skip to content

perf(cascade): relativise watcher paths without a resolve() per event - #459

Merged
0xKT merged 3 commits into
mainfrom
fix/watcher-relative-path-without-resolve
Sep 24, 2026
Merged

0xKT merged 3 commits into
mainfrom
fix/watcher-relative-path-without-resolve

Conversation

@gloryfromca

@gloryfromca gloryfromca commented Sep 24, 2026 •

Copy link
Copy Markdown
Member

Summary

_relative_to_root in the cascade watcher called Path(raw).resolve() on every watchdog event (create / modify / move / delete) to defend against an external symlink surfacing inside the tree. watchdog already hands us absolute paths inside the watched root, so in the common case that is a filesystem round trip per path component for nothing — on Windows a GetFinalPathNameByHandle call per component, each through Defender.

The textual relative_to now runs first; resolve() is kept only for a path that is not under the root as written, or that carries .. segments. Same results for the symlink case (still maps) and for outside paths (still None).

Evidence

py-spy, 25 s, the soak server under raised write load (three feeders ≈ 30 md rewrites/s + two cascade sync storms + a search storm, PR #454's box): of 1 082 worker-thread samples, 435 (40 %) were in realpath / _getfinalpathname_nonstrict / stat, and 403 of those (93 %) came from _relative_to_root via _Handler._enqueue (on_modified 221, on_created 72, on_moved 91, on_deleted 19). The Python-level ntpath/pathlib frames around those calls hold the GIL; the Win32 calls themselves release it.

Area

  • performance (cascade / Windows)

Verification

  • tests/unit/test_memory/test_cascade: 221 passed.
  • New test_relative_to_root_within_does_not_touch_the_filesystem: Path.resolve patched to raise; red without the fast path (AssertionError: resolve() must not run for an in-root path), green with it.
  • New test_relative_to_root_via_symlink_still_resolves (POSIX): a path reaching the root through an external symlink still maps to the same relative path.
  • make lint green.

Checklist

  • tests added, mutation checked
  • behaviour unchanged for symlink / outside / .. inputs

Notes for Reviewers

root is MemoryRoot.root as configured; if it is itself given through a symlink or a different case than watchdog reports, the textual relative_to fails and the old resolve() path runs — so the fallback is exercised, not lost. Windows PureWindowsPath.relative_to compares case-insensitively.

🤖 Generated with Claude Code

zhanghui and others added 3 commits September 24, 2026 13:24
`_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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
@arelchan
arelchan self-requested a review September 24, 2026 08:32
@0xKT
0xKT merged commit 46b63c3 into main Sep 24, 2026
10 checks passed
@0xKT
0xKT deleted the fix/watcher-relative-path-without-resolve branch September 24, 2026 08:33
This was referenced Sep 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants