Skip to content

Conversation

whitslack
Copy link
Contributor

Since Nagle's algorithm is disabled, each and every call to send(2) can potentially generate a separate TCP segment on the wire. This is especially inefficient when sending the tiny header preceding each message payload.

Linux implements a MSG_MORE flag that tells the kernel not to push the passed data immediately to the connected peer but rather to collect it in the socket's internal transmit buffer where it can be combined with data from successive calls to send(2). Where available, specify this flag when calling send(2) in CConnman::SocketSendData(CNode *) if the data buffer being sent is not the last one in pnode->vSendMsg.

Since Nagle's algorithm is disabled, each and every call to send(2) can potentially generate a separate TCP segment on the wire. This is especially inefficient when sending the tiny header preceding each message payload.

Linux implements a MSG_MORE flag that tells the kernel not to push the passed data immediately to the connected peer but rather to collect it in the socket's internal transmit buffer where it can be combined with data from successive calls to send(2). Where available, specify this flag when calling send(2) in CConnman::SocketSendData(CNode *) if the data buffer being sent is not the last one in pnode->vSendMsg.
@theuni
Copy link
Member

theuni commented Feb 23, 2018

I'd rather not, as we'll be switching to sendmsg soon, instead taking advantage of scatter/gather which is highly preferable as it only requires one syscall. I've implemented this here: 77cee16 as part of #11227. It may make sense to go ahead and take that change rather than waiting for all of the other libevent changes to go in first.

Note that something like TCP_CORK may still be useful, though, in the event that more than 1 sendmsg would be required.

@fanquake fanquake added the P2P label Feb 23, 2018
@laanwj
Copy link
Member

laanwj commented Mar 6, 2018

Thanks for the contribution, but I agree it'd be better to not change this in the mean time until @theuni's change (hopefully soon) will go in.

@laanwj laanwj closed this Mar 6, 2018
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants