Skip to content

Commit a1a1060

Browse files
committed
net: measure send buffer fullness based on memory usage
This more accurately captures the intent of limiting send buffer size, as many small messages can have a larger overhead that is not counted with the current approach. It also means removing the dependency on the header size (which will become a function of the transport choice) from the send buffer calculations.
1 parent 009ff8d commit a1a1060

File tree

3 files changed

+43
-9
lines changed

3 files changed

+43
-9
lines changed

src/init.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -490,7 +490,7 @@ void SetupServerArgs(ArgsManager& argsman)
490490
argsman.AddArg("-listenonion", strprintf("Automatically create Tor onion service (default: %d)", DEFAULT_LISTEN_ONION), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
491491
argsman.AddArg("-maxconnections=<n>", strprintf("Maintain at most <n> connections to peers (default: %u). This limit does not apply to connections manually added via -addnode or the addnode RPC, which have a separate limit of %u.", DEFAULT_MAX_PEER_CONNECTIONS, MAX_ADDNODE_CONNECTIONS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
492492
argsman.AddArg("-maxreceivebuffer=<n>", strprintf("Maximum per-connection receive buffer, <n>*1000 bytes (default: %u)", DEFAULT_MAXRECEIVEBUFFER), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
493-
argsman.AddArg("-maxsendbuffer=<n>", strprintf("Maximum per-connection send buffer, <n>*1000 bytes (default: %u)", DEFAULT_MAXSENDBUFFER), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
493+
argsman.AddArg("-maxsendbuffer=<n>", strprintf("Maximum per-connection memory usage for the send buffer, <n>*1000 bytes (default: %u)", DEFAULT_MAXSENDBUFFER), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
494494
argsman.AddArg("-maxtimeadjustment", strprintf("Maximum allowed median peer time offset adjustment. Local perspective of time may be influenced by outbound peers forward or backward by this amount (default: %u seconds).", DEFAULT_MAX_TIME_ADJUSTMENT), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
495495
argsman.AddArg("-maxuploadtarget=<n>", strprintf("Tries to keep outbound traffic under the given target per 24h. Limit does not apply to peers with 'download' permission or blocks created within past week. 0 = no limit (default: %s). Optional suffix units [k|K|m|M|g|G|t|T] (default: M). Lowercase is 1000 base while uppercase is 1024 base", DEFAULT_MAX_UPLOAD_TARGET), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
496496
argsman.AddArg("-onion=<ip:port>", "Use separate SOCKS5 proxy to reach peers via Tor onion services, set -noonion to disable (default: -proxy)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);

src/net.cpp

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include <crypto/sha256.h>
2020
#include <i2p.h>
2121
#include <logging.h>
22+
#include <memusage.h>
2223
#include <net_permissions.h>
2324
#include <netaddress.h>
2425
#include <netbase.h>
@@ -116,6 +117,14 @@ std::map<CNetAddr, LocalServiceInfo> mapLocalHost GUARDED_BY(g_maplocalhost_mute
116117
static bool vfLimited[NET_MAX] GUARDED_BY(g_maplocalhost_mutex) = {};
117118
std::string strSubVersion;
118119

120+
size_t CSerializedNetMsg::GetMemoryUsage() const noexcept
121+
{
122+
// Don't count the dynamic memory used for the m_type string, by assuming it fits in the
123+
// "small string" optimization area (which stores data inside the object itself, up to some
124+
// size; 15 bytes in modern libstdc++).
125+
return sizeof(*this) + memusage::DynamicUsage(data);
126+
}
127+
119128
void CConnman::AddAddrFetch(const std::string& strDest)
120129
{
121130
LOCK(m_addr_fetches_mutex);
@@ -894,6 +903,14 @@ void V1Transport::MarkBytesSent(size_t bytes_sent) noexcept
894903
}
895904
}
896905

906+
size_t V1Transport::GetSendMemoryUsage() const noexcept
907+
{
908+
AssertLockNotHeld(m_send_mutex);
909+
LOCK(m_send_mutex);
910+
// Don't count sending-side fields besides m_message_to_send, as they're all small and bounded.
911+
return m_message_to_send.GetMemoryUsage();
912+
}
913+
897914
std::pair<size_t, bool> CConnman::SocketSendData(CNode& node) const
898915
{
899916
auto it = node.vSendMsg.begin();
@@ -923,8 +940,8 @@ std::pair<size_t, bool> CConnman::SocketSendData(CNode& node) const
923940
nSentSize += nBytes;
924941
if (node.nSendOffset == data.size()) {
925942
node.nSendOffset = 0;
926-
node.nSendSize -= data.size();
927-
node.fPauseSend = node.nSendSize > nSendBufferMaxSize;
943+
// Update memory usage of send buffer (as *it will be deleted).
944+
node.m_send_memusage -= sizeof(data) + memusage::DynamicUsage(data);
928945
it++;
929946
} else {
930947
// could not send full message; stop sending more
@@ -944,9 +961,11 @@ std::pair<size_t, bool> CConnman::SocketSendData(CNode& node) const
944961
}
945962
}
946963

964+
node.fPauseSend = node.m_send_memusage + node.m_transport->GetSendMemoryUsage() > nSendBufferMaxSize;
965+
947966
if (it == node.vSendMsg.end()) {
948967
assert(node.nSendOffset == 0);
949-
assert(node.nSendSize == 0);
968+
assert(node.m_send_memusage == 0);
950969
}
951970
node.vSendMsg.erase(node.vSendMsg.begin(), it);
952971
return {nSentSize, !node.vSendMsg.empty()};
@@ -2985,14 +3004,21 @@ void CConnman::PushMessage(CNode* pnode, CSerializedNetMsg&& msg)
29853004
if (bytes.empty()) break;
29863005
// Update statistics per message type.
29873006
pnode->AccountForSentBytes(msg_type, bytes.size());
2988-
// Update number of bytes in the send buffer.
2989-
pnode->nSendSize += bytes.size();
2990-
if (pnode->nSendSize > nSendBufferMaxSize) pnode->fPauseSend = true;
29913007
pnode->vSendMsg.push_back({bytes.begin(), bytes.end()});
3008+
// Update memory usage of send buffer. For now, use static + dynamic memory usage of
3009+
// byte vectors in vSendMsg as send memory. In a future commit, vSendMsg will be
3010+
// replaced with a queue of CSerializedNetMsg objects, and we'll use their memory usage
3011+
// instead.
3012+
pnode->m_send_memusage += sizeof(pnode->vSendMsg.back()) + memusage::DynamicUsage(pnode->vSendMsg.back());
29923013
// Notify transport that bytes have been processed (they're not actually sent yet,
29933014
// but pushed onto the vSendMsg queue of bytes to send).
29943015
pnode->m_transport->MarkBytesSent(bytes.size());
29953016
}
3017+
// At this point, m_transport->GetSendMemoryUsage() isn't very interesting as the
3018+
// transport's message is fully flushed (and converted to byte arrays). It's still included
3019+
// here for correctness, and will become relevant in a future commit when a queued message
3020+
// inside the transport may survive PushMessage calls.
3021+
if (pnode->m_send_memusage + pnode->m_transport->GetSendMemoryUsage() > nSendBufferMaxSize) pnode->fPauseSend = true;
29963022

29973023
// If the write queue was empty before and isn't now, attempt "optimistic write":
29983024
// because the poll/select loop may pause for SELECT_TIMEOUT_MILLISECONDS before actually

src/net.h

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,9 @@ struct CSerializedNetMsg {
122122

123123
std::vector<unsigned char> data;
124124
std::string m_type;
125+
126+
/** Compute total memory usage of this object (own memory + any dynamic memory). */
127+
size_t GetMemoryUsage() const noexcept;
125128
};
126129

127130
/**
@@ -313,6 +316,9 @@ class Transport {
313316
* If bytes_sent=0, this call has no effect.
314317
*/
315318
virtual void MarkBytesSent(size_t bytes_sent) noexcept = 0;
319+
320+
/** Return the memory usage of this transport attributable to buffered data to send. */
321+
virtual size_t GetSendMemoryUsage() const noexcept = 0;
316322
};
317323

318324
class V1Transport final : public Transport
@@ -399,6 +405,7 @@ class V1Transport final : public Transport
399405
bool SetMessageToSend(CSerializedNetMsg& msg) noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_send_mutex);
400406
BytesToSend GetBytesToSend() const noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_send_mutex);
401407
void MarkBytesSent(size_t bytes_sent) noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_send_mutex);
408+
size_t GetSendMemoryUsage() const noexcept override EXCLUSIVE_LOCKS_REQUIRED(!m_send_mutex);
402409
};
403410

404411
struct CNodeOptions
@@ -429,8 +436,9 @@ class CNode
429436
*/
430437
std::shared_ptr<Sock> m_sock GUARDED_BY(m_sock_mutex);
431438

432-
/** Total size of all vSendMsg entries */
433-
size_t nSendSize GUARDED_BY(cs_vSend){0};
439+
/** Total memory usage of vSendMsg (counting the vectors and their dynamic usage, but not the
440+
* deque overhead). */
441+
size_t m_send_memusage GUARDED_BY(cs_vSend){0};
434442
/** Offset inside the first vSendMsg already sent */
435443
size_t nSendOffset GUARDED_BY(cs_vSend){0};
436444
uint64_t nSendBytes GUARDED_BY(cs_vSend){0};

0 commit comments

Comments
 (0)