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

TCP_CORK #2130

Closed
totaam opened this issue Jan 31, 2019 · 15 comments
Closed

TCP_CORK #2130

totaam opened this issue Jan 31, 2019 · 15 comments
Labels

Comments

@totaam
Copy link
Collaborator

totaam commented Jan 31, 2019

Issue migrated from trac ticket # 2130

component: network | priority: major | resolution: fixed

2019-01-31 12:44:22: antoine created the issue


Related to #619 and #2121#comment:5.

We know when there are more chunks to be written to the socket, so we can use TCP_CORK.

@totaam
Copy link
Collaborator Author

totaam commented Jan 31, 2019

2019-01-31 13:00:11: antoine changed owner from antoine to maxmylyn

@totaam
Copy link
Collaborator Author

totaam commented Jan 31, 2019

2019-01-31 13:00:11: antoine commented


Done in r21517. (note: no support for BSD OS variants - patches welcome)

Results in aggregated TCP packet chunks for large packets (ie: "draw"), including on websockets and SSL.
As per example in #2121#comment:5, the PNG pixel data and the xpra packet metadata no longer require an extra TCP frame:

T 127.0.0.1:10000 -> 127.0.0.1:38956 [AP] #12476
  82 7e 00 b5 50 00 00 07    00 00 00 7a 89 50 4e 47    .~..P......z.PNG
  0d 0a 1a 0a 00 00 00 0d    49 48 44 52 00 00 00 0c    ........IHDR....
  00 00 00 0d 08 02 00 00    00 12 4b 18 15 00 00 00    ..........K.....
  41 49 44 41 54 78 5e 63    fc ff ff 3f 03 2a 60 64    AIDATx^c...?.*`d
  64 44 13 61 42 e3 63 e5    42 15 61 ea 46 56 0d 52    dD.aB.c.B.a.FV.R
  84 5f 05 50 01 13 50 05    a6 b3 d0 dd 44 50 05 c8    ._.P..P.....DP..
  24 12 1c 8e 5f 29 f5 4c    42 b7 07 ab 3f 58 a8 e6    $..._).LB...?X..
  3b 00 f7 39 0f 17 4e 12    5b 8c 00 00 00 00 49 45    ;..9..N.[.....IE
  4e 44 ae 42 60 82 50 00    00 00 00 00 00 2b 6c 34    ND.B`.P......+l4
  3a 64 72 61 77 69 31 65    69 31 37 65 69 31 35 65    :drawi1ei17ei15e
  69 31 32 65 69 31 33 65    33 3a 70 6e 67 30 3a 69    i12ei13e3:png0:i
  31 34 65 69 30 65 64 65    65                         14ei0edee       
#

@maxmylin: Just like #619, this should result in slightly lower bandwidth / better bandwidth utilization, and lower latency. Most noticeable on low-bandwidth setups.

@totaam
Copy link
Collaborator Author

totaam commented Feb 9, 2019

2019-02-09 03:44:54: antoine changed owner from maxmylyn to encodedEntropy

@totaam
Copy link
Collaborator Author

totaam commented Aug 1, 2019

2019-08-01 13:02:19: smo changed owner from encodedEntropy to smo

@totaam
Copy link
Collaborator Author

totaam commented Aug 13, 2019

2019-08-13 19:15:14: smo uploaded file test_cork.tar.gz (113.4 KiB)

test of cork on and off

@totaam
Copy link
Collaborator Author

totaam commented Aug 13, 2019

2019-08-13 19:16:19: smo changed owner from smo to Antoine Martin

@totaam
Copy link
Collaborator Author

totaam commented Aug 13, 2019

2019-08-13 19:16:19: smo commented


What do you think of this as a baseline for testing with bandwidth constraints.

I think I may need longer tests for these I was only testing with rgb and auto encodings.

@totaam
Copy link
Collaborator Author

totaam commented Aug 14, 2019

2019-08-14 06:52:46: antoine changed owner from Antoine Martin to smo

@totaam
Copy link
Collaborator Author

totaam commented Aug 14, 2019

2019-08-14 06:52:46: antoine commented


I don't see the raw test data, what was the bandwidth constraint used?
Looks like the gtkperf test failed with CORK=0. (no data)
The only surprise so far is how the max-damage-latency is quite a bit higher and the min-quality is lower. It could be that using TCP_CORK makes the network layer push back more aggressively - which is not a bad thing. (fits with the lower max-batch-delay which is more coarse grained than damage-latency)
We would need to get the round-trip latency figures to verify that.

As per #619#comment:27, it would be useful to combine CORK with NODELAY.

@totaam
Copy link
Collaborator Author

totaam commented Aug 15, 2019

2019-08-15 12:54:45: antoine changed status from new to closed

@totaam
Copy link
Collaborator Author

totaam commented Aug 15, 2019

2019-08-15 12:54:45: antoine set resolution to fixed

@totaam
Copy link
Collaborator Author

totaam commented Aug 15, 2019

2019-08-15 12:54:45: antoine commented


As per #619#comment:29, this is better, even without measuring the effect on end-to-end latency, which should also be improved.

@totaam totaam closed this as completed Aug 15, 2019
@totaam
Copy link
Collaborator Author

totaam commented Aug 19, 2019

2019-08-19 17:38:21: antoine commented


Will follow up in #2381.

@totaam
Copy link
Collaborator Author

totaam commented Aug 20, 2019

2019-08-20 06:12:35: antoine commented


The charts are now available here: [https://xpra.org/stats/nodelay-cork/].

@totaam
Copy link
Collaborator Author

totaam commented Sep 25, 2019

2019-09-25 09:47:16: antoine commented


This option can now be enabled on a per-socket basis: #2424#comment:1.

See also #2975.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant