-
Notifications
You must be signed in to change notification settings - Fork 215
Fix problem with too many ripgrep processes being spawned by VSCode #1287
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix problem with too many ripgrep processes being spawned by VSCode #1287
Conversation
So I've made some changes here:
We do still listen to onDidOpenTextDocument(…) which can be called even for documents not in a visible editor. This is fine because the search only happens one time. After checking a number of other extensions they all pretty much use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR addresses the issue of spawning too many ripgrep processes by introducing caching for the file search operation. Key changes include:
- Introducing a new module-level variable, pendingSearch, to cache the pending search promise.
- Refactoring activate() to use pendingSearch via the nullish coalescing assignment (??=) and awaiting that promise.
- Ensuring that bootClientIfNeeded() is invoked only once per activation, reducing redundant searches.
50432c9
to
0fc5697
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the handling of file searches for Tailwind CSS in VSCode to prevent unnecessary spawning of ripgrep processes. Key changes include removing redundant file exclusion logic from extension.ts, creating dedicated modules (exclusions.ts, api.ts, analyze.ts) to manage file analysis and exclusion, and updating the search logic to run only when a document is visible and the language server requires it.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
File | Description |
---|---|
packages/vscode-tailwindcss/src/extension.ts | Refactored search logic to use a new API and removed redundant exclusion and cancellation code. |
packages/vscode-tailwindcss/src/exclusions.ts | Moved file exclusion logic into its own module. |
packages/vscode-tailwindcss/src/api.ts | Introduced an API for checking if the language server should be started. |
packages/vscode-tailwindcss/src/analyze.ts | Moved file analysis logic (including searching for Tailwind CSS-related patterns) into a separate module. |
Comments suppressed due to low confidence (1)
packages/vscode-tailwindcss/src/api.ts:39
- The function name 'stylesheeNeedsLanguageServer' appears to be misspelled. Consider renaming it to 'stylesheetNeedsLanguageServer' for clarity.
async function stylesheeNeedsLanguageServer(uri: Uri) {
4436f5a
to
027a18a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did generate a lot of files in a mono repo setup and didn't run into any issues.
I also checked that it still works as expected on "normal" projects.
We run a file search on a per-workspace basis but we wait until a file has been opened. However, there are two big problems here:
A file being "opened" does not mean it is visible. Just that an extension has, effectively, taken an interest in it, can read its contents, etc. This happens for things like tsconfig files, some files inside a
.git
folder, etc…We're running the search any time we see an opened document. What should happen is that we run the search when a document is opened and visible, the language server has not started, and we need to checking a workspace folder that has not been searched yet.
This code here needs to be restructured to ensure that these searches only run when they are needed. If the searches don't return anything or time out then they should not be run again. Notifications from file watching should take care of the rest in case the initial search turned up nothing and the user adds a file that should cause the server to start.
Fixes #986 (for real maybe this time??)