Skip to content

Commit 564238a

Browse files
committed
Merge bitcoin/bitcoin#31164: net: Use actual memory size in receive buffer accounting
d22a234 net: Use actual memory size in receive buffer accounting (laanwj) 047b5e2 streams: add DataStream::GetMemoryUsage (laanwj) c3a6722 net: Use DynamicUsage(m_type) in CSerializedNetMsg::GetMemoryUsage (laanwj) c6594c0 memusage: Add DynamicUsage for std::string (laanwj) 7596282 memusage: Allow counting usage of vectors with different allocators (laanwj) Pull request description: Add a method `CNetMessage::GetMemoryUsage` and use this for accounting of the size of the process receive queue instead of the raw message size (like we already do for the send buffer and `CSerializedNetMsg`). This ensures that allocation and deserialization overhead is better taken into account. On average, this counts about ~100 extra bytes per packet on x86_64: ``` 2024-10-27T09:50:12Z [net] 24 bytes -> 112 bytes 2024-10-27T10:36:37Z [net] 61 bytes -> 176 bytes 2024-10-27T10:36:38Z [net] 1285 bytes -> 1392 bytes 2024-10-27T09:50:21Z [net] 43057 bytes -> 43168 bytes ``` ACKs for top commit: l0rinc: ACK d22a234 achow101: ACK d22a234 i-am-yuvi: ACK d22a234 danielabrozzoni: Light ACK d22a234 - code looks good to me, but I'm not very familiar with C++ memory management specifics Tree-SHA512: ef09707e77b67bdbc48e9464133e4fccfa5c05051c1022e81ad84f20ed41db83ac5a9b109ebdb8d38f70785c03c5d6bfe51d32dc133d49e52d1e6225f6f8e292
2 parents 2c90f8e + d22a234 commit 564238a

File tree

5 files changed

+36
-9
lines changed

5 files changed

+36
-9
lines changed

src/memusage.h

+16-3
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include <map>
1616
#include <memory>
1717
#include <set>
18+
#include <string>
1819
#include <vector>
1920
#include <unordered_map>
2021
#include <unordered_set>
@@ -84,10 +85,22 @@ struct stl_shared_counter
8485
size_t weak_count;
8586
};
8687

87-
template<typename X>
88-
static inline size_t DynamicUsage(const std::vector<X>& v)
88+
template<typename T, typename Allocator>
89+
static inline size_t DynamicUsage(const std::vector<T, Allocator>& v)
90+
{
91+
return MallocUsage(v.capacity() * sizeof(T));
92+
}
93+
94+
static inline size_t DynamicUsage(const std::string& s)
8995
{
90-
return MallocUsage(v.capacity() * sizeof(X));
96+
const char* s_ptr = reinterpret_cast<const char*>(&s);
97+
// Don't count the dynamic memory used for string, if it resides in the
98+
// "small string" optimization area (which stores data inside the object itself, up to some
99+
// size; 15 bytes in modern libstdc++).
100+
if (!std::less{}(s.data(), s_ptr) && !std::greater{}(s.data() + s.size(), s_ptr + sizeof(s))) {
101+
return 0;
102+
}
103+
return MallocUsage(s.capacity());
91104
}
92105

93106
template<unsigned int N, typename X, typename S, typename D>

src/net.cpp

+8-6
Original file line numberDiff line numberDiff line change
@@ -121,10 +121,12 @@ std::string strSubVersion;
121121

122122
size_t CSerializedNetMsg::GetMemoryUsage() const noexcept
123123
{
124-
// Don't count the dynamic memory used for the m_type string, by assuming it fits in the
125-
// "small string" optimization area (which stores data inside the object itself, up to some
126-
// size; 15 bytes in modern libstdc++).
127-
return sizeof(*this) + memusage::DynamicUsage(data);
124+
return sizeof(*this) + memusage::DynamicUsage(m_type) + memusage::DynamicUsage(data);
125+
}
126+
127+
size_t CNetMessage::GetMemoryUsage() const noexcept
128+
{
129+
return sizeof(*this) + memusage::DynamicUsage(m_type) + m_recv.GetMemoryUsage();
128130
}
129131

130132
void CConnman::AddAddrFetch(const std::string& strDest)
@@ -3772,7 +3774,7 @@ void CNode::MarkReceivedMsgsForProcessing()
37723774
for (const auto& msg : vRecvMsg) {
37733775
// vRecvMsg contains only completed CNetMessage
37743776
// the single possible partially deserialized message are held by TransportDeserializer
3775-
nSizeAdded += msg.m_raw_message_size;
3777+
nSizeAdded += msg.GetMemoryUsage();
37763778
}
37773779

37783780
LOCK(m_msg_process_queue_mutex);
@@ -3789,7 +3791,7 @@ std::optional<std::pair<CNetMessage, bool>> CNode::PollMessage()
37893791
std::list<CNetMessage> msgs;
37903792
// Just take one message
37913793
msgs.splice(msgs.begin(), m_msg_process_queue, m_msg_process_queue.begin());
3792-
m_msg_process_queue_size -= msgs.front().m_raw_message_size;
3794+
m_msg_process_queue_size -= msgs.front().GetMemoryUsage();
37933795
fPauseRecv = m_msg_process_queue_size > m_recv_flood_size;
37943796

37953797
return std::make_pair(std::move(msgs.front()), !m_msg_process_queue.empty());

src/net.h

+3
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,9 @@ class CNetMessage
245245
CNetMessage(const CNetMessage&) = delete;
246246
CNetMessage& operator=(CNetMessage&&) = default;
247247
CNetMessage& operator=(const CNetMessage&) = delete;
248+
249+
/** Compute total memory usage of this object (own memory + any dynamic memory). */
250+
size_t GetMemoryUsage() const noexcept;
248251
};
249252

250253
/** The Transport converts one connection's sent messages to wire bytes, and received bytes back. */

src/streams.cpp

+6
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Distributed under the MIT software license, see the accompanying
33
// file COPYING or https://opensource.org/license/mit/.
44

5+
#include <memusage.h>
56
#include <span.h>
67
#include <streams.h>
78
#include <util/fs_helpers.h>
@@ -110,3 +111,8 @@ bool AutoFile::Truncate(unsigned size)
110111
{
111112
return ::TruncateFile(m_file, size);
112113
}
114+
115+
size_t DataStream::GetMemoryUsage() const noexcept
116+
{
117+
return sizeof(*this) + memusage::DynamicUsage(vch);
118+
}

src/streams.h

+3
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,9 @@ class DataStream
277277
{
278278
util::Xor(MakeWritableByteSpan(*this), MakeByteSpan(key));
279279
}
280+
281+
/** Compute total memory usage of this object (own memory + any dynamic memory). */
282+
size_t GetMemoryUsage() const noexcept;
280283
};
281284

282285
template <typename IStream>

0 commit comments

Comments
 (0)