Merged
Conversation
On the plane I was reverse-engineering ipc.ts to implement it in Rust and see if we could have a "service mode" for the CLI that we could interact with like any other vscode process. In doing so, I noticed that numbers in the protocol--which are used at least twice in the message header and ID--were encoded as JSON. I was curious what benefits we'd get from encoding them as variable-length integers instead. It makes the message shorter, as expected. Encode/decode time are very, very slightly lower. I'm not sure it's worth the extra complexity, but I have included it here for your consideration.
Member
Author
|
hm, I had unit tests passing locally 😛 if there's interest I'll circle back and fix those up |
Member
|
👍 to doing this from my side. This particular byte-protocol is owned by @joaomoreno If you'd like to give it a try and optimize it too, there's a different one for the extension host here. |
Member
Author
|
Huh, I didn't realize that, I always assumed ipc.ts was the underlying transport. Do you recall why we have a different implementation there vs generic ipc? |
Member
|
I don't quite remember, we kept refactoring things. I think they might have ended up in a situation where rpcProtocol.ts is built on top of ipc.ts. |
rebornix
approved these changes
Dec 2, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On the plane I was reverse-engineering ipc.ts to implement it in Rust and see if we could have a "service mode" for the CLI that we could interact with like any other vscode process. (And maybe reuse it if we have more native processes in the future)
In doing so, I noticed that numbers in the protocol--which are used at least twice in the message header and ID--were encoded as JSON. I was curious what benefits we'd get from encoding them as variable-length integers instead.
It makes the message shorter, as expected. Encode/decode time are very, very slightly lower. I'm not sure it's worth the extra complexity, but I have included it here for your consideration.
Encode speed
Details
Decode speed
Details