Skip to content
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

Rewrite with compression support #163

Merged
merged 56 commits into from
Feb 16, 2020
Merged

Rewrite with compression support #163

merged 56 commits into from
Feb 16, 2020

Conversation

nhooyr
Copy link
Contributor

@nhooyr nhooyr commented Oct 15, 2019

@nhooyr
Copy link
Contributor Author

nhooyr commented Oct 15, 2019

Memory usage is still a problem but I think this feature can still be useful in some cases and it is standard so might as well implement it given it doesn't add too much to the API.

@nhooyr
Copy link
Contributor Author

nhooyr commented Oct 15, 2019

Thoughts on the API? @andersfylling @coadler @FZambia

@coder coder deleted a comment from github-actions bot Oct 15, 2019
@nhooyr nhooyr changed the base branch from master to compress2 October 15, 2019 01:55
@nhooyr nhooyr changed the title Implement RFC 7692 WebSocket Compression Extensions Implement RFC 7692 WebSocket Compression Extensions PR 1 Oct 15, 2019
@nhooyr nhooyr changed the title Implement RFC 7692 WebSocket Compression Extensions PR 1 RFC 7692 WebSocket Compression Extensions PR 1 Oct 15, 2019
@nhooyr nhooyr changed the title RFC 7692 WebSocket Compression Extensions PR 1 RFC 7692 WebSocket Compression Extensions [1] Oct 15, 2019
@andersfylling
Copy link

It's not relevant to my use-cases as I use gzip, I think(?). But looks easy enough to use.

I would add a validation step on Treshold and Level field.

@nhooyr
Copy link
Contributor Author

nhooyr commented Oct 23, 2019

Sounds good.

Only issue I could forsee with this API is you can't control per message compression which I imagine is something people will want control over. But I'd have to break backwards compat and add a writer options structure to allow for that.

Or maybe just a way to disable compression for an individual message (that may already have some other form of compression, e.g. a JPEG image) but keep the level and threshold global to the connection.

🤔

@nhooyr
Copy link
Contributor Author

nhooyr commented Nov 9, 2019

Also, I noticed Facebook uses per message deflate for their chat, which has very small message sizes. Could either be a oversight or an optimization, not sure. 🤔

@nhooyr nhooyr changed the title RFC 7692 WebSocket Compression Extensions [1] RFC 7692 WebSocket Compression Extensions Nov 10, 2019
@nhooyr nhooyr changed the base branch from compress2 to master November 10, 2019 02:39
@nhooyr nhooyr force-pushed the compress branch 3 times, most recently from 14baab4 to 2a9e139 Compare November 12, 2019 02:14
@nhooyr nhooyr merged commit b961007 into master Feb 16, 2020
@nhooyr nhooyr deleted the compress branch February 16, 2020 02:56
@nhooyr nhooyr mentioned this pull request May 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants