Skip to content

Commit

Permalink
Merge bitcoin/bitcoin#31164: net: Use actual memory size in receive b…
Browse files Browse the repository at this point in the history
…uffer 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
  • Loading branch information
achow101 committed Nov 6, 2024
2 parents 2c90f8e + d22a234 commit 564238a
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 9 deletions.
19 changes: 16 additions & 3 deletions src/memusage.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include <map>
#include <memory>
#include <set>
#include <string>
#include <vector>
#include <unordered_map>
#include <unordered_set>
Expand Down Expand Up @@ -84,10 +85,22 @@ struct stl_shared_counter
size_t weak_count;
};

template<typename X>
static inline size_t DynamicUsage(const std::vector<X>& v)
template<typename T, typename Allocator>
static inline size_t DynamicUsage(const std::vector<T, Allocator>& v)
{
return MallocUsage(v.capacity() * sizeof(T));
}

static inline size_t DynamicUsage(const std::string& s)
{
return MallocUsage(v.capacity() * sizeof(X));
const char* s_ptr = reinterpret_cast<const char*>(&s);
// Don't count the dynamic memory used for string, if it resides in the
// "small string" optimization area (which stores data inside the object itself, up to some
// size; 15 bytes in modern libstdc++).
if (!std::less{}(s.data(), s_ptr) && !std::greater{}(s.data() + s.size(), s_ptr + sizeof(s))) {
return 0;
}
return MallocUsage(s.capacity());
}

template<unsigned int N, typename X, typename S, typename D>
Expand Down
14 changes: 8 additions & 6 deletions src/net.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -121,10 +121,12 @@ std::string strSubVersion;

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

size_t CNetMessage::GetMemoryUsage() const noexcept
{
return sizeof(*this) + memusage::DynamicUsage(m_type) + m_recv.GetMemoryUsage();
}

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

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

return std::make_pair(std::move(msgs.front()), !m_msg_process_queue.empty());
Expand Down
3 changes: 3 additions & 0 deletions src/net.h
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,9 @@ class CNetMessage
CNetMessage(const CNetMessage&) = delete;
CNetMessage& operator=(CNetMessage&&) = default;
CNetMessage& operator=(const CNetMessage&) = delete;

/** Compute total memory usage of this object (own memory + any dynamic memory). */
size_t GetMemoryUsage() const noexcept;
};

/** The Transport converts one connection's sent messages to wire bytes, and received bytes back. */
Expand Down
6 changes: 6 additions & 0 deletions src/streams.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Distributed under the MIT software license, see the accompanying
// file COPYING or https://opensource.org/license/mit/.

#include <memusage.h>
#include <span.h>
#include <streams.h>
#include <util/fs_helpers.h>
Expand Down Expand Up @@ -110,3 +111,8 @@ bool AutoFile::Truncate(unsigned size)
{
return ::TruncateFile(m_file, size);
}

size_t DataStream::GetMemoryUsage() const noexcept
{
return sizeof(*this) + memusage::DynamicUsage(vch);
}
3 changes: 3 additions & 0 deletions src/streams.h
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,9 @@ class DataStream
{
util::Xor(MakeWritableByteSpan(*this), MakeByteSpan(key));
}

/** Compute total memory usage of this object (own memory + any dynamic memory). */
size_t GetMemoryUsage() const noexcept;
};

template <typename IStream>
Expand Down

0 comments on commit 564238a

Please sign in to comment.