Skip to content

Support compression - #299

Merged
rauchg merged 2 commits into
socketio:masterfrom
nkzawa:compression
Jan 21, 2015
Merged

rauchg merged 2 commits into
socketio:masterfrom
nkzawa:compression

Conversation

@nkzawa

@nkzawa nkzawa commented Dec 22, 2014

Copy link
Copy Markdown
Contributor

Add the permessage-deflate extension support for WebSocket, and http compression for polling. They are enabled by default.
Also you can disable the compression per message through the option of send method.

Related: socketio/socket.io#1148, #298 #271

@nkzawa

nkzawa commented Dec 22, 2014

Copy link
Copy Markdown
Contributor Author

It seems we should have a minimum size threshold for http compression, since compressing small data makes them larger.

@leearmstrong

Copy link
Copy Markdown

When will this make it into the live SocketIO code? Anyway to use it now?

@defunctzombie

Copy link
Copy Markdown
Contributor

@nkzawa Why wouldn't we just let the proxy handle this?

@nkzawa

nkzawa commented Dec 31, 2014

Copy link
Copy Markdown
Contributor Author

@defunctzombie mainly because we want to turn the compression on/off per message(payload) to emulate the permessage-deflate extension of WebSocket. I think we can't support this with proxy.
And also we can't use Content-Type header to decide whether to compress data or not like a http server.

@3rd-Eden

Copy link
Copy Markdown
Contributor

@defunctzombie you can still have your proxy handle this by turning compression off in engine.io right?

@nkzawa

nkzawa commented Jan 4, 2015

Copy link
Copy Markdown
Contributor Author

@lpinca updated, thx!

Comment thread README.md Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo

@leearmstrong

Copy link
Copy Markdown

Any ETA when this will be merged. This could be huge!!

@nkzawa

nkzawa commented Jan 5, 2015

Copy link
Copy Markdown
Contributor Author

@Leeus I'm working to reflect code review. Maybe this will be merged after that if there is no problem anymore.

@nkzawa

nkzawa commented Jan 5, 2015

Copy link
Copy Markdown
Contributor Author

@rauchg fixed and rebased. could you check again?

@rauchg

rauchg commented Jan 5, 2015

Copy link
Copy Markdown
Contributor

When we call sendPacket manually, should the compress flag be undefined so that it obtains it from the default provided by the configuration in the constructor?

Otherwise, it seems that if we disable compression globally, it'll still force to true when we send non-user packets (like noop)

@rauchg

rauchg commented Jan 5, 2015

Copy link
Copy Markdown
Contributor

Before, it seems that it was always defaulting to compress: true if it was undefined?

@nkzawa

nkzawa commented Jan 5, 2015

Copy link
Copy Markdown
Contributor Author

compress: true for sendPacket(send) is just one of the conditions, so it's ignored when compression is disabled globally and when client doesn't support compression.

@defunctzombie defunctzombie mentioned this pull request Jan 9, 2015
@lpinca lpinca mentioned this pull request Jan 17, 2015
@rauchg

rauchg commented Jan 19, 2015

Copy link
Copy Markdown
Contributor

@nkzawa please rebase so I can merge.

@nkzawa

nkzawa commented Jan 21, 2015

Copy link
Copy Markdown
Contributor Author

@rauchg rebased

rauchg added a commit that referenced this pull request Jan 21, 2015
@rauchg
rauchg merged commit ddc64a2 into socketio:master Jan 21, 2015
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.

6 participants