[WIP] Integrate new message-rpc prototype into core messaging API (extensions) #10809

tortmayr · 2022-02-28T12:55:40Z

What it does

Refactors and improves the prototype of a faster JSON-RPC protocol intially contributed by @tsmaeder (See also Protoype of a faster RPC protocol #10781).
The encoding approach used in the initial POC has some performance drawbacks when encoding plain JSON objects. We refactored the protocol to improve the performance for JSON objects whilst maintaining the excellent performance for encoding objects that contain binary data. In addition, the error handling and lifecycle management is improved.
Integrates the new message RPC protocol into Theia's core messaging API and therfore replaces the previously used vscode-ws-jsonrpc protocol. This has major impacts and the Messaging API as we no longer expose a Connection object (which was provided by vscode-ws-jsonrpc) and directly rely on the generic Channel implementation instead.
Adapt the connection providers for the different connection types (Websocket, IPC, custom connections like TerminalWidget-> underlying process) to efficiently use the new RPC protocol. Efficiently in this case means that for some providers adoptions are necessary so that they can send messages as binary data instead of strings.

A performance test suite has been executed to mesaure the improvements gained by this change (See #10684 (comment) for more details).

Note: This is a WIP PR that is not ready to be reviewed yet and contains a lot of temporary hacks and workarounds (e.g. commenting out of .spec.ts files that have compilation errors). It is currently only working in the browser-context as the electron-ipc connection provider is not fully adopted to the new protocol yet.
It's intention is to present the current state of #10684 to interested parties.

Contributed on behalf of STMicroelectronics.
Closes #10684

How to test

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 · 2022-03-18T20:43:47Z

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

+    getWriteBuffer(): WriteBuffer {
+        const result = new ArrayBufferWriteBuffer();
+        const httpUrl = this.createHttpWebSocketUrl('/services');
+        if (this.useHttpFallback) {


The whole HTTP fallback shouldn't be required anymore?

We're using socket.io which should handle this natively.

Good catch. Yes this file is a remainder from the initial message-rpc PoC code drop and should be removed

Signed-off-by: Thomas Mäder <tmader@redhat.com>

…tensions) Integrates the new message-rpc prototype into the core messaging API (replacing vscode-ws-jsonrpc). This has major impacts and the Messaging API as we no longer expose a `Connection` object (which was provided by vscode-ws-jsonrpc) and directly rely on the generic `Channel` implementation instead. Note: This is a WIP PR that is not ready to be reviewed yet and contains a lot of temporary hacks and workarounds (e.g. commenting out of *.spec.ts* files that have compilation errors) and is currently only working in browser-applications. It's intention is to present the current state of eclipse-theia#10684 to interested parties. Contributed on behalf of STMicroelectronics. Closes eclipse-theia#10684

- Refactor the message encoders of the new message-rpc protocol to improve overall performance. - Align main websocket creation: Implement `ẀebSocketMainChannel` class which is responsible for establishing the main websocket connection between frontend and backend. Use a `IWebSocket` wrapper so that the class can be reused in the frontend and backend runtimes and is independent of the actual library used for creating the websocket. - Improve rpc protocol error handling. In particular ensure that remote call errors are properly decoded to `ResponseErrors`. - Ensure that the `RemoteFileSystemProvider` API uses Uint8Arrays over plain number arrays. This enables direct serialization as buffers and reduces the overhead of unnecessarily converting from and to Uint8 arrays. - Refactor terminal widget and terminal backend contribution so that the widgets communicates with the underlying terminal process using the new rpc protocol. - Rework the IPC bootstrap protocol so that it uses a binary pipe for message transmission instead of the `ipc` pipe which only supports string encoding. Note: This is a WIP PR that is not ready to be reviewed yet as some concepts are not cleaned up yet or fully implemented (e.g. electron support & debug adapter protocol support) It's intention is to present the current state of eclipse-theia#10684 to interested parties. Contributed on behalf of STMicroelectronics.

tsmaeder · 2022-04-04T08:03:30Z

@tortmayr still WIP?

tortmayr · 2022-04-04T08:23:13Z

@tsmaeder Yes, still a little bit of code cleanup to do and testing to ensure that everything works as expected electron.
We will provide an update soon that should make the PR ready-to-review

paul-marechal · 2022-04-05T02:52:17Z

@tortmayr do you mind joining the technical meeting on Wednesday in order to sync up our respective WIPs?

I have been re-writing a few core APIs related to messaging, and I'd like to go over some of those changes with you to see if anything needs to be changed either here or on my side.

tortmayr · 2022-04-05T06:38:11Z

@tortmayr do you mind joining the technical meeting on Wednesday in order to sync up our respective WIPs?

I have been re-writing a few core APIs related to messaging, and I'd like to go over some of those changes with you to see if anything needs to be changed either here or on my side.

@paul-marechal No at all, I'm happy to join the technical meeting on Wednesday. Could provide me with a link to your WIP so that I can have a look at it beforehand?

paul-marechal · 2022-04-06T03:13:16Z

@tortmayr here's the very wip branch. It builds but doesn't run yet.

I rewrote our proxies to hook themselves to some RpcConnection interface which abstracts away what it takes to do a RPC call:

e8b5605#diff-0e651b8d4ece117f58140576d68cb6a86b4ed1cb2c52057850b8dae52045c935R28-R34

Whenever you do proxy.someMethod(arg1, arg2) it will actually go through rpcConnection.sendRequest('someMethod', arg1, arg2). I was wondering if your new RPC implementation could fit in that abstraction, or if something prevents doing so.

I want to go over some other details during the call and see if some of the other abstractions will benefit you as well or not.

tortmayr · 2022-04-12T05:49:26Z

This WIP PR has served its purpose as I now have opened the actual PR: #11011

tortmayr force-pushed the wip-json-rpc-performance branch from d85ec88 to c5dc1b3 Compare February 28, 2022 13:15

tortmayr changed the title ~~Wip json rpc performance~~ [WIP] Integrate new message-rpc prototype into core messaging API (extensions) Feb 28, 2022

tortmayr force-pushed the wip-json-rpc-performance branch from c5dc1b3 to 82e6f2c Compare February 28, 2022 13:17

tortmayr mentioned this pull request Feb 28, 2022

Improve performance of JSON-RPC communication #10684

Closed

5 tasks

paul-marechal reviewed Mar 18, 2022

View reviewed changes

tsmaeder and others added 2 commits April 1, 2022 13:32

Protoype of a faster RPC protocol

5b16961

Signed-off-by: Thomas Mäder <tmader@redhat.com>

tortmayr force-pushed the wip-json-rpc-performance branch 3 times, most recently from 53f0198 to ff7f4f8 Compare April 1, 2022 14:10

tortmayr force-pushed the wip-json-rpc-performance branch from ff7f4f8 to 6bcf872 Compare April 1, 2022 14:25

tortmayr closed this Apr 12, 2022

tortmayr mentioned this pull request Apr 12, 2022

Integrate new message-rpc prototype into core messaging API (extensions) #11011

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Integrate new message-rpc prototype into core messaging API (extensions) #10809

[WIP] Integrate new message-rpc prototype into core messaging API (extensions) #10809

tortmayr commented Feb 28, 2022 •

edited

Loading

paul-marechal Mar 18, 2022

tortmayr Apr 1, 2022 •

edited

Loading

tsmaeder commented Apr 4, 2022

tortmayr commented Apr 4, 2022

paul-marechal commented Apr 5, 2022

tortmayr commented Apr 5, 2022 •

edited

Loading

paul-marechal commented Apr 6, 2022

tortmayr commented Apr 12, 2022

[WIP] Integrate new message-rpc prototype into core messaging API (extensions) #10809

[WIP] Integrate new message-rpc prototype into core messaging API (extensions) #10809

Conversation

tortmayr commented Feb 28, 2022 • edited Loading

What it does

How to test

Review checklist

Reminder for reviewers

paul-marechal Mar 18, 2022

Choose a reason for hiding this comment

tortmayr Apr 1, 2022 • edited Loading

Choose a reason for hiding this comment

tsmaeder commented Apr 4, 2022

tortmayr commented Apr 4, 2022

paul-marechal commented Apr 5, 2022

tortmayr commented Apr 5, 2022 • edited Loading

paul-marechal commented Apr 6, 2022

tortmayr commented Apr 12, 2022

tortmayr commented Feb 28, 2022 •

edited

Loading

tortmayr Apr 1, 2022 •

edited

Loading

tortmayr commented Apr 5, 2022 •

edited

Loading