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

Fix #79047 - Wasted stat call on workspace root #79052

Merged
merged 2 commits into from Aug 14, 2019

Conversation

@gjsjohnmurray
Copy link
Contributor

commented Aug 13, 2019

This fix prevents a wasted stat call being made to a FileSystemProvider in specific circumstances. See #79047

@isidorn isidorn self-assigned this Aug 13, 2019

@isidorn isidorn added this to the August 2019 milestone Aug 13, 2019

@@ -159,6 +159,12 @@ export class ExplorerService implements IExplorerService {
const options: IResolveFileOptions = { resolveTo: [resource], resolveMetadata: this.sortOrder === 'modified' };
const workspaceFolder = this.contextService.getWorkspaceFolder(resource);
const rootUri = workspaceFolder ? workspaceFolder.uri : this.roots[0].resource;

This comment has been minimized.

Copy link
@isidorn

isidorn Aug 14, 2019

Contributor

Your check makes sense but I think we can make this more general.
I do not like the statment on line 161 that if the workspaceFolder does not exist we take the first root.

Let's change that, suche that if the workspaceFolder is not found the rootUri is undefined.
And if it is undefined we should just return.

I think this should also handle your use case. Let me know what you think about this and how ti goes.

This comment has been minimized.

Copy link
@gjsjohnmurray

gjsjohnmurray Aug 14, 2019

Author Contributor

@isidorn I agree. Have just updated my branch.

@isidorn

This comment has been minimized.

Copy link
Contributor

commented Aug 14, 2019

I have commented on your change directly. Once you address that we can merge this in.
The reason why I wouldp refer that approach is that if the resource does not belong to any root than a select is simply a no-op.

@isidorn

This comment has been minimized.

Copy link
Contributor

commented Aug 14, 2019

Looks great, merging in.
Thanks a lot!

@isidorn isidorn merged commit 9465c78 into microsoft:master Aug 14, 2019

2 checks passed

VS Code #20190814.15 succeeded
Details
license/cla All CLA requirements met.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.