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.

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

Smarter workspaceContains handling? #34823

Closed
bpasero opened this issue Sep 22, 2017 · 7 comments
Closed

Smarter workspaceContains handling? #34823

bpasero opened this issue Sep 22, 2017 · 7 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug

Comments

@bpasero
Copy link
Member

bpasero commented Sep 22, 2017

Would it be possible to avoid a full workspace scan from every workspaceContains that an extension declares by stopping a search once the first result is found? I know that this should work from the classic file traversal implementation but I am not sure about RipGrep. Maybe there is a command to use RipGrep in such a way that it would just return 0 or 1 when a result is found?

Alternatively we could also do a heuristic: we could first do a quick check of the root level files of the workspace folders and only run the more expensive file search if no files match. I would argue that in most cases the file of interest would actually be in the root folder (e.g. tsconfig.json).

@bpasero bpasero added the under-discussion Issue is under discussion for relevance, priority, approach label Sep 22, 2017
@bpasero bpasero changed the title Smarter workspaceContains Smarter workspaceContains? Sep 22, 2017
@bpasero bpasero changed the title Smarter workspaceContains? Smarter workspaceContains handling? Sep 22, 2017
@alexdima
Copy link
Member

At the time this was implemented, that was the contract of the DiskSearch.

I don't know if that changed in the meantime.

https://github.com/Microsoft/vscode/blob/de99997ab5715de51a1a9903acf46e487e73f7fa/src/vs/workbench/node/extensionHostMain.ts#L224

		const query: ISearchQuery = {
			folderQueries: this._workspace.folders.map(folder => ({ folder: folder.uri })),
			type: QueryType.File,
			maxResults: 1,
			includePattern: includes
		};

@alexdima alexdima removed their assignment Sep 22, 2017
@bpasero
Copy link
Member Author

bpasero commented Sep 22, 2017

Ah ok, didn't know about maxResults, @roblourens can maybe clarify if this is supported in RipGrep too.

@roblourens
Copy link
Member

It is implemented for text search with ripgrep, but looks to be not implemented for filesearch with rg or find. @chrmarti

@chrmarti
Copy link
Collaborator

@roblourens --quiet will tell us if there is a matching file (in ripgrep 0.6.0, we'd need to upgrade from 0.5.2) but not what the match is (which we need for maxResults: 1, but not necessarily for workspaceContains). I don't see any other options we can use, --max-count doesn't do anything with --files.

@roblourens
Copy link
Member

We can kill the process as soon as we get the right number of results, this is what text search does.

@chrmarti
Copy link
Collaborator

We could optimize this case with an additional flag exists: true and for that pass --quiet. That will avoid all output to stdout and terminate immediately on the first hit. This will need an upgrade of ripgrep we ship though.

@chrmarti chrmarti added this to the September 2017 milestone Sep 26, 2017
@chrmarti chrmarti added bug Issue identified by VS Code Team member as probable bug and removed under-discussion Issue is under discussion for relevance, priority, approach labels Sep 27, 2017
@chrmarti
Copy link
Collaborator

One way to verify with extension code is by using workspace.findFiles with a low maxResults against a large workspace and make sure it is much faster than without maxResults.

@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.
Labels
bug Issue identified by VS Code Team member as probable bug
Projects
None yet
Development

No branches or pull requests

4 participants