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

evaluate TCP_NODELAY #514

Closed
totaam opened this issue Feb 9, 2014 · 12 comments
Closed

evaluate TCP_NODELAY #514

totaam opened this issue Feb 9, 2014 · 12 comments

Comments

@totaam
Copy link
Collaborator

totaam commented Feb 9, 2014

Issue migrated from trac ticket # 514

component: client | priority: minor | resolution: fixed

2014-02-09 11:54:24: antoine created the issue


From How do I use TCP_NODELAY?: TCP_NODELAY is for a specific purpose; to disable the Nagle buffering algorithm. It should only be set for applications that send frequent small bursts of information without getting an immediate response, where timely delivery of data is required (the canonical example is mouse movements).

Some background reading: TCP/IP options for high-performance data transmission

I think that the client could benefit from this as it only sends small packets anyway, and those tend to require low latency (damage ACK, pings, etc). It should be investigated, and maybe it should be the default or at least be an option for certain usage scenarios. (like games?)

@totaam
Copy link
Collaborator Author

totaam commented Feb 9, 2014

2014-02-09 11:54:51: antoine uploaded file socket-NODELAY.patch (0.5 KiB)

adds the NODELAY option to the client socket connection in TCP mode

@totaam
Copy link
Collaborator Author

totaam commented Feb 9, 2014

2014-02-09 12:05:14: antoine edited the issue description

@totaam
Copy link
Collaborator Author

totaam commented Feb 19, 2014

2014-02-19 17:22:56: markh commented


Out of curiosity (whether xpra was using these flags) I was searching on this topic and came to this ticket.

I wanted to chime in and say that if xpra isn't using nodelay/nowait then, yes, I think from previous experience (of implementing servers) that this could make a massive difference.

Also, the use of TCP_CORK as useful to temporarily disable this; for example to send a header followed by the data payload.

However, we're currently enjoying the ease of xpra over SSH. And it's here that, I presume, that a caller to SSH cannot provoke it to send data in this way. (I wonder if some terminal code or option can be used)

@totaam
Copy link
Collaborator Author

totaam commented Feb 24, 2014

2014-02-24 16:31:51: markh commented


I tested the patch above, with xpra 0.9.8 and a TCP connection.

It did not break anything. It feels better, but I did not do a 'blind' test.
It's not significantly better (in my case I have 82ms round trip, yet mouse clicks still feel to take approx 0.5 seconds to register in a QT application)

It looks to me like the patch only affects the upstream connection, and should maybe be applied to the server-side as well, but would be different given there is more data involved.

@totaam
Copy link
Collaborator Author

totaam commented Feb 25, 2014

2014-02-25 02:42:39: totaam commented


I have merged the NODELAY stuff for both client and server in r5572, defaults to on.
It will make it easier to test:

XPRA_TCP_NODELAY=0 xpra ...

I will try to run the full automated tests both with and without and see what the differences are, if any. smo: feel free to beat me to it.


markh:

I have 82ms round trip, yet mouse clicks still feel to take approx 0.5 seconds
[[BR]]
Please post "xpra info" for your session, sounds like something is wrong.
[[BR]]

But first the obvious: you are using an old unsupported version with known bugs... (probably stuck with the gentoo ebuilds or something like that?)

@totaam
Copy link
Collaborator Author

totaam commented Feb 25, 2014

2014-02-25 11:48:19: markh commented


We're using an older version as this is a production system with many users, so we tend to be pretty conservative.

I'm set the wheels in motion of testing and upgrading the production system to 0.11.3.

I can still do small localised testing. I had some problems with compatibility between 0.9 and 0.10 versions.

Thanks

@totaam
Copy link
Collaborator Author

totaam commented Feb 28, 2014

2014-02-28 00:45:42: totaam uploaded file delay-vs-nodelay.csv (65.7 KiB)

performance tests results

@totaam
Copy link
Collaborator Author

totaam commented Feb 28, 2014

2014-02-28 00:49:39: totaam commented


I left the automated tests running overnight and the results are in, see attachment above and the pretty graphs here: NODELAY.

[[BR]]

Overall, it seems to make a very noticeable difference to the client and server latency, but not much in terms of anything else. Only mmap encoding suffered slightly in some cases, and also some of the pathological tests (ie: gtkperf) though it was actually better in some cases. Some tests were discarded from the graphs because of incomplete data with some encodings (and the graphs only show averages).

@totaam
Copy link
Collaborator Author

totaam commented Mar 18, 2014

2014-03-18 14:27:30: totaam changed status from new to closed

@totaam
Copy link
Collaborator Author

totaam commented Mar 18, 2014

2014-03-18 14:27:30: totaam changed resolution from ** to fixed

@totaam
Copy link
Collaborator Author

totaam commented Mar 18, 2014

2014-03-18 14:27:30: totaam commented


Done, closing.

markh:

I had some problems with compatibility between 0.9 and 0.10 versions.

Please report them so they can get fixed.

@totaam totaam closed this as completed Mar 18, 2014
@totaam
Copy link
Collaborator Author

totaam commented Jan 28, 2019

2019-01-28 16:57:57: antoine commented


Follow up: #619

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

No branches or pull requests

1 participant