Watch with way fewer globs in LSP #971
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR reduces the number of file watchers set up by fixing the root file globs and updating the logic for gathering failed lookup locations.
- Introduces a new glob mapping function (createGlobMapper) for consolidating failed lookup directories with proper normalization
- Updates the initialization of watched files in project.go to use the new glob mapper and NormalizePath for consistency
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| internal/project/watch.go | Adds the glob mapper function and refactors directory lookup logic |
| internal/project/project.go | Modifies watcher initialization to use the new glob mapper |
| packageDirPath *tspath.Path | ||
| } | ||
|
|
||
| func getDirectoryToWatchFailedLookupLocation( |
There was a problem hiding this comment.
This is a pure port, basically line for line. I made no attempts to make it go any faster.
|
thank you for this. my machine was absolutely crunching. |
| isRootWatchable bool, | ||
| currentDirectory string, | ||
| preferNonRecursiveWatch bool, | ||
| ) *directoryOfFailedLookupWatch { |
There was a problem hiding this comment.
Strada has baselines on what to watch : https://github.com/microsoft/TypeScript/blob/main/tests/baselines/reference/canWatch/getDirectoryToWatchFailedLookupLocationIndirDos.baseline.md like these. do we want these atleast till we have this logic and dont make this any faster.
There was a problem hiding this comment.
We can do that, though I'm wary of continuing to have these sorts of super large baselines.
| } | ||
|
|
||
| globs := make([]string, 0, len(globSet)) | ||
| for dir, recursive := range globSet { |
There was a problem hiding this comment.
In strada there is code to watch "packageDir" if it exits and dir otherwise so we can handle the symlinks .. a note here to handle package symlinks later would be good
There was a problem hiding this comment.
Ah, I should have added that, yeah. Will follow up with that.
There was a problem hiding this comment.
I was dogfooding tsgo so couldn't find all refs on it; is it just packageDir ?? dir?
There was a problem hiding this comment.
its packageDir if packageDir exists and is symlink otherwise dir. Its in createDirectoryWatcherForPackageDir in strada. Its little complicated with having to manage state but since you dont need to, it should make it easier.
Fixes #956
Before, opening the VS Code source would net a super large number of file watchers:
With this PR, this is much reduced:
And, the root file globs have been fixed (though it's possible VS Code was normalizing them).
This is not particularly fast; for VS Code it takes some 60ms to process all of the failed lookup locations, but it doesn't change often. Mainly, this is to make things better than they were.