Skip to content

Investigate compressing TS Server socket communication #43156

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mjbvz opened this issue Mar 9, 2021 · 8 comments
Open

Investigate compressing TS Server socket communication #43156

mjbvz opened this issue Mar 9, 2021 · 8 comments
Labels
Committed The team has roadmapped this issue Suggestion An idea for TypeScript

Comments

@mjbvz
Copy link
Contributor

mjbvz commented Mar 9, 2021

Problem

On desktop, TS Server uses a json based protocol. This protocol up being quite verbose for larger documents, especially for requests such as navTree which return results from the entire file

Just opening checker.ts for example results in individual requests and responses that are a few MB in size.

Proposal

We've seen up to a 10x reduction of on the wire data by enabling websocket per-message compression for VS Code's internal communications. We aren't using web sockets here, but I believe we should be able to fairly easily compress individual messages on the server side and decompress them on the editor side. Most importantly, we could enable this without any changes to the json protocol itself.

For backwards compatibility, this would have to be an opt-in feature

@RyanCavanaugh RyanCavanaugh added Committed The team has roadmapped this issue Suggestion An idea for TypeScript labels Mar 9, 2021
@RyanCavanaugh
Copy link
Member

@mjbvz let's discuss more next Wednesday.

@mjbvz
Copy link
Contributor Author

mjbvz commented Mar 9, 2021

Sounds good. Just a few quick notes from an initial investigation :

  • For small messages, the savings by compressing the message body using deflate are minimal. For example going from 76 bytes to 68 bytes. However for large messages, the savings are much better, with a quick example I pulled up going from 14529 to 1884 (7x size reduction)

  • From my (limited) understanding, if we used gzip for compression we could avoid using the content-length on wire header entirely since gzip already includes its own header. This would simplify parsing on the client side, although I haven't found good node examples of reading multiple gzip compressed messages from a stream

@dbaeumer Does the LSP use compression? We would like to align with the LSP if possible here

@amcasey
Copy link
Member

amcasey commented Mar 10, 2021

I wonder if there's an intermediate approach where only the body property of the response is compressed? That might make it easier for the server to decide per-message whether compression is worthwhile (assuming the client has opted in via user-preference).

@dbaeumer
Copy link
Member

@mjbvz JSONRPC does support compression and even a different encoding (for example MessagePack). But this currently needs to be setup on the client and server side when the connection is created. The LSP spec does currently not have support to upgrade a connection from an uncompressed to a compressed state.

@dbaeumer
Copy link
Member

To clarify: compression and encoding is pluggable. So you can choose whatever you want as long as client and server do the same :-)

@Kingwl
Copy link
Contributor

Kingwl commented Apr 1, 2021

I wonder if there's an intermediate approach where only the body property of the response is compressed?

As far as I know. We may have to apply base64 encode. but that will increase (about) 1/3 size.

I thought we may just add some header (As Content-Length) like Content-Encoding ?

@dbaeumer
Copy link
Member

dbaeumer commented Apr 1, 2021

@Kingwl I looked into that approach in LSP / JSONRPC and it makes things complicated with notification. In addition Content-Encoding header value in the response would require a Accept-Encoding header in the request. Otherwise the server doesn't know which content encoding the client understands.

For LSP I decided to propose the following route: during initialization handshake the client & server agree on a specific encoding. After the handshake the connection will be upgraded to that encoding or left at the default if no agreement could be reached.

@Kingwl
Copy link
Contributor

Kingwl commented Apr 1, 2021

would require a Accept-Encoding header in the request.

Seems we are repeating http protocol. :XD.

Another problem is TypeScript does not have external dependency (If i'm correct). So will we add something (for gzip) dependency, or just implement yet another compressor?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Committed The team has roadmapped this issue Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants