watchers: use singleton server

[paul-marechal]

Theia spawns as many nsfw servers as there are clients, maybe more.

This commit replaces this behaviour by centralizing everything into the
same service process in order to optimize resources (watch handles are a
limited resource on a given Linux host).

Some notes on the current design:

• Every time someone requests to watch an (uri, options) couple we will
  re-use the same watch handle (and actual nsfw instance). This allows for
  resource allocation optimizations.
• Since a given service can only serve a single client, I added an event
  dispatcher named FileSystemWatcherServerDispatcher. You must register
  yourself with some unique id you will use to make requests.
• The new .watchUriChanges function now takes a clientId so that
  when events are sent to the dispatcher it then gets routed back to your
  own client.
• Added a --nsfw-watcher-verbose backend argument to enable verbose
  logging for the watcher server.

Signed-off-by: Paul Maréchal paul.marechal@ericsson.com

How to test

• Enable the sample.file-watching.verbose preference.
• Run yarn --cwd examples/browser start --nsfw-watcher-verbose
• Watch for the different logs coming out of the watcher server:
  • There shouldn't be new watchers without a frontend opened.
  • Once you open a frontend on a given workspace for the first time, you should see NEW log notifications.
  • If you open a second frontend on the same workspace, you should see REF++ log notifications
  • Closing one of the two frontend should emit REF-- log notifications.
  • Closing the last frontend should schedule the watcher de-allocation about 10s later.
    • But if you re-open a frontend before 10s, it should reference the watchers again and not dispose of them.
• Comparing with master:
  • Run the backend.
  • Run lsof | grep inotify | wc -l a couple of times before opening a frontend and note the general value.
  • Open frontends on some big workspace (your home maybe?) while noting the new value for lsof | grep inotify | wc -l each time.
  • The conclusion should be that on master the number increases with each frontend, but not with these changes.
• Running the @theia/example-browser app:
  • Open 2 tabs on different workspaces.
  • Create and delete files in the respective workspaces.
  • Make sure that a log is only reported from the frontend that was watching the modified workspace.
    • Sample File Watching: X file(s) changed! <workspace>

Review checklist

• as an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• as a reviewer, I agree to behave in accordance with the review guidelines

[paul-marechal]

This is a draft pull-request, I'd like to know if this change cause trouble to any party.

This is an optimization that should help us at Ericsson handle very large workspaces when users open multiple windows.

The changes should be fully functional, only the API would require some cleaning: watchUriChanges2 and the like. I want to know if the shape is good before making anything definitive.

[paul-marechal]

@akosyakov I originally wanted to get a unique identifier for a given frontend connecting to the backend in the FileSystemWatcherClient class, but I had to create a sequence manually instead: https://github.com/eclipse-theia/theia/blob/mp/watch-server-singleton/packages/filesystem/src/node/filesystem-watcher-client.ts#L39

Is there a way using inversify to generate one when handling an incoming frontend connection, and pass that around? When I tried I couldn't get anything to be injected correctly.

[akosyakov]

I won't have time for reviews in this and next months please proceed. File watching we should ensure that clients only get events which they ask for. If this PR improves performance and does not break it then it is fine. For performance optimisations PR should provide a reproducible method to collect some numbers before and after. A reviewer should run this method too and decide whether code change is worth improvements.

[paul-marechal]

@akosyakov got it, thanks for your feedback! I'll provide a way to benchmark tomorrow (although one can already get a feel just enabling the --nsfw-watcher-verbose flag).

[paul-marechal]

Added more things to test.

[akosyakov]

Please add tests to check that opening 2 tabs with different folders ensures that each tab does not receive events about workspace of another tab.

[paul-marechal]

@akosyakov thanks to this simple request, it made me notice a bug where events weren't correctly forwarded when running the server in a child process. This is now fixed and I added more things to test in the PR description.

[paul-marechal]

This PR is good for review now.

[paul-marechal]

@kittaakos thanks for your comments! I think I addressed everything :)

[kittaakos]

I think I addressed everything :)

Very nice. Thanks for the heads-up, @marechal-p. I will give it a try on Windows this week.

[paul-marechal]

I rebased to resolve the conflict. cc @kittaakos

[kittaakos]

I rebased to resolve the conflict. cc @kittaakos

Thanks for the heads-up, @marechal-p. I tried it last week: it worked for me although I did not fully understand the verbose file-watcher logs. Please someone else also take a look into the changeset. Thanks!

[vince-fugnitto]

The changes work very well for me and it is definitely an overall improvement to the file watchers 👍

I verified the source code (offline review), as well as tested as many use-cases and scenarios I could think of to validate the functionality.