Fix 100% CPU caused due to FS watchers when using liveshare #20006
Conversation
| @@ -179,7 +179,7 @@ function watchRoots(args: WatchRootsArgs): IDisposable { | |||
|
|
|||
| function createWorkspaceLocator(ext: ExtensionState): WorkspaceLocators { | |||
| const locators = new WorkspaceLocators(watchRoots, [ | |||
| (root: vscode.Uri) => [new WorkspaceVirtualEnvironmentLocator(root.fsPath), new PoetryLocator(root.fsPath)], | |||
| (root: vscode.Uri) => [new WorkspaceVirtualEnvironmentLocator(root), new PoetryLocator(root.fsPath)], | |||
There was a problem hiding this comment.
is PeotryLocator safe? if root happen to have pyproject.toml on guest live share box, it might start to execute some shell commands on the remote machine? feels like getRoots needs to be changed to Uri and any locator that assumes file:// need to deal with non local uri?
There was a problem hiding this comment.
The WorkspaceEnvironmentLocator seems to be the only one that doesn't pass a file to the file watcher. All the others seem to pass an exact file to look for.
There was a problem hiding this comment.
But yeah they should all use the uri. I didn't want to make that big of a change though. It ripples all over the place.
There was a problem hiding this comment.
@karthiknadig thoughts? Should I switch them all to use a URI and check the scheme?
There was a problem hiding this comment.
Oh wait I just realized what Heejae was suggesting. It doesn't look it the locator runs any code, but callers of this might.
There was a problem hiding this comment.
@karrtikr Can you answer the question?
There was a problem hiding this comment.
Discovering or watching for Python interpreters doesn't make sense in vsls:/, as it's a virtual workspace. So I think ideally discovery component should be disabled as a whole. The issue is that we're initializing FS watchers in the constructor itself: #20023.
For now try checking if it's a virtual workspace before activating the watchers here:
Changes to URI are then not needed. Add:
export async function isVirtualWorkspace(): Promise<boolean> {
const service = internalServiceContainer.get<IWorkspaceService>(IWorkspaceService);
return service.isVirtualWorkspace;
}in externalDependencies.ts and import it here.
|
Plan to have a look tomorrow, marking as draft. |
| @@ -179,7 +179,7 @@ function watchRoots(args: WatchRootsArgs): IDisposable { | |||
|
|
|||
| function createWorkspaceLocator(ext: ExtensionState): WorkspaceLocators { | |||
| const locators = new WorkspaceLocators(watchRoots, [ | |||
| (root: vscode.Uri) => [new WorkspaceVirtualEnvironmentLocator(root.fsPath), new PoetryLocator(root.fsPath)], | |||
| (root: vscode.Uri) => [new WorkspaceVirtualEnvironmentLocator(root), new PoetryLocator(root.fsPath)], | |||
There was a problem hiding this comment.
Discovering or watching for Python interpreters doesn't make sense in vsls:/, as it's a virtual workspace. So I think ideally discovery component should be disabled as a whole. The issue is that we're initializing FS watchers in the constructor itself: #20023.
For now try checking if it's a virtual workspace before activating the watchers here:
Changes to URI are then not needed. Add:
export async function isVirtualWorkspace(): Promise<boolean> {
const service = internalServiceContainer.get<IWorkspaceService>(IWorkspaceService);
return service.isVirtualWorkspace;
}in externalDependencies.ts and import it here.
| traceError(ex); | ||
| this.watchersReady?.reject(ex); | ||
| }); | ||
| this.watchersReady.resolve(); |
There was a problem hiding this comment.
This should be outside the if statement.
There was a problem hiding this comment.
I should test again. It might have been working just because the promise never resolved.
…hiodo/vscode-python into rchiodo/filewatcher_liveshare
karrtikr
changed the title
…/vscode-python#20006) Fixes microsoft/vscode-python#20005 Root cause is roots coming from liveshare. Liveshare gives a workspaceFolder of `vsls:/`.
Fixes #20005
Root cause is roots coming from liveshare. Liveshare gives a workspaceFolder of
vsls:/.