Use binary message RPC protocol for plugin API #11261

tortmayr · 2022-06-06T16:44:35Z

What it does

Refactors the plugin RPC protocol to make use of the new message-rpc introduced with #11011/#11228.

Refactor plugin-ext RpcProtocol API to reuse the new message-rpc protocol
- Remove custom RPC message encoding and handling reuse message-rpc
- Implement QueuingChannelMultiplexer that queues messages and sends them accumulated on the next process.tick (replaces the old Multiplexer
  implementation)
- Refactors proxy handlers and remote target handlers
- Use Channel instead of MessageConnection for creating new instances of RPCProtocol
- Refactor RpcMessageEncoder/RpcMessageDecoder to enable overwritting of already registered value encoders/decoders.
- Add mode property to base RpcProtocol to enable switching from a bidirectional RPC protocol to a client-only or server-only variant.
Implement special message encoders and decoders for the plugin communication. (Replacement for the old ObjectTransferrer JSON replacers/revivers)
Adapt HostedPluginServer and HostedPluginClient API to send/receive messages in binary format instead of strings. This enables direct writethrough of the binary messages received from the hosted plugin process.
Adapt hosted-plugin-process and plugin-host to directly send binary messages via IpcChannel/BinaryMessagePipe
Remove incorrect (and unused) notification proxy identifiers and instantiation
- NotificationExt was instantiated in the main context
- There were unused notification proxy identifiers for main and ext in the wrong contexts

Co-authored-by: Lucas Koehler [email protected]
Contributed on behalf of STMicroelectronics

How to test

For regression testing, any plugin loaded in the example applications can be used. Everything (i.e. interacting with and using the plugin should work as before.

For performance testing see comment below

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

tortmayr · 2022-06-06T16:57:28Z

Performance Testing

This PR greatly improves the performance of FileSystem calls from within a plugin/VS Code extension. To test this with have used a simply VS Code extension that offers to commands to write/read file of different sample sizes to the workspace and logs the execution time in the console. The source code is available here: https://github.com/eclipsesource/theia/tree/json-rpc-performance-test-extension

Benchmarks

We used the extension to measure performance in browser and electron context on:

The last commit to master with the old RPC system (53a272e)
The current master state (96a3950)
The PR state
Local VS Code to establish a base line

Plugin File Write (Browser)	100 KB	1000 KB	10000 KB
With old RPC (#53a272)	60.23 ms	545.11 ms	4903 ms
With current master	50.83 ms	210.46 ms	2301 ms
With PR	42.23 ms	42.63 ms	158.46 ms

Plugin File Read (Browser)	100 KB	1000 KB	10000 KB
With old RPC (#53a272)	55.86 ms	546.54 ms	4970 ms
With current master	34.64 ms	287.36 ms	2945 ms
With PR	13.56 ms	20.03 ms	151.54ms

Plugin File Write (Electron)	100 KB	1000 KB	10000 KB
With old RPC (#53a272)	44.69 ms	430.86 ms	4274 ms
With current master	40.34 ms	211.51 ms	2024 ms
With PR	14.19 ms	41.91 ms	171.02 ms

Plugin File Read (Electron)	100 KB	1000 KB	10000 KB
With old RPC (#53a272)	46.98 ms	463.21 ms	4600 ms
With current master	40.12 ms	283.28 ms	2479 ms
With PR	16.62 ms	23.665 ms	153.41 ms

Local VS Code (Baseline)	100 KB	1000 KB	10000 KB
File Write	15.81 ms	30.52 ms	87.27 ms
File Read	8.1 ms	27.56 ms	80.2 ms

tsmaeder · 2022-06-08T13:36:24Z

I've done a little testing, and I noticed that I get a ton of cancellation error stack traces in the console

Uncaught Error: There is no document for breakpointinput:vs.editor.ICodeEditor%3A4

Error: There is no document for breakpointinput:vs.editor.ICodeEditor%3A4
at LinkProviderAdapter.provideLinks (C:\Users\thomas\code\chedev\theia\packages\plugin-ext\lib\plugin\languages\link-provider.js:31:35)
at C:\Users\thomas\code\chedev\theia\packages\plugin-ext\lib\plugin\languages.js:287:97
at LanguagesExtImpl.withAdapter (C:\Users\thomas\code\chedev\theia\packages\plugin-ext\lib\plugin\languages.js:118:20)
at LanguagesExtImpl.$provideDocumentLinks (C:\Users\thomas\code\chedev\theia\packages\plugin-ext\lib\plugin\languages.js:287:21)
at C:\Users\thomas\code\chedev\theia\packages\plugin-ext\lib\common\proxy-handler.js:105:71
at processTicksAndRejections (node:internal/process/task_queues:96:5)
at async RpcProtocol.handleRequest (C:\Users\thomas\code\chedev\theia\packages\core\lib\common\message-rpc\rpc-protocol.js:176:28)
unexpectedErrorHandler errors.ts:26
setTimeout handler*ErrorHandler/this.unexpectedErrorHandler errors.ts:24
onUnexpectedExternalError errors.ts:67
onUnexpectedExternalError errors.ts:88
errors.ts:26:11
unexpectedErrorHandler errors.ts:26
(Async: setTimeout handler)
unexpectedErrorHandler errors.ts:24
onUnexpectedExternalError errors.ts:67
onUnexpectedExternalError errors.ts:88

Monaco silently accepts cancellation if it detects that it is a cancellation. I think the proper layering would be to catch our own cancellation error and to translate it into a VS Code Cancellation Error at some appropriate place. see https://github.com/microsoft/vscode/blob/f52952332d14de7517cb22b002ef1352ecdee889/src/vs/base/common/errors.ts#L149

tsmaeder · 2022-06-08T13:37:44Z

Every once in a while, completion just seems to either fail or take a long time. I can't repeat nor do I have a handle on what's wrong yet.

tsmaeder · 2022-06-08T15:35:07Z

Turns out, the completion doesn't really fail, but it kind of looks like it's waiting for something: when I just let the cursor sit there, the completion does not complete. As soon as I move the cursor around, I get the completion.

tortmayr · 2022-06-09T07:19:21Z

Thanks for testing this change @tsmaeder.

I've done a little testing, and I noticed that I get a ton of cancellation error stack traces in the console

Details
Monaco silently accepts cancellation if it detects that it is a cancellation. I think the proper layering would be to catch our own cancellation error and to translate it into a VS Code Cancellation Error at some appropriate place. see https://github.com/microsoft/vscode/blob/f52952332d14de7517cb22b002ef1352ecdee889/src/vs/base/common/errors.ts#L149

Good catch! I double checked with the "old" plugin rpc protocol implementation and it indeed throws a VS Code Cancellation Error whereas the new protocol didn't:

theia/packages/plugin-ext/src/common/rpc-protocol.ts

Line 166 in 53a272e

return Promise.reject(canceled());

I have adapted the new protocol accordingly and we now should throw the expected Cancellation Error. In addition, I reverted another "over-optimiatization" that prematurely rejected the response of a canceled request.

I did a little testing and did not encounter cancellation error stack traces anymore.

Every once in a while, completion just seems to either fail or take a long time. I can't repeat nor do I have a handle on what's wrong yet.

Turns out, the completion doesn't really fail, but it kind of looks like it's waiting for something: when I just let the cursor sit there, the completion does not complete. As soon as I move the cursor around, I get the completion.

I could not observe this behavior anymore. Maybe it got fixed with the cancellation error fixes? Could you please retest this? Thanks!

packages/core/src/common/message-rpc/rpc-protocol.ts

tsmaeder · 2022-06-09T09:28:44Z

There is a problem with the completion in the PR: I've made two videos, first the behavior in master:

https://youtu.be/53T4Hvcw1rM

and now in the PR

https://youtu.be/sj8GVgzBC2k

Both videos are mad in master, on the source code of Theia and with the browser example, using Firefox. Completion seems slower and erratic in timing: sometimes leaving the cursor where you clicked before completion seems to block the completion somehow, but eventually the completion appears. To me it feels as if we were "crossing the streams". Is it possible that cancellations and multiple requests might be blocking or interfering with each other? If I look at what's changed with respect to master, what I'm seeing is that we're using a different multiplexing strategy for routing to multiple services and message batching has changed, as well. I think it would be good to have some configurable tracing in the code to find out what's going on. We just need to make sure we're not sending it over the wire with the remote log infrastructure again ;-)

tortmayr · 2022-06-14T12:26:12Z

Some of this issues might be related to the behavior we have encountered in #11249.
Also the potential improvements discussed in #11159.
I will work on these two issues and then continue with cleaning up this PR.

paul-marechal · 2022-06-22T18:21:18Z

I wanted to try out this PR to run benchmarks, but plugins don't seem to work anymore. I am running Windows 10.

tsmaeder · 2022-06-23T08:30:27Z

@paul-marechal how did you resolve the conflicts? @tortmayr could you, please? ;-)

tortmayr · 2022-06-23T14:26:08Z

Sure I can resolve the conflicts and retest it under Windows.

paul-marechal · 2022-06-23T14:32:42Z

@tsmaeder I just pulled the branch as is and built it, I didn't think I had to rebase myself?

packages/plugin-ext/src/hosted/node/hosted-plugin-process.ts

paul-marechal · 2022-07-28T15:37:43Z

~~See eclipsesource#48 for a potential fix for Windows.~~

edit: Obsolete, see new comment.

packages/plugin-ext/src/hosted/node/hosted-plugin-process.ts

lucas-koehler · 2022-08-16T08:37:08Z

@paul-marechal Because @tortmayr is on vacation until the end of this week, I committed your suggestion. Thanks for that :)

tortmayr · 2022-11-02T08:09:36Z

@tsmaeder I have rebased the PR and adapted to the latest changes (msgpack).

There is a problem with the completion in the PR: I've made two videos, first the behavior in master:

https://youtu.be/53T4Hvcw1rM

and now in the PR

https://youtu.be/sj8GVgzBC2k

Both videos are mad in master, on the source code of Theia and with the browser example, using Firefox. Completion seems slower and erratic in timing: sometimes leaving the cursor where you clicked before completion seems to block the completion somehow, but eventually the completion appears. To me it feels as if we were "crossing the streams". Is it possible that cancellations and multiple requests might be blocking or interfering with each other? If I look at what's changed with respect to master, what I'm seeing is that we're using a different multiplexing strategy for routing to multiple services and message batching has changed, as well. I think it would be good to have some configurable tracing in the code to find out what's going on. We just need to make sure we're not sending it over the wire with the remote log infrastructure again ;-)

The recent change to msgpack and other bugfixes seem to have resolved this issue. I can no longer reproduce it

@paul-marechal This PR now also includes your fix to ensure that the BinaryMessagePipe is working when using Windows.

kittaakos · 2022-11-02T08:26:34Z

packages/plugin-ext/src/hosted/browser/hosted-plugin.ts

+        const onCloseEmitter = new Emitter<ChannelCloseEvent>();
+        const onErrorEmitter = new Emitter<unknown>();
+        const onMessageEmitter = new Emitter<MessageProvider>();
+
+        // Create RPC protocol before adding the listener to the watcher to receive the watcher's cached messages after the rpc protocol was created.
+        const rpc = new RPCProtocolImpl({
+            close: () => {
+                onCloseEmitter.dispose();
+                onErrorEmitter.dispose();
+                onMessageEmitter.dispose();
+            },
+            getWriteBuffer: () => {
+                const writer = new Uint8ArrayWriteBuffer();
+                writer.onCommit(buffer => {
+                    this.server.onMessage(pluginHostId, buffer);
+                });
+                return writer;
+            },
+            onClose: onCloseEmitter.event,
+            onError: onErrorEmitter.event,
+            onMessage: onMessageEmitter.event
+        });


This could deserve a dedicated function. What do you think?

The same logic is also in packages/plugin-ext/src/hosted/browser/plugin-worker.ts and packages/plugin-ext/src/hosted/browser/worker/worker-main.ts.

Introduced a new "BasicChannel" class with 2b4016e to reduce the need for code duplication.

kittaakos · 2022-11-02T08:27:26Z

packages/plugin-ext/src/hosted/browser/plugin-worker.ts

+        this.worker.onerror = e => onErrorEmitter.fire(e);
+
+        this.worker.onerror = e => console.error(e);


One of them is probably not needed.

Fixed with 2b4016e

tsmaeder

Mostly questions.

packages/core/src/common/message-rpc/channel.ts

packages/core/src/common/message-rpc/msg-pack-extension-manager.ts

packages/core/src/common/message-rpc/rpc-protocol.ts

packages/plugin-ext/src/common/proxy-handler.ts

packages/plugin-ext/src/common/rpc-protocol.ts

tsmaeder · 2022-11-04T12:56:48Z

In testing I get the following error message in the log (with electron)

2
node.cjs:963 Uncaught (in promise) Error: unknown document
    at currentExtensions.<computed> (node.cjs:963)
    at read (node.cjs:403)
    at Array.readObject [as read] (node.cjs:496)
    at recordDefinition (node.cjs:956)
    at read (node.cjs:392)
    at checkedRead (node.cjs:193)
    at Packr.unpack (node.cjs:103)
    at Packr.decode (node.cjs:175)
    at MsgPackMessageDecoder.decode (rpc-message-encoder.ts:162)
    at MsgPackMessageDecoder.parse (rpc-message-encoder.ts:166)

packages/plugin-ext/src/common/proxy-handler.ts

packages/core/src/common/message-rpc/msg-pack-extension-manager.ts

tsmaeder · 2022-11-21T12:40:58Z

@tortmayr I didn't see anything about the comments in #11261 (comment). What's the story there?

Otherwise, the code looks fine to me now.

tortmayr · 2022-11-28T14:45:13Z

@tortmayr I didn't see anything about the comments in #11261 (comment). What's the story there?

Ah sorry, I forgot to add a comment regarding this. We need msgpackR extension for error encoding to keep the original stacktrace information (i.e. the pendant to

theia/packages/plugin-ext/src/common/rpc-protocol.ts

Line 519 in 32deea2

export function transformErrorForSerialization(error: Error): SerializedError {

for the new RPC protocol)
I'm going to add this once the discussion regarding the msgpackR extension mechanism is resolved (#11261 (comment))

tortmayr · 2022-11-29T08:31:37Z

@tsmaeder I pushed an update that improves the error encoding via msgpack and preserves the stack trace.
Error output from plugins should now look as expected again.

e.g. trying to execute a non-existing command in a plugin would previously result in this message:

2022-11-29T08:18:18.440Z root ERROR [hosted-plugin: 449972] Promise rejection not handled in one second: Error: Command with id 'non.existing.command' is not registered. , reason: Error: Command with id 'non.existing.command' is not registered.
2022-11-29T08:18:18.444Z root ERROR [hosted-plugin: 449972] With stack trace: Error: Command with id 'non.existing.command' is not registered.
    at /home/tobias/Git/OpenSource/theia/theia/node_modules/msgpackr/dist/node.cjs:971:39
    at read (/home/tobias/Git/OpenSource/theia/theia/node_modules/msgpackr/dist/node.cjs:411:15)
    at Array.readObject [as read] (/home/tobias/Git/OpenSource/theia/theia/node_modules/msgpackr/dist/node.cjs:504:18)
    at recordDefinition (/home/tobias/Git/OpenSource/theia/theia/node_modules/msgpackr/dist/node.cjs:964:19)
    at read (/home/tobias/Git/OpenSource/theia/theia/node_modules/msgpackr/dist/node.cjs:400:13)
    at checkedRead (/home/tobias/Git/OpenSource/theia/theia/node_modules/msgpackr/dist/node.cjs:195:13)
    at Packr.unpack (/home/tobias/Git/OpenSource/theia/theia/node_modules/msgpackr/dist/node.cjs:105:12)
    at Packr.decode (/home/tobias/Git/OpenSource/theia/theia/node_modules/msgpackr/dist/node.cjs:177:15)
    at MsgPackMessageDecoder.decode (/home/tobias/Git/OpenSource/theia/theia/packages/core/lib/common/message-rpc/rpc-message-encoder.js:84:29)
    at MsgPackMessageDecoder.parse (/home/tobias/Git/OpenSource/theia/theia/packages/core/lib/common/message-rpc/rpc-message-encoder.js:87:21)

and now correctly preserves the stacktrace of the original error:

2022-11-29T08:23:58.196Z root ERROR [hosted-plugin: 455858] Promise rejection not handled in one second: Error: Command with id 'non.existing.command' is not registered. , reason: Error: Command with id 'non.existing.command' is not registered.
2022-11-29T08:23:58.197Z root ERROR [hosted-plugin: 455858] With stack trace: Error: Command with id 'non.existing.command' is not registered.
    at CommandRegistryMainImpl.$executeCommand (http://localhost:3000/packages_plugin-ext_lib_hosted_browser_hosted-plugin_js.js:1436:19)
    at http://localhost:3000/packages_plugin-ext_lib_common_plugin-api-rpc_js.js:955:71
    at async RpcProtocol.handleRequest (http://localhost:3000/bundle.js:142581:28)

tsmaeder · 2022-11-29T08:39:35Z

@tortmayr just a hint: I consider it good practice to keep updates to PR's in separate commits until ready to merge: it makes it possible just review the delta.

tortmayr · 2022-11-29T09:07:35Z

@tortmayr just a hint: I consider it good practice to keep updates to PR's in separate commits until ready to merge: it makes it possible just review the delta.

Noted, thanks. Apparently I have interacted with too many gerrit based systems recently and developed the bad habit of rebasing/force pushing all changes into a single commit 😉

tsmaeder · 2022-11-29T09:14:05Z

Apparently I have interacted with too many gerrit based systems recently

It's a bit early for a flamewar ;-)

tsmaeder

Some more testing did not reveals anything untoward for me. I'm fine with the code as it stands now.

tsmaeder · 2022-11-30T09:14:55Z

@paul-marechal it looks to me your like your comments have been addressed and we're ready to merge this one?

tortmayr · 2022-11-30T14:37:11Z

For anyone interested I did some benchmarking for plugins and frontend plugins to compare the file reading performance starting from the 1.26 Release:

Benchmarks

Test scenario: Read a file of size x from the workspace using the FileSystem plugin API.
Median of 50 test runs.

Theia 1.26.0 -> Old json-rpc based protocol
Theia 1.32.0 -> Current release
PR -> latest state of this PR branch

Browser

Test context	100K	1000K	5000K	10000K
Plugin (1.26.0)	49 ms	347 ms	1964 ms	4090 ms
Plugin (1.32.0)	27 ms	165 ms	940 ms	2021 ms
Plugin (PR)	11 ms	18 ms	68.5 ms	120 ms

Frontend Plugin (1.32.0)	20 ms	140 ms	741 ms	1642.5 ms
Frontend Plugin(PR)	6.5 ms	10 ms	41 ms	90 ms

Electron

Test context	100K	1000K	5000K	10000K
Plugin (1.26.0)	49 ms	351 ms	2042 ms	4189 ms
Plugin (1.32.0)	40 ms	175.5 ms	969 ms	2018 ms
Plugin (PR)	13.5 ms	20 ms	77 ms	132 ms

VS Code

Test context	100K	1000K	5000K	10000K
Extension (1.73.1 local)	12 ms	15 ms	40 ms	87 ms
Web Extension (1.73.1 local)	6 ms	11 ms	34 ms	53.5 ms

paul-marechal · 2022-11-30T16:48:37Z

@tsmaeder @tortmayr I had trouble formulating what was bothering me but I think I got it:

I don't understand the whole rationale behind the msgpackr extension API:

It stinks that the mspackr API for extensions is global to the whole process, because it means we cannot define sets of extensions to use per connection either. Already it makes me not want to rely on extensions, but I understand that it's the way it currently works.
I don't understand what's special with ResponseError that requires a global extension to be installed. If the rationale is that it's a commonly used type, then what about other recurring types like URI? What's the difference between those types, where one requires an extension and not the other? I have the feeling we could do without msgpackr extensions completely.

Obligatory "it works right now" so if you are in a hurry and want to merge feel free to do so, but this is my only outstanding concern.

paul-marechal · 2022-11-30T16:51:04Z

Lastly I also am not a fan of the class + static member way of creating JS singletons as it's more of a Java way. Why not just exposing a global function like msgpackr does?

tortmayr · 2022-11-30T17:56:37Z

It stinks that the mspackr API for extensions is global to the whole process, because it means we cannot define sets of extensions to use per connection either. Already it makes me not want to rely on extensions, but I understand that it's the way it currently works.

Totally agreed. This global extension mechanism is less than optimal. However, it's currently the only way to customize the encoding of certain types. We can try and raise this issue in the msgpackR repo. From what I could see up until now maintainers are quite open for feedback and suggestions so we might be able to get a fix for this in the upstream repo.
We should tackle this with a follow up issue.

2. I don't understand what's special with ResponseError that requires a global extension to be installed. If the rationale is that it's a commonly used type, then what about other recurring types like URI? What's the difference between those types, where one requires an extension and not the other? I have the feeling we could do without msgpackr extensions completel

The ResponseError is currently used in the proxy-factory to serialize and derserialize Application Errors. Some services have a special handling for these Application Errors (debug service, remote file system provider, task protocol ...). So we have to make sure the the type information for ResponseErrors doesn't get lost when sending the over the wire. Otherwise all services that rely on ApplicationErrors no longer work as expected.

I guess this dependency on ResponseErrors is a remainder from the original vscode-jsonrpc protocol and we should probably refactor the code to remove it. IMO this is also something that can easily be handled in a follow-up task. Nevertheless, I will take a look on it. If it can be removed rather easily without any side effects I will do it right away.

If the rationale is that it's a commonly used type, then what about other recurring types like URI? What's the difference between those types, where one requires an extension and not the other?

They do require a custom extension. We currently register custom extension at two places:
https://github.com/eclipsesource/theia/blob/7927546f39d411dd2e01a9af99be7add2d9a6859/packages/core/src/common/message-rpc/rpc-message-encoder.ts#L173 and https://github.com/eclipsesource/theia/blob/7927546f39d411dd2e01a9af99be7add2d9a6859/packages/plugin-ext/src/common/rpc-protocol.ts#L224

In a nutshell, we have three topics that require custom encoding/decoding which can currently only be done (efficiently) with msgpackr extensions:

Preserving error stack traces when sending them over the wire (see also Use binary message RPC protocol for plugin API #11261 (comment))
Preserve type information for RepsonseErrors to ensure that the error handling derived from the old json-rpc protocol doesn't break
The old plugin-rpc implementation had JSON replacers/revivers in place to handle the serialization of the common types you just mentioned (uri,vscode uri, range, BinaryBuffer). This is now also handled via msgpackR extensions.

Lastly I also am not a fan of the class + static member way of creating JS singletons as it's more of a Java way. Why not just exposing a global function like msgpackr does?

I agree, a global function is probably the lesser evil here.

tsmaeder · 2022-12-01T09:58:51Z

IMO, we're ready to merge this one:

msgpackr only supports global extensions. That sucks, but what can we do except fix msgpackr? Allow for per-instance extensions kriszyp/msgpackr#93
The rest of the objections seem purely style issues, and those should not hold up merging of a PR. It's up to @tortmayr, though. (https://github.com/eclipse-theia/theia/blob/master/doc/pull-requests.md#requesting-changes)

tortmayr · 2022-12-01T10:05:37Z

I would be in favor of merging this asap. This will give us the chance to fix potential bugs before the next release.
The rest of the objections can be easily addressed with a follow-up PR.

Refactors the plugin RPC protocol to make use of the new message-rpc introduced with eclipse-theia#11011/eclipse-theia#11228. - Refactor plugin-ext RpcProtocol API to reuse the new message-rpc protocol - Remove custom RPC message encoding and handling reuse message-rpc - Implement `BatchingChannel` that queues messages and sends them accumulated on the next process.tick (replaces the old Multiplexer implementation) - Refactors proxy handlers and remote target handlers - Use `Channel` instead of `MessageConnection` for creating new instances of RPCProtocol - Refactor `RpcMessageEncoder`/`RpcMessageDecoder` to enable overwritting of already registered value encoders/decoders. - Add mode property to base `RpcProtocol` to enable switching from a bidirectional RPC protocol to a client-only or server-only variant. - Implement special message encoders and decoders for the plugin communication. (Replacement for the old `ObjectTransferrer` JSON replacers/revivers) - Adapt `HostedPluginServer` and `HostedPluginClient` API to send/receive messages in binary format instead of strings. This enables direct writethrough of the binary messages received from the hosted plugin process. - Adapt `hosted-plugin-process` and `plugin-host` to directly send binary messages via `IpcChannel`/`BinaryMessagePipe` - Remove incorrect (and unused) notification proxy identifiers and instantiation - NotificationExt was instantiated in the main context - There were unused notification proxy identifiers for main and ext in the wrong contexts Part of eclipse-theia#10684 Fixes eclipse-theia#9514 Contributed on behalf of STMicroelectronics

tortmayr · 2022-12-01T12:09:36Z

@tsmaeder @paul-marechal @JonasHelming
I have now squashed the commit history and updated the changelog. PR should be ready to merge now.
I will tackle the issues that Paul raised in a follow up PR (#11941).

tortmayr force-pushed the plugin-rpc-v2 branch 2 times, most recently from 3a83b92 to 28009a3 Compare June 7, 2022 05:55

lucas-koehler mentioned this pull request Jun 7, 2022

Use binary message RPC protocol for plugins #11093

Closed

1 task

tortmayr force-pushed the plugin-rpc-v2 branch from 28009a3 to 0d32810 Compare June 9, 2022 07:10

tsmaeder reviewed Jun 9, 2022

View reviewed changes

packages/core/src/common/message-rpc/rpc-protocol.ts Outdated Show resolved Hide resolved

tsmaeder reviewed Jun 9, 2022

View reviewed changes

packages/core/src/common/message-rpc/rpc-protocol.ts Outdated Show resolved Hide resolved

tortmayr force-pushed the plugin-rpc-v2 branch from 0d32810 to d1fe95b Compare June 9, 2022 09:18

paul-marechal reviewed Jul 15, 2022

View reviewed changes

packages/plugin-ext/src/hosted/node/hosted-plugin-process.ts Show resolved Hide resolved

tortmayr mentioned this pull request Jul 27, 2022

File watching broken on Windows #11442

Closed

paul-marechal reviewed Aug 11, 2022

View reviewed changes

packages/plugin-ext/src/hosted/node/hosted-plugin-process.ts Outdated Show resolved Hide resolved

tortmayr force-pushed the plugin-rpc-v2 branch 2 times, most recently from cffeda8 to 768950c Compare November 1, 2022 23:37

kittaakos reviewed Nov 2, 2022

View reviewed changes

tortmayr requested a review from tsmaeder November 2, 2022 10:27

tsmaeder reviewed Nov 4, 2022

View reviewed changes

paul-marechal reviewed Nov 15, 2022

View reviewed changes

packages/plugin-ext/src/common/proxy-handler.ts Show resolved Hide resolved

packages/core/src/common/message-rpc/msg-pack-extension-manager.ts Show resolved Hide resolved

This was referenced Nov 16, 2022

Use binary message RPC protocol for plugin API eclipsesource/theia#42

Closed

[WIP] Plugin message-rpc eclipsesource/theia#38

Closed

tortmayr force-pushed the plugin-rpc-v2 branch 2 times, most recently from 63a9a11 to 25c7d1f Compare November 29, 2022 08:19

tsmaeder approved these changes Nov 29, 2022

View reviewed changes

tortmayr force-pushed the plugin-rpc-v2 branch from 7927546 to 2711928 Compare December 1, 2022 12:03

tortmayr mentioned this pull request Dec 1, 2022

Improve/Rework msgpackR extension integration #11941

Open

tsmaeder merged commit c8914a5 into eclipse-theia:master Dec 5, 2022

vince-fugnitto added this to the 1.33.0 milestone Dec 20, 2022

vince-fugnitto added the messaging issues related to messaging label Dec 20, 2022

This was referenced Feb 13, 2023

[Snyk] Upgrade @theia/editor from 1.31.0 to 1.33.0 magnologan/theia#8

Open

[Snyk] Upgrade @theia/workspace from 1.31.0 to 1.33.0 magnologan/theia#9

Open

[Snyk] Upgrade @theia/file-search from 1.31.0 to 1.33.0 magnologan/theia#10

Open

safisa mentioned this pull request Aug 21, 2023

Save confirmation popup does not save on custom binary editor #12844

Closed

		this.worker.onerror = e => onErrorEmitter.fire(e);

		this.worker.onerror = e => console.error(e);

Use binary message RPC protocol for plugin API #11261

Use binary message RPC protocol for plugin API #11261

Conversation

tortmayr commented Jun 6, 2022 • edited Loading

What it does

How to test

Review checklist

Reminder for reviewers

tortmayr commented Jun 6, 2022 • edited Loading

Performance Testing

Benchmarks

tsmaeder commented Jun 8, 2022

tsmaeder commented Jun 8, 2022

tsmaeder commented Jun 8, 2022

tortmayr commented Jun 9, 2022

tsmaeder commented Jun 9, 2022

tortmayr commented Jun 14, 2022

paul-marechal commented Jun 22, 2022

tsmaeder commented Jun 23, 2022

tortmayr commented Jun 23, 2022

paul-marechal commented Jun 23, 2022

paul-marechal commented Jul 28, 2022 • edited Loading

lucas-koehler commented Aug 16, 2022

tortmayr commented Nov 2, 2022

kittaakos Nov 2, 2022

Choose a reason for hiding this comment

tortmayr Nov 8, 2022

Choose a reason for hiding this comment

kittaakos Nov 2, 2022

Choose a reason for hiding this comment

tortmayr Nov 8, 2022

Choose a reason for hiding this comment

tsmaeder left a comment

Choose a reason for hiding this comment

tsmaeder commented Nov 4, 2022

tsmaeder commented Nov 21, 2022 • edited Loading

tortmayr commented Nov 28, 2022

tortmayr commented Nov 29, 2022

tsmaeder commented Nov 29, 2022

tortmayr commented Nov 29, 2022

tsmaeder commented Nov 29, 2022

tsmaeder left a comment

Choose a reason for hiding this comment

tsmaeder commented Nov 30, 2022

tortmayr commented Nov 30, 2022

Benchmarks

Browser

Electron

VS Code

paul-marechal commented Nov 30, 2022

paul-marechal commented Nov 30, 2022 • edited Loading

tortmayr commented Nov 30, 2022

tsmaeder commented Dec 1, 2022

tortmayr commented Dec 1, 2022

tortmayr commented Dec 1, 2022

tortmayr commented Jun 6, 2022 •

edited

Loading

tortmayr commented Jun 6, 2022 •

edited

Loading

paul-marechal commented Jul 28, 2022 •

edited

Loading

tsmaeder commented Nov 21, 2022 •

edited

Loading

paul-marechal commented Nov 30, 2022 •

edited

Loading