WebSocket unmask speedup #579

Closed
wants to merge 5 commits into from

2 participants

@fancycode

Added optimized C-level function to mask/unmask WebSocket frames including a very simple testcase.

@bdarnell
tornadoweb member

I have mixed feelings about this change - C modules add a lot of complexity and room for subtle failures, and I've always been fond of the fact that you can just add the tornado directory to your path and it works with no build (on py26/py27 at least). On the other hand, a python-level loop over all the characters in a string is pretty bad and probably worth doing something about.

Do you have benchmark numbers for this change? You can use the autobahn scripts in maint/test/websocket. Please test with pypy, since in pypy C extensions can sometimes make things slower by disabling JIT in the surrounding code.

The test should include cases where some/all bytes of the key and data have the high bit set, to make sure there aren't signedness problems in the C code. There should also be some null bytes to make sure it's always using the length.

It looks like you're assuming that sizeof(unsigned int)==4; this should be changed to explicitly use a 32-bit type.

I don't think PyString_AS_STRING guarantees proper alignment for use as an int*, so you may need to allow for unaligned data at both the beginning and end of the string.

If we're going to introduce C modules we'll probably want to add other functions in the future, so it might be best to have a generic tornado/_speedups.c file to contain them all.

@bdarnell
tornadoweb member

I've just checked in a cython-based implementation of this idea: e8dc5e4#diff-2

@bdarnell
tornadoweb member

This was released in Tornado 3.2.

@bdarnell bdarnell closed this Jan 18, 2014
@mvollrath mvollrath referenced this pull request in RobotWebTools/rosbridge_suite Oct 19, 2014
Closed

Slow web socket performance #135

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment