Net: Cork socket send writes #6874

pstratem commented at 2:31 AM on October 23, 2015: contributor

Builds on top of #6867.

This allows the kernel to collect multiple messages into a single packet.

400f4be12d08f5027f865bc6a1880ddd8fff5abee6a0c53d4f0f9b4661c4df3f

pstratem renamed this:
~~Net: Cork socket send writes with MSG_MORE~~
Net: Cork socket send writes with MSG_MORE [WIP]
on Oct 23, 2015

laanwj commented at 8:17 AM on October 23, 2015: member

For those wondering what this is about (I did): TCP_CORK: More than you ever wanted to know

laanwj added the label P2P on Oct 23, 2015

laanwj added the label Linux on Oct 23, 2015

pstratem renamed this:
~~Net: Cork socket send writes with MSG_MORE [WIP]~~
Net: Cork socket send writes
on Oct 23, 2015

theuni commented at 2:57 PM on October 23, 2015: member

After looking into the options here, I think writing with scatter/gather (sendmsg/writev/etc) would be simpler and just as effective now that NODELAY is set.

pstratem commented at 11:08 PM on October 23, 2015: contributor

I agree but there does not seem to be a reasonable cross platform way to do that. On Oct 23, 2015 7:58 AM, "Cory Fields" notifications@github.com wrote:

After looking into the options here, I think writing with scatter/gather (sendmsg/writev/etc) would be simpler and just as effective now that NODELAY is set.

— Reply to this email directly or view it on GitHub #6874 (comment).

Cork socket send() by unsetting TCP_NODELAY 98589446ec

laanwj commented at 7:18 AM on October 26, 2015: member

Concept ACK - but I think we should hold back on doing a lot of work here unless the impact is immediate and obvious (such as for TCP_NODELAY). With libevent we will get a few advanced networking features for free - including use of readv/writev where available.

pstratem commented at 11:46 PM on October 26, 2015: contributor

@laanwj this is now cross platform afaict

sipa commented at 1:56 AM on October 28, 2015: member

@gavinandresen Feel like testing this on your 5-hop setup?

gavinandresen commented at 3:10 PM on October 28, 2015: contributor

ACK, tested on OSX and Linux. Timing for 5-node 4-hop is the same as for just the TCP_NODELAY fix:

First node that found the block:
50.638 UpdateTip: new best=226860e92c853c45cd9afc571353be9cbf8e4116b586ae1481c32e2adb8ffb65  height=4  log2_work=3.3219281  tx=5  date=2015-10-28 15:04:40 progress=1.000000  cache=0.0MiB(4tx)
50.649 AddToWallet 86ef5c25a58631da87d6199fae5ebd82f55311e37e6c9bc1adc9669d55bbbeda  new
50.660 sending: inv (37 bytes) peer=1
50.661 keypool keep 5
50.748 received: getheaders (165 bytes) peer=1
50.748 getheaders 4 to 226860e92c853c45cd9afc571353be9cbf8e4116b586ae1481c32e2adb8ffb65 from peer=1
50.748 sending: headers (82 bytes) peer=1
50.748 received: getdata (37 bytes) peer=1
50.749 received getdata (1 invsz) peer=1
50.749 received getdata for: block 226860e92c853c45cd9afc571353be9cbf8e4116b586ae1481c32e2adb8ffb65 peer=1
50.750 sending: block (179 bytes) peer=1

Last node in the chain, after 4 hops:

51.225 received: inv (37 bytes) peer=1
51.225 got inv: block 226860e92c853c45cd9afc571353be9cbf8e4116b586ae1481c32e2adb8ffb65  new peer=1
51.225 sending: getheaders (165 bytes) peer=1
51.225 getheaders (3) 226860e92c853c45cd9afc571353be9cbf8e4116b586ae1481c32e2adb8ffb65 to peer=1
51.225 sending: getdata (37 bytes) peer=1
51.317 received: headers (82 bytes) peer=1
51.318 received: block (179 bytes) peer=1
51.318 received block 226860e92c853c45cd9afc571353be9cbf8e4116b586ae1481c32e2adb8ffb65 peer=1
51.321 UpdateTip: new best=226860e92c853c45cd9afc571353be9cbf8e4116b586ae1481c32e2adb8ffb65  height=4  log2_work=3.3219281  tx=5  date=2015-10-28 15:04:40 progress=1.000000  cache=0.0MiB(4tx)

Ping latency for each hop: round-trip min/avg/max/stddev = 90.495/92.060/95.230/1.281 ms

laanwj commented at 11:19 AM on October 29, 2015: member

If it is the same, TBH I don't see the point in this extra complication.

pstratem commented at 11:27 AM on October 29, 2015: contributor

@laanwj there was no expectation of this improving latency

this is to improve the bandwidth used

laanwj commented at 11:38 AM on October 29, 2015: member

Can we measure that too, then?

pstratem commented at 12:47 PM on November 16, 2015: contributor

@laanwj it's actually more about packets per second rather than bandwidth or latency

I could measure that

laanwj commented at 3:35 PM on November 16, 2015: member

Great! I'm just wary of merging this if we can't measure any clear improvement. Could see it breaking (or cancelling the NO_DELAY improvement) on obscure platforms which don't allow for high-frequency switching of NO_DELAY mode, if those exist...

morcos commented at 9:54 PM on November 16, 2015: member

If you're new to this PR, see discussion here for background: http://bitcoinstats.com/irc/bitcoin-dev/logs/2015/10/23

sipa commented at 1:28 PM on November 28, 2015: member

Ok, I missed too that this was mostly about reducing unnecessary packets. That seems less urgent than latency reductions, and numbers would indeed be nice.

sipa removed the label Linux on Nov 28, 2015

pstratem closed this on Dec 13, 2015

DrahtBot locked this on Sep 8, 2021