Call IsDirCoveredByWatch less often to improve performance#4582
Conversation
There was a problem hiding this comment.
Pull request overview
Optimizes watch-mode directory watch computation in internal/execute by reducing the number of expensive watchmanager.IsDirCoveredByWatch checks and avoiding repeatedly growing the directory map during iteration.
Changes:
- Deduplicate
seenFilePathsby directory so coverage checks run once per directory (not once per file). - Defer adding newly discovered non-recursive watch directories until after coverage checks to avoid accidental quadratic behavior from iterating an ever-growing map.
| toAddDirs := make(map[string]bool) // dir → should add | ||
| for _, filePath := range seenFilePaths { | ||
| dir := tspath.GetDirectoryPath(filePath) | ||
| if !watchmanager.IsDirCoveredByWatch(resolvedDirs, dir, opts) { | ||
| if watchmanager.CanWatchDirectory(dir) { | ||
| resolvedDirs[dir] = false | ||
| } | ||
| if _, has := toAddDirs[dir]; !has { | ||
| toAddDirs[dir] = !watchmanager.IsDirCoveredByWatch(resolvedDirs, dir, opts) && watchmanager.CanWatchDirectory(dir) |
|
I tested this PR with tsgo --build --watch in a private monorepo containing This PR optimizes internal/execute/watcher.go, but build mode computes I prepared a proof-of-concept branch that replaces repeated full-map scans https://github.com/AkisArou/typescript-go/tree/fix/watch-directory-coverage Commit: AkisArou@82eee7b4 The main changes are:
The core lookup walks only the candidate’s ancestors: func (s *DirectorySet) IsCovered(dir string) bool {
key := s.key(dir)
if _, ok := s.canonical[key]; ok {
return true
}
for parent := tspath.Path(tspath.GetDirectoryPath(string(key)));
rent != "" && parent != key; parent =
path.Path(tspath.GetDirectoryPath(string(parent))) {
if existing, ok := s.canonical[parent]; ok && existing.recursive {
return true
}
key = parent
}
return false
} This makes exact and recursive-ancestor checks O(path depth) in both regular A 5,000-directory comparison measured approximately 2.75 ms for the indexed I’m sharing this branch as additional data for the build-mode case and for |
|
This PR only changes the code path used by I chose not to modify the
|
I'm experiencing a significant performance penalty when using
tsc --watch. I'm seeingtsccomplete in ~2 seconds whiletsc --watchwill hang for ~15 seconds after reporting the first build status.Profiling shows a large amount of time being spent doing string manipulation as a result of calls to
watchmanager.IsDirCoveredByWatch. I noticed a couple of pieces of low hanging optimization fruit.watchmanager.IsDirCoveredByWatchwas being called once for each path inseenFilePaths, even if two paths were in the same directory. For my projectseenFilePathshad a length of around 40k files, but only 2k directories. This is where I was the most improvement for my project, reducing time spent in this part of the code from ~15s to ~200ms.resolvedDirsmap was (potentially) having new directories added to it each iteration of the loop. SinceIsDirCoveredByWatchiterates over that map to check if a directory is covered, this led to some accidentally quadratic time complexity. Since all directories being added to it are not recursive, they don't affect behavior and can be added after doing all the checks. This improved time spent in this part of the code from ~200ms to ~20ms.I can't easily share the project I'm testing with, so I also checked the performance difference when checking https://github.com/grafana/grafana
Before, using
@typescript/native-preview@7.0.0-dev.20260707.2(~30 seconds)After, with this PR (~1 second):
AI Assistance: I used Github Coilot Completions, and asked Claude Code to give the commit + PR description a sanity check.