-
Notifications
You must be signed in to change notification settings - Fork 37.8k
Set TCP_NODELAY on P2P sockets. #6867
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
Conversation
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. |
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. |
I think you'll also need it in SocketSendData:
|
I ran my test (similar to gavin's, testing block relay at the tip on regtest) and this brings the time from receiving the |
@ptschip I don't believe so. Have a citation? |
@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. |
Concept ACK. |
@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. |
Tested ACK, on OSX and Linux. Decreases a 4-hop cross-country-twice block relay from 1.3 seconds to 0.7 seconds. |
@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. |
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.
Updated to set it on the accepted sockets as well, "just in case". |
ACK |
utACK |
a4e28b3 Set TCP_NODELAY on P2P sockets. (Gregory Maxwell)
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. Conflicts: src/net.cpp Rebased-From: a4e28b3 Github-Pull: #6867
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. Conflicts: src/net.cpp Rebased-From: a4e28b3 Github-Pull: #6867
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. Conflicts: src/net.cpp Rebased-From: a4e28b3 Github-Pull: bitcoin#6867
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. Conflicts: src/net.cpp Rebased-From: a4e28b3 Github-Pull: bitcoin#6867 (cherry picked from commit 5297194)
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.