Don't watch node_modules when using the FallbackWatcher #950
Conversation
|
Your assumptions are exactly right. I think adding
|
FallbackWatcher used in the language server was watching files in the node_modules and .git directories, which resulted in a V8 heap out of memory and unbearable performance when using the language server standalone, without VSCode (e.g. using neovim LSP).
|
@dummdidumm Thanks, I did not know about such a use of I have modified the code a bit. I could not come up with a regexp that would match The latest commit is still working well for me (neovim's LSP does not crash), but I did not explicitly test whether |
Do you know what exactly VSCode's built-in watcher does? The relevant code from svelte extension is here: And the vscode watcher implementation is here: https://github.com/microsoft/vscode/blob/fd88b56ab91ed7f6a43ae98a010fa098226db636/src/vs/workbench/api/common/extHostFileSystemEventService.ts But I haven't seen any indication that I'm wondering whether it ignores |
|
@rchl Unfortunately, I don't have any answer for you, as I don't know and did not check that |
|
VSCode has a |
Thanks! Windows: But then I wonder how this relates to the comment above as those patterns, I believe, ignore EDIT: here is a glob tester: https://www.digitalocean.com/community/tools/glob?comments=true&glob=%2A%2A%2Fnode_modules%2F%2A%2A&matches=false&tests=%2Fnode_modules%2Ftest.js&tests=%2Fsrc%2Fnode_modules%2Ftest.js&tests=%2Ffoo%2Ftest.js |
|
Good find. I guess that noone noticed this then yet. Since Sapper is deprecated and SvelteKit will not (ab)use node_modules anymore, it's not very relevant for much longer anyway. |
Looking at it a bit more... that setting seems to be related to I'm not entirely convinced that those exclude patterns are also used when creating file watcher programmatically like many extensions do. |
|
I dig through the vscode codebase. It has a super complex file watching system. And this is what I found:
And the The WorkspaceWatcher also received As for the actual file watching, It seems to have multiple implementations in this watcher directory. VSCode also uses chokidar in some cases. I think we can also reference it to optimize our fallback watcher. |
FallbackWatcherused in the language server was watching files in thenode_modulesand.gitdirectories, which resulted in a V8 heap out of memory and unbearable performance when using the language server standalone, without VSCode (e.g. using neovim LSP).Investigation
When using the svelte language server in neovim LSP I have noticed very poor performance (when asking for a hover message, I got it after a minute, with the language server crashing right after that).
I checked in
~/.cache/nvim/lsp.logthat the language server was initializing a new TS server for eachtsconfig.jsonit found innode_modules:Logs with a V8 crash at the end
Some debugging later, I found out that I was using the
FallbackWatcher:https://github.com/Gelio/language-tools/blob/597cafeaebb30348b787b52bdac61e17ae34f674/packages/language-server/src/server.ts#L103-L105
which in turn used
chokidarto watch the whole workspace. AFAIK chokidar does not ignore any directories by default, so it rightfully went over all ofnode_modules, which ended up initializing multiple unnecessary TS servershttps://github.com/Gelio/language-tools/blob/597cafeaebb30348b787b52bdac61e17ae34f674/packages/language-server/src/plugins/typescript/service.ts#L55
With this change, the logs are more reasonable and I am able to use the language server, with the only TS server being initialized for my own project.
Logs
I assume the problem was not found before since the
FallbackWatcherwas not used extensively, as I assume when using the language server from within VSCode, it leverages VSCode's watcher, which ignored directories such asnode_modules,.git, etc.