Remove redundant work from polling watcher#3677
Merged
andrewbranch merged 3 commits intomicrosoft:mainfrom May 1, 2026
Merged
Conversation
…nterval to 2s snapshotPaths already walks the full recursive wildcard directory tree via fs.WalkDir and stores every visited directory in watchState with a ChildrenHash (covering both files and subdirectories). The hasChanges baseline loop then re-hashes every entry with ChildrenHash != 0, which already detects any addition, deletion, or rename under any watched directory. The old hasChanges had a second fs.WalkDir pass over the same wildcard trees that called GetAccessibleEntries on every directory a second time. This was completely redundant: all directories are already in watchState from snapshotPaths, and the baseline loop covers them. Measured on vscode/extensions/copilot (7092 watched paths, 1145 dirs, 3 recursive wildcard dirs covering 1024 subdirectories) on M5 Pro / macOS: Before: ~350ms/scan at 1s interval ≈ 35% idle CPU After: ~145ms/scan at 1s interval ≈ 14.5% idle CPU CPU profile before fix: WalkDir cumulative = 3.65s out of 6.38s in hasChanges over 48 seconds (57%). Also bump the default poll interval from 1000ms to 2000ms. The extra latency is imperceptible for a watch workflow and halves the remaining idle CPU: ~145ms/scan at 2s interval ≈ 7% idle CPU Also add TSGO_WATCH_DEBUG=1 env var support (via sys.GetEnvironmentVariable) that logs per-scan timing to the output writer for future diagnosis. Add a regression test (TestHasChangesNoRedundantGetAccessibleEntries) that counts GetAccessibleEntries calls through a wrapping FS adapter and asserts each tracked directory is hashed exactly once per poll, not twice. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
watchState contains every path the compiler accessed during a build (via trackingvfs). Many of those paths happen to be directories (DirectoryExists, Stat, GetAccessibleEntries calls during module resolution), but for those explicit-path entries the watcher only needs to know whether the path itself exists and what its mtime is — it does NOT depend on what's inside. Previously snapshotPaths stored a ChildrenHash for every directory in the explicit paths list, causing hasChanges to readdir each one on every poll even when nothing depended on its listing. The wildcard-tree branch already adds the directories that DO need listing tracking, via snapshotDirEntry. Measured on vscode/extensions/copilot (M5 Pro, macOS), poll scan time: Previous commit: ~250ms (1855 dirs hashed) This commit: ~165ms (1024 dirs hashed) The 831 eliminated hashes were directories accessed during compilation (mostly node_modules paths) whose listings were never load-bearing for change detection. Strengthen the regression test to assert that an explicit-paths directory that is NOT in any wildcard tree gets only a Stat, never a readdir. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jakebailey
reviewed
May 1, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR reduces redundant filesystem work in the polling-based watcher to lower idle CPU usage in tsgo --watch, primarily by avoiding directory listing/hashing work when only existence/mtime tracking is needed, and by simplifying change detection to iterate a single baseline. It also adds optional scan timing diagnostics for watch-mode debugging and adjusts the default polling interval.
Changes:
- Avoid
GetAccessibleEntries/children hashing for directories that are only present due to explicit path lookups (non-wildcard). - Simplify
hasChangesto compare against the baseline only (no second iteration over wildcard directories). - Add optional per-scan timing logging (gated by
TSGO_WATCH_DEBUG) and increase the default watch interval to 2s.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| internal/vfs/vfswatch/vfswatch.go | Removes redundant directory hashing for explicit paths, simplifies change detection, and adds optional scan timing debug logging. |
| internal/vfs/vfswatch/vfswatch_test.go | Adds a regression test ensuring GetAccessibleEntries is only called for wildcard-tree directories. |
| internal/execute/watcher.go | Enables vfswatch scan timing logs when TSGO_WATCH_DEBUG is set. |
| internal/core/watchoptions.go | Changes the default watch polling interval from 1s to 2s. |
johnfav03
approved these changes
May 1, 2026
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
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.
Two issues:
pathsseen by the compiler host were being read and children hashed, but for those, we only need to stat them for existence. We only need to compute a hash of a directory's children when it's part of a tsconfig include pattern. Directories in thepathsportion of the watch state are triggered by module resolution lookups, and we only need to track their existence so we can tell if, e.g., a node_modules package directory that would have had individual file lookups inside it gets created. (We no longer store those would-be file lookups as paths to check after Delete failed lookup locations #3038; checking the directory is sufficient.)hasChangeswas iterating both the previous watch state baseline as well as the current set of wildcard directories, but the state baseline already includes the results of scanning the wildcard directories, so just iterating the baseline is sufficient.On top of that, I doubled the polling interval from 1 second to 2 seconds, in line with
esbuild --watch's worst-case latency.Together, these changes bring
tsgo --watchonvscode/extensions/copilotdown from 20% idle CPU to 4%.We could squeeze a little more out of polling by checking recently-changed files more often and rarely changed files less often, but we're also exploring some non-polling options that seem more promising. It's hard to accept a high-latency solution at 4% CPU that scales linearly in latency or CPU utilization with project size, when you can plausibly have a low-latency solution at 0% idle CPU that scales quite well.
Related: #3611