fix(runtime): cache canonicalized watch paths, fix spurious events#33123
Open
bartlomieju wants to merge 2 commits intomainfrom
Open
fix(runtime): cache canonicalized watch paths, fix spurious events#33123bartlomieju wants to merge 2 commits intomainfrom
bartlomieju wants to merge 2 commits intomainfrom
Conversation
The is_file_removed fallback added in #27041 had no path filtering, causing any file removal (e.g. temp files during Vim atomic saves) to emit events to all watchers regardless of watched paths. This made Vite's HMR unusable under Deno >= 2.1.2 with editors that do atomic saves. Add path filtering to the remove-detection branch by checking the removed file's parent directory against watched paths. Fixes #27558 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Pre-canonicalize watched paths once at registration time instead of re-canonicalizing both paths on every filesystem event. Previously, starts_with_canonicalized called canonicalize() on both paths for every event x sender x watched path, causing redundant stat syscalls. Also fixes #27558 by adding path filtering to the removed-file fallback, which previously forwarded events for any removed file to all watchers regardless of path. This caused editors with atomic saves (Vim, WebStorm) to trigger full Vite HMR reloads. Skip watchFsRemove test on macOS where FSEvents does not reliably emit remove events for individually watched files (previously passed only due to the unfiltered fallback bug). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
bartlomieju
force-pushed
the
perf/cache-canonicalized-watch-paths
branch
from
fc308e7 to
a76a220
Compare
bartlomieju
changed the title
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
op_fs_events_opentime instead of callingcanonicalize()on both the event path and the watched path on every filesystem event for every watcherstarts_with_canonicalizeddid 2canonicalize()calls (each resolving all symlinks and stat-ing every path component) per event x sender x watched path -- hundreds of redundant syscalls/sec under loadWatchSenderstruct; only the event path is canonicalized (once per event per sender)watchFsRemovetest on macOS where FSEvents does not reliably emit remove events for individually watched files (previously passed only due to the unfiltered fallback bug)Fixes #27558
Supersedes #33120
Test plan
./x test-unit fs_eventspassescargo clippy -p deno_runtime -- -D warningsclean./tools/format.jscleanwatchFsNoSpuriousEventsFromSiblingDirverifies no spurious events from sibling dirs🤖 Generated with Claude Code