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

[tortmayr]

What it does

Refactors and improves the prototype of a faster JSON-RPC protocol initially contributed by @tsmaeder (See also #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.

Integrates the new message-rpc prototype into the core messaging API (replacing vscode-ws-jsonrpc).
This has major impacts on the Messaging API as we no longer expose a Connection object (which was provided by vscode-ws-jsonrpc) and directly rely on a generic transport Channel implementation instead.

• Introduce Channel as the main transport concept for messages (instead of the dedicated Connection from vscode-jsonrpc)
• Replace usage of vscode-ws-jsonrpc with a custom binary RPC protocol.
• Refactor all connection providers to use the new binary protocol.
• 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 Uint8Arrays.
• 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.
• Extend the JsonRpcProxyFactory with an optional RpcConnectionFactory that enables adopter to creates proxies with a that use a custom RpcProtocol/RpcConnection.

The plugin API still uses its own RPC protocol implementation. Currently we have to encode/decode between binary data to handle RPC calls from a plugin context. Aligning the two protocols and zero-copy tunneling of RPC messages is planned for a follow-up PR.

Contributed on behalf of STMicroelectronics.
Closes #10684

How to test

RPC messaging is a central concept in Theia and is heavily used across all services. The change is already implicitly tested
by many of the existing unit and API tests. For manual testing one should ensure that the different RPC connection types
(websocket, IPC, electron IPC etc.) work as expected.

• Websocket connections: can be tested with any arbitrary service e.g the FileService
• TerminalWidget-TerminalProcess connection: can be tested with the terminal widget
• IPC connections: Can be tested with the FileWatchServer
• Electron IPC: can be tested in the same fashion as websocket connections

In addition, we executed a set of performance benchmarks to measure the improvements in regards to handling large binary messages (see also #10684 (comment))

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

Co-authored-by: Thomas Mäder tmader@redhat.com

[tortmayr]

@tsmaeder @paul-marechal
As discussed in the WIP draft (#10809) and the last technical meeting we
have now cleaned up our proposal for the new messaging RPC protocol and consider it ready to be reviewed.

Since this is already quite a large and impactfull change we decided to split the implementation of ##10684 into two PRs. This PR focuses on integrating the new RPC protocol into the core messaging API but does not include any direct improvements or changes to the plugin RPC communication. We will prepare a follow-up PR that aligns the plugin RPC protocol with the new message RPC protocol which also should enable zero-copy message tunneling. We expect that this will even further increase the performance for plugin RPC communication.

@paul-marechal As discussed last week, I'm happy to test how well/easy the new message RPC protocol can be integrated with your planned refactoring of the whole Proxy/Messaging API. Just let me know once you have a PR proposal ready.

[tortmayr]

@tsmaeder As already discussed here your initially proposed encoding approach had a couple of performance limitations for plain json messages.
I implemented some of our proposed improvements:

• Dynamically use one or more bytes to encode the tags of value encoder/decoders depending on the number of encoders
• Use dynamic encoding for length values of string and arrays. Instead of always using Uint32 which potentially allocates a lof o unnecessary space we use a composed approach. The length value is encoded with the best suiting representation (i.e. Uint8 for values <256, Uint16 for values <=65536, Uint32 otherwise) and the first byte indicates which Uint format is used.

In the initial proposal each object was encoded/decoded individually which resulted in very high encoding times compared to just encoding the object with JSON.stringify. After some experimentation I ended up using the following approach:

The default encoding approach for all objects is JSON.stringify. For some object types (e.g. binary buffers or errors) there are custom encoders registered. In addition, we have a special handling for arrays.
For an array we check whether it can be encoded in one go with JSON.stringify or if its elements need to be encoded separately. An array can only be encoded in one go if it does not contain a binary buffer.
This approach seems to work really well and seems to be the best trade off between performance and efficient encoding.
VSCode is using a similar approach in their custom RPC protocol. The only downside to this approach is that we currently don't check for nested buffers in arrays (for obvious performance reasons). This means if an array contains a nested buffer it will be encoded with the inferior JSON.stringify. However, in practice I could not observe any message service that uses nested buffers. VSCode seems to handle these corner case with a special SerializedObjectWithBuffers wrapper class to mark such nested buffers. If needed we could introduce a similar concept.

[tsmaeder]

Thanks for this PR, Tobias. My main objection at this time is about how we encode the byte arrays, I think that needs to be looked at.

[tsmaeder]

Am I correct that we're not yet using message-rpc for the plugin main/ext stuff? Is there a followup-PR planned?

[tortmayr]

Thanks for the detailed feedback @tsmaeder.

Am I correct that we're not yet using message-rpc for the plugin main/ext stuff? Is there a followup-PR planned?

Yes that is correct. We are currently working on the follow-up PR right now that introduces message-rpc for the plugin main/ext stuff. Since this PR is already quite large we decided to make a clean cut and handle plugin API related RPC improvements in a follow up PR.

[tsmaeder]

@tortmayr I did a couple of experiments, and I was getting the encoding performance to within 2.5 * JSON.stringify() for the rather complex object in the test case you created. I'll have more on this Monday.

[tsmaeder]

@tortmayr you can see my latest optimizations in this commit: tsmaeder/message-rpc@18ff9e2

The most important changes are:

1. Encode all primitive types separately: integer, boolean, string, etc.
2. ArrayBuffer(Read|Write)Buffer: Use UInt8Array instead of ArrayBuffer=> this allows us to not create UInt8Array instances all the time. Do the same for the DataView
3. Use TextEncoder.encodeInto() instead of encode()
4. Preallocate a single TextEncoder

[tortmayr]

Thanks @tsmaeder I will have a look at the optimizations right away

[paul-marechal]

@tsmaeder regarding a previous discussion we had about what to send over our frontend-backend connection: It seems like my guess was correct and Socket.io will efficiently serialize buffers parts of regular JS objects: https://socket.io/blog/introducing-socket-io-1-0/#binary-support

What this means is that we don't have to send a buffer directly to socket.emit('message', buffer): We could have the buffer wrapped like socket.emit('message', { someField: 'abc', message: buffer }) and Socket.io automagically optimizes the serialization of the structure with the buffer. How precisely I don't know, but I would assume the cost is negligible compared to directly sending it unwrapped.

Though this only applies to the Socket.io transport between frontend and backend, Electron or Node IPC might behave differently.

[tortmayr]

@tsmaeder I have rebased the PR, incorporated your review feedback and applied your performance improvements.
I did not update the message encoder as part of this change as I'm still experiencing some errors with the new and improved (or rather the original) message encoder. I plan to fix the remaining issues tomorrow, and then I will integrate the improved encoder.
Nevertheless I already did some performance tests with some more "realistic" large json messages.

The largest plain message I encountered during Theia startup is the result of the HostedPluginServer.getDeployedPlugins request. It contains the metadata of all plugins and is about 8 mb large. Even for such a large JSON structure I can can confirm the 2.5x performance compared to JSON.stringify. However, what worries me is that the decoding performance seems to be rather around the factor 5-6. I'm wondering whether this has negative impacts on the overall startup performance.

[tsmaeder]

@tsmaeder regarding a previous discussion we had about what to send over our frontend-backend connection: It seems like my guess was correct and Socket.io will efficiently serialize buffers parts of regular JS objects: https://socket.io/blog/introducing-socket-io-1-0/#binary-support

If we can offload the efficient encoding/decoding of complex values to a library (be it socket.io or something else), I'm all for it. However, I would offer some caveats:

1. We don't just have back end/front end socket comms, but also IPC, Electron, and forwarded plugin<=>front end comms. How would using socket.io address those use cases?
2. We'd need to test that what socket.io uses is really in the ballpark of what VS Code offers for send/receive speed

The code that socket.io uses lives here seem they are using an approach similar to what VS Code uses (extract buffers from message object and send them separately from the stringified message structure.

[tsmaeder]

@tsmaeder I have rebased the PR, incorporated your review feedback and applied your performance improvements. I did not update the message encoder as part of this change as I'm still experiencing some errors with the new and improved (or rather the original) message encoder. I plan to fix the remaining issues tomorrow, and then I will integrate the improved encoder.

Glad to see you back on deck. I'll have a look when ready.

Nevertheless I already did some performance tests with some more "realistic" large json messages.

The largest plain message I encountered during Theia startup is the result of the HostedPluginServer.getDeployedPlugins request. It contains the metadata of all plugins and is about 8 mb large. Even for such a large JSON structure I can can confirm the 2.5x performance compared to JSON.stringify. However, what worries me is that the decoding performance seems to be rather around the factor 5-6. I'm wondering whether this has negative impacts on the overall startup performance.

I haven't looked at decoding in any depth. If we apply the same "tricks" (aka not allocating objects or copying data, mostly), I could imagine we get some improvements.

[tsmaeder]

@paul-marechal @tortmayr @JonasHelming I think one of the things Paul disagrees with is the concept of a Channel as something the can send/receive buffers (@paul-marechal you correct me if I'm wrong). He would like a Channel to send/receive objects and the endoding/decoding of said object to be an implementation detail of the Channel implementation (the encoding implementation could still be shared code between the use cases).
In theory, I like this concept better than what we have with the buffer right now, for example, multiplexing or tunneling becomes trivial: you just wrap the object to send in another object containing routing information.
However, there are a couple of things that we need to think about:

1. Some objects need application-specific pre-processing (dehydrate/revive) before sending over the wire. For example, VS Code URIs cannot just be encoded from their property structure. Same with object that have getter/setter functions. Such processing is currently done via JSON.stringify() object replacers. We would need to offer something similar
2. With the object-channel approach, we need to load any object into memory in it's entirety. With the "buffers approach", we could do stuff like

   while (part= readFromFile(...)) {
      buffer.write(part);
   }
   

   Do we care about this? Can you think of a way to do this with object channels (maybe a special object (ByteSource) we can special-case?
3. API change path: We need to think about how we move forward with these changes: what's the API breakage for clients and how many times should we break API? I would prefer if we can land this PR first and do further refactorings in follow-ups, but only if it is acceptable for adopters. The relevant thing here is releases. I don't care about breakage withing a release cycle.

[JonasHelming]

@paul-marechal and @tsmaeder : Thanks lot for the review feedback. One comment, especially about the last point of Thomas:
I would aim at getting this PR in as soon as we are convinced it is stable and a significant improvement. I would then rather create follow-up tickets for improvements we identify. The reasoning for this is:

• We are still in the beginning of a release cycle giving us some time to stabilize if necessary and to fix follow ups
• There is a follow-up for plugins (Use binary message RPC protocol for plugins #11093)
• There is Pauls refactoring, which I really do not want to block any further if possible (refactor core messaging and RPC (electron-only so far) #11076)

Ideally we achieve the performance improvement and the API refactoring in one release cycle resulting in a faster and much easier to use solution. So let's keep this PR as minimal as possible.

[tortmayr]

@tsmaeder The PR is now ready for to be re-reviewed. 😉 The message encoder is now based on the version of your performance optimizations.

I did a few runs of the startup performance scripts and could not notice any major performance regression compared to the master or the older version of the message encoder. So that's great 😄

I haven't looked at decoding in any depth. If we apply the same "tricks" (aka not allocating objects or copying data, mostly), I could imagine we get some improvements.

I don't think there is much room for improvement here. The decoding is mostly just reading elements from the uint8 array (so there is not much object allocation or copying data involved). However, I also preallocated a single TextDecoder instance which at least slightly increased the performance.

[paul-marechal]

1. Some objects need application-specific pre-processing (dehydrate/revive) before sending over the wire. For example, VS Code URIs cannot just be encoded from their property structure. Same with object that have getter/setter functions. Such processing is currently done via JSON.stringify() object replacers. We would need to offer something similar

We can always convert the original type into the "dehydrated" type before sending it, and vice versa.

2. With the object-channel approach, we need to load any object into memory in it's entirety. With the "buffers approach", we could do stuff like [pass through writing]

This is why I think this new RPC protocol has value: it seems to do a good job at serializing binary data, and it "hides" the serialization step from the transport.

e.g. Socket.io won't try to stringify the object on send and "unstringify" it on reception. Instead it will only efficiently transfer the buffer, and it is upon us on reception to decide when to eat the deserialization cost.

That's pretty cool though I don't know the exact savings we get from that. From my understanding, most of the performance issues were caused by the previous idea that we had to send strings over our connections, so we'd inefficiently encode buffers into one big string, then pass it to Socket.io which would inefficiently send the big string over, then we'd inefficiently deserialize that string before maybe re-serializing it to route it elsewhere...

3. API change path: We need to think about how we move forward with these changes: what's the API breakage for clients and how many times should we break API? I would prefer if we can land this PR first and do further refactorings in follow-ups, but only if it is acceptable for adopters. The relevant thing here is releases. I don't care about breakage withing a release cycle.

If I understand correctly this change does two things:

1. Implement a new serializer.
2. Replace JSON-RPC by a custom RPC protocol using the new serializer.

I don't think a lot of APIs need to be broken for that. @tortmayr I don't see anything under the changelog's breakages, does that mean nothing got broken?