Set TCP_NODELAY on P2P sockets. #6867

1. 
   gmaxwell commented at 11:55 pm on October 21, 2015: contributor

   Nagle appears to be a significant contributor to latency now that the static sleeps are gone. Most of our messages are relatively large compared to IP + TCP so I do not expect this to create enormous overhead.

   This may also reduce traffic burstyness somewhat.

2. 
   jgarzik commented at 11:59 pm on October 21, 2015: contributor

   ut ACK

   Needs some measuring - I do not expect any overhead, given that messages are composed and then sent to write(2), as much as write(2) will swallow. Should create packets as large as path MTU and conditions will permit.

3. 
   gmaxwell commented at 0:15 am on October 22, 2015: contributor

   Gavin did a test which was basically the right test for measuring the performance impact, I pinged him and asked him to try this.

   We do have some messages that are fairly small– things like pings. There will be some amplification as a result, but I doubt it will be much. I’ll see if I can figure out a setup for measuring that.

4. 
   ptschip commented at 0:23 am on October 22, 2015: contributor

   I think you’ll also need it in SocketSendData:

   0   -int nBytes = send(pnode->hSocket, &data[pnode->nSendOffset], data.size() - pnode->nSendOffset, MSG_NOSIGNAL | MSG_DONTWAIT);
   1   +int nBytes = send(pnode->hSocket, &data[pnode->nSendOffset], data.size() - pnode->nSendOffset, MSG_NOSIGNAL | MSG_DONTWAIT | TCP_NODELAY);
   

5. 
   sdaftuar commented at 0:30 am on October 22, 2015: member

   I ran my test (similar to gavin’s, testing block relay at the tip on regtest) and this brings the time from receiving the inv to receiving the block down to (just over) the round-trip time between the peers.
6. 
   gmaxwell commented at 0:30 am on October 22, 2015: contributor

   @ptschip I don’t believe so. Have a citation?
7. 
   ptschip commented at 0:47 am on October 22, 2015: contributor

   @gmaxwell I think what I was thinking of was the outgoing connections also need it. I didn’t see you had that already covered. My eyes are a bit blurry tonight.

   Looks good.

8. 
   TheBlueMatt commented at 0:50 am on October 22, 2015: member

   Concept ACK.
9. 
   theuni commented at 1:08 am on October 22, 2015: member

   @gmaxwell Are you sure this option is inherited by accept()ed sockets?
10. 
    laanwj commented at 9:54 am on October 22, 2015: member

    @gmaxwell Are you sure this option is inherited by accept()ed sockets?

    Is TCP_NODELAY inherited from listening sockets on your platform? seems to indicate “yes” here, but I’m not sure either that’s a safe assumption cross-platform. Let’s add it to accepted sockets too just to be sure.

11. 
    gavinandresen commented at 2:25 pm on October 22, 2015: contributor

    Tested ACK, on OSX and Linux.

    Decreases a 4-hop cross-country-twice block relay from 1.3 seconds to 0.7 seconds.

12. 
    theuni commented at 3:56 pm on October 22, 2015: member

    @laanwj nice link. Interestingly, osx (here, anyway) inherits TCP_NODELAY, but not O_NONBLOCK! That seems suspicious, as I assume that would be pretty obvious. I’ll investigate separately.
13. Set TCP_NODELAY on P2P sockets.

    Nagle appears to be a significant contributor to latency now that the static
     sleeps are gone.  Most of our messages are relatively large compared to
     IP + TCP so I do not expect this to create enormous overhead.
    
    This may also reduce traffic burstyness somewhat.

    a4e28b3d1e
14. 
    gmaxwell commented at 6:00 pm on October 22, 2015: contributor

    Updated to set it on the accepted sockets as well, “just in case”.
15. 
    btcdrak commented at 6:19 am on October 23, 2015: contributor

    ACK
16. 
    laanwj commented at 7:09 am on October 23, 2015: member

    utACK
17. laanwj merged this on Oct 23, 2015
18. laanwj closed this on Oct 23, 2015

    ---

19. laanwj referenced this in commit b2b173acab on Oct 23, 2015
20. laanwj added the label P2P on Oct 23, 2015
21. gmaxwell referenced this in commit 95a50390e1 on Oct 23, 2015
22. gmaxwell referenced this in commit 5297194bbd on Oct 23, 2015
23. Infernoman referenced this in commit b6a17756a5 on Jan 25, 2017
24. reddink referenced this in commit 75c7458fa6 on May 27, 2020
25. MarcoFalke locked this on Sep 8, 2021

Contributors
gmaxwell jgarzik ptschip sdaftuar TheBlueMatt theuni laanwj gavinandresen btcdrak

Labels
P2P