Skip to content
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.

Sign up for GitHub

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

Jump to bottom

workspaceContains starts a search over full workspace, including .git/, node_modules/ #34487

Closed
roblourens opened this issue Sep 15, 2017 · 7 comments
Closed

workspaceContains starts a search over full workspace, including .git/, node_modules/ #34487

roblourens opened this issue Sep 15, 2017 · 7 comments
Assignees
@alexdima
Labels
extension-host Extension host issues perf
Milestone
September 2017

Comments

@roblourens
Copy link
Member

roblourens commented Sep 15, 2017 •
edited

Since 10c0610

This could be very expensive. We should include the exclude settings, or hardcode an exclusion to a few folders.
@roblourens roblourens added extension-host Extension host issues perf labels Sep 15, 2017
@roblourens
Copy link
Member Author

Also, we could batch workspaceContains searches into a single search. For example the C# extension will spawn 6 ripgrep processes.

@roblourens roblourens mentioned this issue Sep 15, 2017
Closed
@jrieken jrieken assigned alexdima and unassigned jrieken Sep 18, 2017
@jrieken jrieken added this to the September 2017 milestone Sep 18, 2017
@jrieken
Copy link
Member

jrieken commented Sep 18, 2017

We should include the exclude settings, or hardcode an exclusion to a few folders.

So, how do you then active on .git/HEAD?

@roblourens
Copy link
Member Author

Maybe add some smartness to search .git/ or node_modules/ if it's explicitly mentioned in the workspaceContains: search glob?

@alexdima
Copy link
Member

I will not push any change that changes the semantics. But I agree, if an extension has multiple glob patterns, we can invoke DiskSearch.search at most once per extension.

@roblourens roblourens mentioned this issue Sep 20, 2017
Closed
@roblourens
Copy link
Member Author

Thanks, that should help. Opening #34711 to track other strategies

@roblourens
Copy link
Member Author

Now I see it spawning some rg processes with no glob args at all. I think there might be a bug. Debugging...

@alexdima
Copy link
Member

alexdima commented Sep 21, 2017 •
edited

@roblourens Nice catch! Yes, it can happen if an extension uses strict file names and no glob patterns

alexdima added a commit that referenced this issue Sep 21, 2017
@alexdima
Do not search for empty glob patterns (#34487)
883a04e
@deysaikat95 deysaikat95 mentioned this issue Sep 25, 2017
Closed
@roblourens roblourens mentioned this issue Oct 12, 2017
Closed
@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@alexdima alexdima

Labels
extension-host Extension host issues perf
Projects
None yet
Milestone
September 2017
Development

Successfully merging a pull request may close this issue.

Update from original (#1)
3 participants
@roblourens @jrieken @alexdima