-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
GH-3413: Made the FS watcher start
non-blocking.
#5111
Conversation
I tried it locally, the FS change events from the watcher arrived as expected, the navigator worked as before. The application startup has improved a lot. |
c2aa5b8
to
607795c
Compare
I don't understand why it helps. Could you elaborate? I could not find a single client awaiting |
}); | ||
} | ||
} | ||
toDisposeWatcher.push(Disposable.create(async () => { |
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.
it is not the same as before. This code should not be executed if toDisposeWatcher.disposed
is true
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.
Thanks for spotting it!
@@ -130,30 +129,39 @@ export class NsfwFileSystemWatcherServer implements FileSystemWatcherServer { | |||
console.warn(`Failed to watch "${basePath}":`, error); | |||
this.unwatchFileChanges(watcherId); | |||
} | |||
}).then((watcher: nsfw.NSFW | undefined) => { | |||
if (!watcher) { |
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.
how can it be undefined
here?
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.
It cannot be, but based on the original code, we set the reference to undefined
, so this is the only way to trick the compiler. By the way, see the original code, it was undefined
there as well. I think this change is correct.
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.
i see, it seems before tsc was smart enough to figure out that it cannot be undefined after calling nsfw
Closes #3413 Signed-off-by: Akos Kitta <[email protected]>
I have experienced the performance drop in the electron-based application for a downstream project, but I could reproduce it with the Theia electron example too. Note, one does not have to bundle it, just start with |
I could not reproduce it in the browser consistently, it should be an electron specific problem. Here is my CPU profile of the electron application. I have used
|
A short update before I forget; this performance drop issue does not happen at all on my other mac. It also works perfectly with the browser example. |
Closes #3413
Signed-off-by: Akos Kitta [email protected]