Exclude **/node_modules/** from .vscode/launch.json searches

[gluxon]

This is a strange PR and I would have no qualms with it being rejected.

Problem

Excluding **/node_modules/** from vscode.workspace.findFiles(...) calls is somewhat necessary to prevent excessive CPU usage when this extension is installed and users open a large frontend project in VS Code. findFiles searches the entire repo by default, even .gitignore files.

My teammates are running into this excessive CPU usage problem as they jump between frontend repos and Java repos in VS Code.

Workaround

One comment from a VS Code member recommends extensions exclude node_modules since a proper fix to the findFiles API would be a breaking change: microsoft/vscode#75314 (comment)

This seems to be somewhat standard, even in extensions that have nothing to do with Node.js. For example:

• Searching workspace.findFiles in the microsoft/vscode repo shows almost every usage excluding **/node_modules/** explicitly. https://github.com/microsoft/vscode/search?q=workspace.findFiles
• This is the case for even the stock Markdown extension: https://github.com/microsoft/vscode/blob/c62f59fcae4d9fd029b542639f3ef58bd698f982/extensions/markdown-language-features/src/features/workspaceSymbolProvider.ts#L40

I've also PR'ed a similar change in other repos:

• Exclude **/node_modules/** from Chart.yaml searches vscode-kubernetes-tools/vscode-kubernetes-tools#1041

Would this be reasonable in vscode-java-debug as well?

Risks

The docs on workspace.findFiles(...) say an undefined 2nd argument defaults to the user's files.exclude setting. Changing this argument to **/node_modules/** means this search no longer excludes the user configured files.exclude. If that's a concern, I think we can concatenate **/node_modules/** after looking up the files.exclude setting.

[testforstephen]

@gluxon this is a good finding, thanks for bringing it up for our awareness.

[testforstephen]

I'm OK with excluding both node_modules and .git.

[gluxon]

Thanks! So to be clear, updating this argument **/node_modules/** to {**/node_modules/**,.git/**} would be ideal?

I'll test that out to make sure I have the right glob syntax.

[testforstephen]

It would be interesting if there is some performance comparison data to share.

[gluxon]

It looks like the 2nd argument appends rather than replaces. The existing command in this PR:

await vscode.workspace.findFiles(".vscode/launch.json", "**/node_modules/**");

runs the following process:

'/Applications/Visual Studio Code.app/Contents/Resources/app/node_modules.asar.unpacked/@vscode/ripgrep/bin/rg' \
    --files \
    --hidden \
    --case-sensitive \
    -g '/.vscode/launch.json' \
    -g '!**/node_modules/**' \
    -g '!**/.git' \
    -g '!**/.svn' \
    -g '!**/.hg' \
    -g '!**/CVS' \
    -g '!**/.DS_Store' \
    -g '!**/Thumbs.db' \
    --no-ignore \
    --follow \
    --no-config \

The -g '!**/.git' exclusion comes from VS Code's default settings.

{
  "files.exclude": {
    "**/.git": true,
    "**/.svn": true,
    "**/.hg": true,
    "**/CVS": true,
    "**/.DS_Store": true,
    "**/Thumbs.db": true
  }
}

https://github.com/microsoft/vscode/blob/2798da9d47387c0fb6ab4ebbead95efb5a85c250/src/vs/workbench/contrib/files/browser/files.contribution.ts#L143

It would be interesting if there is some performance comparison data to share.

Definitely. I was curious too, but hadn't had time until now to run some tests.

Without the **/node_modules/** exclusion, the rg process runs for over an hour at 1100%+ CPU in the repo my team works on. I can run the process overnight to see exactly how long it takes if that's something we're curious about.

Here's some data for comparing whether .git is excluded. For my specific machine and repo:

node_modules Excluded	.git Excluded	Trial 1	Trial 2	Average
❌	✅	1 hour+	-	1 hour+
✅	❌	536ms	493ms	514.5ms
✅	✅	788ms	477ms	632.5ms

It looks like excluding .git is fairly negligible in terms of timing. Either way, I don't think we have to do something explicit here since .git should be excluded unless users explicitly set files.excludes["**/.git"] to false?

[testforstephen]

Great to see the perf data, thanks for sharing that. Looks like it's enough to exclude "node_modules" only.

[jdneo]

Looks like this utility can get another improvement if we set the workspaceFolder as the base part of the relative pattern

https://github.com/microsoft/vscode/blob/4b759b0492d1df969e8595e3db0f66f6058c3643/src/vscode-dts/vscode.d.ts#L2116

And then limit the search maxResults to 1

https://github.com/microsoft/vscode/blob/4b759b0492d1df969e8595e3db0f66f6058c3643/src/vscode-dts/vscode.d.ts#L12171

[testforstephen]

I created an issue to track the further optimization. #1252

[gluxon]

@jdneo Those both look like great suggestions. Thanks!

[gluxon]

@gluxon this is a good finding, thanks for bringing it up for our awareness.

Thank you for reviewing and considering it! This change will be a big help to my team.

[testforstephen]

@gluxon thanks for contribution.