don't block web socket with many plugins

[akosyakov]

What it does

• fix plugin system does not scale over JSON-RPC #6228:
  • use permessage-deflate to compress any ws message greater than 256kb
  • remove raw package data from plugin metadata
    • frontend plugins don't have access to it anymore
    • backend plugins recover it from the disk
  • only fetch not yet loaded plugins to the frontend
  • install contributions info only to process
    metadata in the frontend
  • install dependencies info only to process metadata on the backend
  • initialize debugger contributions from metadata, instead of sending it first to the plugin host and then back to the frontend
• closes don't send plugin metadata over JSON-RPC #6233 - superseded
• update the changelog with a breaking change
• checked license compatibility of new dependencies

How to test

• remove native extension from the browse example and rebuild:
  • textmate grammar
  • debug-nodejs
  • java
  • java-debug
• install 57 VS Code extensions in plugins folder:
  https://drive.google.com/file/d/1tuOsorMVOKi_twL9oAuZxgrD7-VUwNsj/view?usp=sharing
  • could not come up with a command to pull it from the gdrive in the terminal, if someone knows how to, please share
• use system proxy to emulate slow connection, e.g. Charles proxy
  • you cannot do it in Chrome, since it cannot throttle web sockets
• measure how long does it takes to activate all plugins before and after
• observe whether you get yellow status bar and ws frames hangs for 20-30 seconds after and before
• play with unstable connections to confirm that plugins get reloaded/unloaded on the reconnection

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

[akosyakov]

In order to reproduce on the master state:

• install Charles proxy and configure it to throttle with 3G for localhost
• apply these patch to collect how long does it take to fetch metadata and start plugins:
  measure_before.txt
• install 57 extensions: https://drive.google.com/file/d/1tuOsorMVOKi_twL9oAuZxgrD7-VUwNsj/view?usp=sharing

My results on 3G for 57 extensions:

• before:
  • yellow status bar
  • unresponsive UI
  • fetching metadata: getDeployedMetadata took 16066.520000109449 ms
  • sending metadata to the plugin host and staring plugins: startPlugins took 48116.30499991588 ms
• after:
  • no yellow status bar
  • responsive UI
  • fetching metadata: Sync of 57 plugins took: 2785.3900000918657 ms
  • starting plugins: Start of 57 plugins took: 1804.2249998543411 ms

I also measured on my normal connection. In this case results are not so different than on 3G with this PR.

[benoitf]

Great improvement.

FYI on macOS there is Apple Network Link Conditioner utility

[svenefftinge]

works nicely

[akosyakov]

@benoitf please have a look again, I've addressed what I could.

[akosyakov]

I'm going to rebase and merge it if the CI is green and no one has questions to changes of this PR. Current state is quite broken and works for couple plugins, leading to bad UX whenever a bit more is installed.

[benoitf]

thx @akosyakov for the great work on rewriting commits

I still have issues with the last two commits
e9acc49

016f6d6

for the first one, it seems we're in the middle of something.
We have metadata with model, etc but in fact they're not the same as for some you will have contribution/dependencies and for some case you won't. So when you're manipulating metadata you don't know quickly if you're in a mode where you have access or not to it.

For example in https://github.com/eclipse-theia/theia/blob/e9acc497ae23441529f20290ebcba8d97fa8c599/packages/plugin-dev/src/node/hosted-plugin-reader.ts we're just interested in knowing if it's a frontend or a backend plug-in but we go in full loop

so we don't need all this extra options method everywhere which is bloating the code

about the last one, which is very related to the previous one, it's still a hack

[akosyakov]

@benoitf What is your suggestions how to handle it?

[benoitf]

remove 2 commits from PR and merge it (but it won't solve the blocking)
or solve the metadata/model.

I'm not saying there are no global issues on deploying a lot of plug-ins, but it's not because we have a PR that fix an issue and that CI test is green that we should merge something that is leading to more complex codebase with dirty hacks.

[akosyakov]

or solve the metadata/model.

I meant what is you suggestion about it? And:

for the first one, it seems we're in the middle of something.
We have metadata with model, etc but in fact they're not the same as for some you will have contribution/dependencies and for some case you won't. So when you're manipulating metadata you don't know quickly if you're in a mode where you have access or not to it.

how do you suggest to solve it?

[akosyakov]

for the first one, it seems we're in the middle of something.
We have metadata with model, etc but in fact they're not the same as for some you will have contribution/dependencies and for some case you won't. So when you're manipulating metadata you don't know quickly if you're in a mode where you have access or not to it.

I don't see how it can happen: if the plugin was deleted it will be unloaded by next deployment from the frontend and the plugin host process. ...It can happen if one somehow manually manipulates with the file disk, but when it can happen at any time when the plugin system reads package.json. Don't think we should expect it.

about the last one, which is very related to the previous one, it's still a hack

For the hack for frontend plugins, I'm still not sure what your expectations. Should we install undefined? Should we throw on access of packageJSON?

[benoitf]

for the first one, it seems we're in the middle of something.
We have metadata with model, etc but in fact they're not the same as for some you will have
contribution/dependencies and for some case you won't. So when you're manipulating metadata >> you don't know quickly if you're in a mode where you have access or not to it.

I don't see how it can happen: if the plugin was deleted it will be unloaded by next deployment
from the frontend and the plugin host process. ...It can happen if one somehow manually
manipulates with the file disk, but when it can happen at any time when the plugin system reads
package.json. Don't think we should expect it.

I'm not talking about the cache or reloading. I mean, when you see a metadata object you might call metadata.model.contributes on it but sometimes it will be there and sometimes no. (even if it's the package.json) because some options are on or off and then you'll have to go to see which option as been given in upper calls, etc.

as I said, by looking in the code, we're giving options parameter without contributes:true just to know if it's a backend or a frontend. overkill.

about the last one, which is very related to the previous one, it's still a hack

For the hack for frontend plugins, I'm still not sure what your expectations. Should we install undefined? Should we throw on access of packageJSON?

the hack is on stubbing and broken raw model

[svenefftinge]

We have metadata with model, etc but in fact they're not the same as for some you will have contribution/dependencies and for some case you won't. So when you're manipulating metadata you don't know quickly if you're in a mode where you have access or not to it.

I don't see how this is a hack or bad code. It is a classic optimization based on fetch profiles and gladly we have a type system that tells us if something can be undefined.

[akosyakov]

I've noticed that with an opened editor loading contributions takes around 15s. It's turned out because we try to detect a language for plaintext editors for each extension, more extension we have, longer it takes. I will schedule auto detection for another tick that we can start plugins earlier. - after removing unnecessary language redetection it is around 3-5s

Another issue is that we register languages before grammars, but grammars are activated when a language is activated. So if a language is activated before the grammar is registered then a grammar is never activated. And it happens if you have an opened editor. We don't see it with textmate-grammars native extension, because everything always registered before any Monaco model is created, in this PR it gets very apparent. I will inverse the registration order to fix it.

Updated
Also Monaco 0.17 guesses languages for opened plain text models by default, so doing us the same just increases the loading time. I will revert #5754

[akosyakov]

@svenefftinge addressed #6252 (comment) with 70e51c4

[akosyakov]

All commits except reducing the plugin metadata are extracted to #6267

[benoitf]

cc @azatsarynnyy @evidolob

From what I see with the breaking changes, it will be a total breakage of Eclipse Che and remote plug-ins.
BTW it is not so clear in changelog that it involves breakage of deployment APIs and that a lot of APIs were renamed, some sync methods are now async and new lifecycle method is added.
Reading changelog it looks like a smooth change while it is definitely not

We don't have many days of usage (even if it was merged last friday) so I would suggest to postpone the merge just after the release of Theia occuring today to let ppl stick to a stable API and enhance the changelog by explicitely saying interfaces that are broken (it's huge)

Anton did a very good rewrite on the last two commits by removing cleanly the raw stuff. I would just postpone merge after upcoming Theia release

[akosyakov]

@benoitf I will update the changelog.

@marcdumais-work the release is today right?

[marcdumais-work]

Hi @akosyakov , @benoitf

It's the plan to do the release today, yes. I see there was a lot of recent action about plugins, and that the situation is perhaps not completely resolved. So I defer to you about whether we should go ahead today or wait a bit for a more satisfactory resolution. Doing a 0.11.1 release in e.g. a week is also an option.

Please advise.

[akosyakov]

@marcdumais-work we are going to land it only afterwards, please proceed with the release

[akosyakov]

@benoitf @svenefftinge @azatsarynnyy @evidolob the changelog is updated, the release is done

From what I see with the breaking changes, it will be a total breakage of Eclipse Che and remote plug-ins.

I've looked at Che on usage of renamed/replaced APIs and found:

• https://github.com/eclipse/che-theia/blob/b3c9138bb25e191e0c4b9cef682edb04ebdfa265/extensions/eclipse-che-theia-plugin-ext/src/browser/plugin/che-plugin-frontend-service.ts which can be replaced with get plugin ids and then for each get metadata as frontend does it now in Theia
• https://github.com/eclipse/che-theia/blob/69ae14f116c189a2561e2d45007fb4d7b47b365f/extensions/eclipse-che-theia-plugin-remote/src/node/plugin-remote-init.ts probably not going to break, since PluginReader still has getPluginMetadata.

APIs were renamed, some sync methods are now async and new lifecycle method is added.

PluginHost.init still supports sync version, it is maybe promise, not a promise. I could not find that lifecycle method PluginManagerExt.$init is used in Che.

I've looked at https://github.com/eclipse/che-theia.

Again such work should be done on the product level when it is about to migrate, not a framework level. JS provides access to anything at runtime, even to private stuff via index access, and DI allow to override and inject whatever product wants. I don't think there is a way to break completely.

[svenefftinge]

Awesome work, Anton!
@benoitf I assume we can merge this now?

[benoitf]

@akosyakov thx for the heads up on che-theia repository.
Well it's not because we have access to private components that the product is extensible. Also DI is not completely there.
BTW thx for the latest improvements, it's a more clean solution.

@svenefftinge yes