-
-
Notifications
You must be signed in to change notification settings - Fork 164
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
get rid of websockify #2121
Comments
2019-01-27 16:34:40: antoine uploaded file
|
First things first: there are a number of bugs in websockify upstream... (see patch) Maybe we should just do the work in our own websocket class? |
2019-01-28 16:48:49: antoine uploaded file
|
The patch above should help, it avoids making unnecessary memory copies of buffers larger than Each websocket header is just a few bytes with the packet size, since the primary purpose of zerocopy chunked packets is to optimize "paint" packets where the pixel data is sent as a separate chunk, we could also just generate the websocket header for both chunks and then submit them to the socket layer separately. The websocket frame already includes the packet size, so xpra's packet size header is a little bit redundant (and then there's also lz4's!) but adding a new packet format is a daunting task. See also #619. |
The reason why this new code did not work with the html5 client... was a bug in the html5 client network code, fixed in r21511. TODO:
|
Here's one of the improvements: number of packets per HTTP request. In all cases the browser's request looks like this:
With trunk:
With version 2.4.3:
The switch to websockets is also more concise. As for websocket traffic, we avoid extra headers, but I think we can still do better packet aggregation, either using NODELAY (#619) or maybe we need a more explicit CORK: When should I use TCP_NODELAY and when TCP_CORK? |
Packet aggregation issue from comment:5 is now solved: #2130. |
2019-02-02 08:17:49: mjharkin commented
|
2019-02-02 08:46:34: mjharkin commented
|
@mark Harkin: I don't get it, works for me with both python2 and python3:
|
2019-02-02 14:44:00: mjharkin commented
|
2019-02-02 16:21:22: antoine uploaded file
|
2019-02-04 04:32:18: mjharkin commented
|
It should work already.
Now you've lost me! Why would a client build need the html5 bits?
Yes, I will remove the websocket-client code completely so this issue should go away. |
2019-02-04 10:52:13: mjharkin commented
|
Still TODO:
|
2019-02-06 12:33:18: mjharkin commented
|
This was meant to be fixed, but there was a typo, fixed in r21552. There may be other bugs though, how to I reproduce fragmented frames? |
2019-02-06 13:33:52: mjharkin commented
|
Thanks for pointing that out, that's fixed in r21560. The problem was that since the logic for packet framing was moved from the socket layer to the protocol layer, we now need to use a different protocol class for websockets and the proxy code had not been updated. |
2019-02-06 17:52:49: mjharkin commented
|
After r21560, when connecting from either client I'm now getting the server-side error: Please post the exact commands you are using for testing. For reference, here's what I am using for testing:
|
r21570 makes the |
2019-02-07 19:34:54: mjharkin commented
|
That will be r21585. |
More testing done as part of #2139. This will do. |
Regression with python3: #2149 |
There was another bug in the legacy websocket framing code: #2793#comment:8. |
Avoid copying memory around, and support sending memoryviews as-is.
See #1926 and #2104.
sendmsg(data)
calls_sendmsg(0x2, data)
, which does this:We should be able to subclass it and override
_encode_hybi
to return multiple frames to send.flush
will need to be told how to handle lists of buffers.We can join the hiby header for payloads smaller than ~64K / 4K? (configurable)
TODO:
TCP_NODELAY
is being toggled as needed.The text was updated successfully, but these errors were encountered: