net: Use actual memory size in receive buffer accounting

laanwj commented at 9:09 am on October 27, 2024: member

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:

02024-10-27T09:50:12Z [net] 24 bytes -> 112 bytes
12024-10-27T10:36:37Z [net] 61 bytes -> 176 bytes
22024-10-27T10:36:38Z [net] 1285 bytes -> 1392 bytes
32024-10-27T09:50:21Z [net] 43057 bytes -> 43168 bytes

laanwj added the label P2P on Oct 27, 2024

DrahtBot commented at 9:09 am on October 27, 2024: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31164.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	l0rinc, i-am-yuvi, danielabrozzoni, achow101
Concept ACK	jarolrod
Stale ACK	theStack

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#28792 (Embed default ASMap as binary dump header file by fjahr)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

jarolrod commented at 8:50 am on October 29, 2024: member

Concept ACK

theStack commented at 2:58 pm on October 30, 2024: contributor

Concept ACK

theStack approved

theStack commented at 1:30 am on November 1, 2024: contributor

ACK ad5c012157c5f261503022cfa22d7124bfda5765

On average, this counts about ~100 extra bytes per packet on x86_64:

02024-10-27T09:50:12Z [net] 24 bytes -> 112 bytes
12024-10-27T10:36:37Z [net] 61 bytes -> 176 bytes
22024-10-27T10:36:38Z [net] 1285 bytes -> 1392 bytes
32024-10-27T09:50:21Z [net] 43057 bytes -> 43168 bytes

Seeing very similar numbers here w.r.t. to the extra byte count (tested on OpenBSD 7.6, x86_64):

02024-11-01T01:25:18Z msg type: inv, raw size: 58, mem usage: 168
12024-11-01T01:25:18Z msg type: inv, raw size: 130, mem usage: 232
22024-11-01T01:25:18Z msg type: inv, raw size: 166, mem usage: 280
32024-11-01T01:25:19Z msg type: tx, raw size: 742, mem usage: 856
42024-11-01T01:25:19Z msg type: tx, raw size: 3010, mem usage: 3112
52024-11-01T01:25:19Z msg type: tx, raw size: 17758, mem usage: 17864

 0diff --git a/src/net.cpp b/src/net.cpp
 1index 49edceead4..2c5725e299 100644
 2--- a/src/net.cpp
 3+++ b/src/net.cpp
 4@@ -3779,6 +3779,7 @@ void CNode::MarkReceivedMsgsForProcessing()
 5         // vRecvMsg contains only completed CNetMessage
 6         // the single possible partially deserialized message are held by TransportDeserializer
 7         nSizeAdded += msg.GetMemoryUsage();
 8+        LogPrintf("msg type: %s, raw size: %d, mem usage: %d\n", msg.m_type, msg.m_raw_message_size, msg.GetMemoryUsage());
 9     }
10 
11     LOCK(m_msg_process_queue_mutex);

DrahtBot requested review from jarolrod on Nov 1, 2024

i-am-yuvi approved

i-am-yuvi commented at 4:23 pm on November 2, 2024: none

Tested ACK ad5c012157c5f261503022cfa22d7124bfda5765

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:
02024-10-27T09:50:12Z [net] 24 bytes -> 112 bytes
12024-10-27T10:36:37Z [net] 61 bytes -> 176 bytes
22024-10-27T10:36:38Z [net] 1285 bytes -> 1392 bytes
32024-10-27T09:50:21Z [net] 43057 bytes -> 43168 bytes

Tested on Ubuntu 22.04.4 LTS x86_64 (extra bytes ~100)

02024-11-02T16:09:18Z msg: pong, raw size: 29, mem usage: 144
12024-11-02T16:09:18Z msg: headers, raw size: 103, mem usage: 224
22024-11-02T16:09:18Z msg: sendheaders, raw size: 33, mem usage: 112
32024-11-02T16:09:18Z msg: cmpctblock, raw size: 23280, mem usage: 23392
42024-11-02T16:09:18Z msg: pong, raw size: 32, mem usage: 144

in src/memusage.h:88 in b58dab98ab outdated

83@@ -84,8 +84,8 @@ struct stl_shared_counter
84     size_t weak_count;
85 };
86 
87-template<typename X>
88-static inline size_t DynamicUsage(const std::vector<X>& v)
89+template<typename X, typename Y>
90+static inline size_t DynamicUsage(const std::vector<X, Y>& v)

l0rinc commented at 8:25 am on November 4, 2024:

Nit: Could we find better names than X and Y (e.g. to avoid readers thinking Y is the size), maybe something like template<typename ElementType, typename AllocatorType>

laanwj commented at 3:41 pm on November 4, 2024:

https://en.cppreference.com/w/cpp/container/vector uses

0template<
1    class T,
2    class Allocator = [std::allocator](http://en.cppreference.com/w/cpp/memory/allocator)<T>
3> class vector

so let’s go with those names

in src/memusage.h:90 in b58dab98ab outdated

83@@ -84,8 +84,8 @@ struct stl_shared_counter
84     size_t weak_count;
85 };
86 
87-template<typename X>
88-static inline size_t DynamicUsage(const std::vector<X>& v)
89+template<typename X, typename Y>
90+static inline size_t DynamicUsage(const std::vector<X, Y>& v)
91 {
92     return MallocUsage(v.capacity() * sizeof(X));

l0rinc commented at 8:25 am on November 4, 2024:

Q: v.capacity() can be significantly more here than v.size(), right? Is it because we can’t size them properly in https://github.com/bitcoin/bitcoin/blob/master/src/streams.h#L82?

l0rinc commented at 9:24 am on November 4, 2024:

Q: we assume that sizeof(v) (the vector’s fixed memory) was already included in the calculation before this call, right?

l0rinc commented at 11:19 am on November 4, 2024:

Slightly unrelated, but MemUsage could be generalized to explain the alignment and rounding:

0static inline size_t MallocUsage(const size_t alloc)
1{
2    if (alloc == 0) return 0;
3    constexpr size_t alignment{2 * sizeof(void*)};
4    constexpr size_t round_up{alignment * 2 - 1};
5    constexpr size_t mask{~(alignment - 1)};
6    return alloc + round_up & mask;
7}

and

 0BOOST_AUTO_TEST_CASE(malloc_usage)
 1{
 2    auto referenceMallocUsage{[](size_t alloc) -> size_t {
 3        // Measured on libc6 2.19 on Linux.
 4        if (alloc == 0) {
 5            return 0;
 6        } else if (sizeof(void*) == 8) {
 7            return ((alloc + 31) >> 4) << 4;
 8        } else if (sizeof(void*) == 4) {
 9            return ((alloc + 15) >> 3) << 3;
10        } else {
11            assert(0);
12        }
13    }};
14
15    FastRandomContext rng{/*fDeterministic=*/false};
16    for (int i{0}; i < 100; ++i) {
17        const size_t randomAlloc{rng.randrange(1000)};
18        auto expected{referenceMallocUsage(randomAlloc)};
19        auto actual{memusage::MallocUsage(randomAlloc)};
20        BOOST_CHECK_EQUAL(expected, actual);
21    }
22}

laanwj commented at 1:18 pm on November 4, 2024:

Right, it can, when reserve() is called with a larger amount, or when the vector is shortened. But it does represent the actually-allocated size which seems what we want here.

laanwj commented at 1:19 pm on November 4, 2024:

Correct, it’s computes the dynamic usage only.

laanwj commented at 11:08 am on November 5, 2024:

yes, explaining it more would make sense!

but i think it’s out of scope for this PR

in src/memusage.h:87 in b58dab98ab outdated

83@@ -84,8 +84,8 @@ struct stl_shared_counter
84     size_t weak_count;
85 };
86 
87-template<typename X>
88-static inline size_t DynamicUsage(const std::vector<X>& v)
89+template<typename X, typename Y>

l0rinc commented at 9:17 am on November 4, 2024:

Nit: Given that MallocUsage warns that it is *not* recursive, would have been great to add a

0requires std::is_trivial_v<X>

here, but apparently it is used for nested structures - nothing to do here, just documenting my journey.

laanwj commented at 11:12 am on November 5, 2024:

right-it can be used for nested structures, but the call site has to do their own iteration and add it up, as this only covers the outermost layer

adding the std::is_trivial_v check is an interesting idea but would prevent it from being used there.

l0rinc commented at 2:42 pm on November 5, 2024:

In a next PR we could add another DynamicUsage which accepts a vector of vectors and does the iteration instead of the call sites

in src/net.cpp:132 in ad5c012157 outdated

126@@ -127,6 +127,12 @@ size_t CSerializedNetMsg::GetMemoryUsage() const noexcept
127     return sizeof(*this) + memusage::DynamicUsage(data);
128 }
129 
130+size_t CNetMessage::GetMemoryUsage() const noexcept
131+{
132+    // Don't count the dynamic memory used for the m_type string (see CSerializedNetMsg::GetMemoryUsage),

l0rinc commented at 11:32 am on November 4, 2024:

I don’t actually understand why we’re ignoring m_type, the comment doesn’t help.

Is it because of https://github.com/bitcoin/bitcoin/blob/master/src/netmessagemaker.h#L17, i.e. the string being moved around instead of being owned?

laanwj commented at 1:11 pm on November 4, 2024:

The comment in the referred function explains it.

0    // Don't count the dynamic memory used for the m_type string, by assuming it fits in the
1    // "small string" optimization area (which stores data inside the object itself, up to some
2    // size; 15 bytes in modern libstdc++).

It has to do with the internal implementation of std::string, where it has a fixed-size buffer that it will use for short strings (such as the message types) before allocating memory on the heap.

laanwj commented at 1:15 pm on November 4, 2024:

i do agree that over-counting is probably better than under-counting here, but i just took this over from CSerializedNetMsg as i assumed this was the right thing to do. If we change it it needs to be changed in both places.

l0rinc commented at 1:19 pm on November 4, 2024:

Can we measure whether this is indeed the case? What is the reason for assuming it fits in the "small string" optimization area?

sipa commented at 1:25 pm on November 4, 2024:

Message type strings are at most 12 bytes long (that’s the limit in both the v1 and v2 transport protocol), while typical standard library implementation allow this optimization up to 15 bytes.

We could actually test for it exactly though: add a DynamicUsage(const std::string& s) function which checks if s.data() >= &s && s.data() + s.size() - sizeof(s) <= &s, and if so, return 0.

laanwj commented at 1:39 pm on November 4, 2024:

For g++ 13.2 on x86_64 it checks out, this returns 16:

 0#include <string>
 1#include <iostream>
 2
 3bool data_is_internal(const std::string &s)
 4{
 5    return s.data() >= reinterpret_cast<const char*>(&s) && s.data() + s.size() - sizeof(s) <= reinterpret_cast<const char*>(&s);
 6}
 7
 8int main()
 9{
10    std::string s;
11    while (data_is_internal(s)) {
12        s += 'a';
13    }
14    std::cout << s.size() << std::endl;
15    return 0;
16}

But yes it’s not possible to guarantee across c++ compilers and platforms so adding the check to DynamicUsage makes sense imo.

l0rinc commented at 2:15 pm on November 4, 2024:

Alternatively, maybe a test checking this exact scenario could help:

0BOOST_AUTO_TEST_CASE(sso_size_test)
1{
2    CSerializedNetMsg empty_msg;
3
4    CSerializedNetMsg msg_with_type;
5    msg_with_type.m_type = std::string(CMessageHeader::COMMAND_SIZE, 'X');
6
7    BOOST_CHECK_EQUAL(sizeof(msg_with_type), sizeof(empty_msg));
8}

sipa commented at 2:17 pm on November 4, 2024:

@l0rinc That doesn’t test anything. sizeof() is a compile-time property, that just depends on the argument’s type. Since empty_msg and msg_with_type are the same type, the check will always be true.

l0rinc commented at 2:28 pm on November 4, 2024:

Lol, that was a stupid mistake, lemme’ try again:

 0BOOST_AUTO_TEST_CASE(sso_size_test)
 1{
 2  auto is_sso{[](const std::string& s, const CSerializedNetMsg& msg) {
 3      const auto msg_start = reinterpret_cast<const char*>(&msg);
 4      const auto msg_end = msg_start + sizeof(msg);
 5      return msg_start <= s.data() && s.data() <= msg_end;
 6  }};
 7
 8  const CSerializedNetMsg empty_msg;
 9  BOOST_CHECK(is_sso(empty_msg.m_type, empty_msg));
10
11  CSerializedNetMsg msg_with_type;
12  msg_with_type.m_type = std::string(CMessageHeader::COMMAND_SIZE, 'X');
13  BOOST_CHECK(is_sso(msg_with_type.m_type, msg_with_type));
14
15  CSerializedNetMsg msg_with_overflowing_type;
16  msg_with_overflowing_type.m_type = std::string(100 * CMessageHeader::COMMAND_SIZE, 'X');
17  BOOST_CHECK(!is_sso(msg_with_overflowing_type.m_type, msg_with_overflowing_type));
18}

sipa commented at 3:04 pm on November 4, 2024:

@laanwj If you were to actually incorporate that into this PR, be aware you’ll need to use !std::greater{}(a, b) instead of a <= b when comparing unrelated pointers like this (using <= for that is technically unspecified behavior, though it works fine in every real compiler IIRC).

laanwj commented at 3:33 pm on November 4, 2024:

Lol, that was a stupid mistake, lemme’ try again:

i’m kind of hestitant to add a test like that because i’m not sure we want to require this behavior from the C++ library. It’s perfectly valid to compile bitcoind with a library that doesn’t do it, it should just take it into account.

If you were to actually incorporate that into this PR, be aware you’ll need to use !std::greater{}(a, b) instead of a <= b

pointer arithmetic is so scary nowadays…

Would this be safe?

0bool string_uses_sso(const std::string &s)
1{
2    const char* s_ptr = reinterpret_cast<const char*>(&s);
3    return !std::less{}(s.data(), s_ptr) && !std::greater{}(s.data() + s.size(), s_ptr + sizeof(s));
4}

(i moved the sizeof(s) to the right hand side of std::greater because it looks more symmetrical)

l0rinc commented at 3:35 pm on November 4, 2024:

Thanks for the hints, @sipa!

According to https://en.cppreference.com/w/cpp/utility/compare/compare_three_way, this also seems safe to use:

0return (msg_start <=> s.data()) <= 0 && (s.data() <=> msg_end) <= 0;

or

0return (msg_start <=> s.data()) != std::strong_ordering::greater
1      && (s.data() <=> msg_end) != std::strong_ordering::greater;

or

0return !std::less{}(s.data(), msg_start)
1    && !std::less{}(msg_end, s.data());

sipa commented at 3:37 pm on November 4, 2024:

@laanwj My understanding is that that is correct (see https://en.cppreference.com/w/cpp/language/operator_comparison#Pointer_total_order; std::greater{} constructs an object of type std::greater<void>).

l0rinc commented at 3:40 pm on November 4, 2024:

i’m kind of hestitant to add a test like that because i’m not sure we want to require this behavior from the C++ library.

I understood the memory calculations will be incorrect if this would fail - is that not the case?

laanwj commented at 3:54 pm on November 4, 2024:

I understood the memory calculations will be incorrect if this would fail - is that not the case?

Yes, unless we add the logic to memusage.h. i guess i’ve always seen memusage more as a “best effort estimate” than something that needs to be correct to the byte, because it’s so hard to guarantee across implementations, we don’t really want to extend our scope to C++ library implementation specifics. But i dunno.

BTW: It looks like currently. memusage.h doesn’t handle std::string (nor std::basic_string at all. This would be new.

sipa commented at 4:12 pm on November 4, 2024:

All of memusage.h is essentially a best effort to model the standard library. There are inevitably limitations to how good it can be.

My suggestion would be to actually add a DynamicUsage(const std::string&) to memusage. That’s far more obviously correct than just using it in a test, and just as much work. If it happens to be wrong, it won’t be more wrong than what we had before.

laanwj commented at 5:42 pm on November 4, 2024:

Done

in src/net.h:249 in ad5c012157 outdated

244@@ -245,6 +245,9 @@ class CNetMessage
245     CNetMessage(const CNetMessage&) = delete;
246     CNetMessage& operator=(CNetMessage&&) = default;
247     CNetMessage& operator=(const CNetMessage&) = delete;
248+
249+    /** Compute total memory usage of this object (own memory + any dynamic memory). */

l0rinc commented at 11:33 am on November 4, 2024:

given that we’re ignoring m_type, is it still fair to say that we’re counting total memory usage?

laanwj commented at 1:20 pm on November 4, 2024:

It’s the total memory, making the assumption about the underlying implementation of std::string and the length of the message type as defined in the protocol. Could add that here too, sure.

l0rinc commented at 1:22 pm on November 4, 2024:

Don’t need to duplicate it, just want to make sure I understand this exceptional case.

in src/net.cpp:3775 in ad5c012157 outdated

3777@@ -3772,7 +3778,7 @@ void CNode::MarkReceivedMsgsForProcessing()
3778     for (const auto& msg : vRecvMsg) {
3779         // vRecvMsg contains only completed CNetMessage
3780         // the single possible partially deserialized message are held by TransportDeserializer
3781-        nSizeAdded += msg.m_raw_message_size;

l0rinc commented at 11:36 am on November 4, 2024:

we still have a few m_raw_message_size usages, can you please explain why that’s still needed?

laanwj commented at 1:23 pm on November 4, 2024:

It’s still useful for one purpose: to count bytes going over the network for bandwidth monitoring. This icnludes the statistics per message type.

l0rinc commented at 1:24 pm on November 4, 2024:

Got it, thanks for clarifying!

memusage: Allow counting usage of vectors with different allocators 7596282a55

laanwj force-pushed on Nov 4, 2024

memusage: Add DynamicUsage for std::string

Add DynamicUsage(std::string) which Returns the dynamic allocation of a std::string,
or 0 if none (in case of small string optimization).

c6594c0b14

net: Use DynamicUsage(m_type) in CSerializedNetMsg::GetMemoryUsage

Now that memusage correctly computes the dynamic size of a string, there
is no need for special handling here.

c3a6722f34

streams: add DataStream::GetMemoryUsage 047b5e2af1

net: Use actual memory size in receive buffer accounting

Add a method CNetMessage::GetMemoryUsage and use this for accounting of
the size of the process receive queue instead of the raw message size.

This ensures that allocation and deserialization overhead is taken into
account.

d22a234ed2

in src/memusage.h:99 in e9baf7bc65 outdated

89@@ -90,6 +90,18 @@ static inline size_t DynamicUsage(const std::vector<T, Allocator>& v)
90     return MallocUsage(v.capacity() * sizeof(T));
91 }
92 
93+static inline size_t DynamicUsage(const std::string& s)
94+{
95+    const char* s_ptr = reinterpret_cast<const char*>(&s);
96+    // Don't count the dynamic memory used for string, if it resides in the
97+    // "small string" optimization area (which stores data inside the object itself, up to some
98+    // size; 15 bytes in modern libstdc++).

l0rinc commented at 5:44 pm on November 4, 2024:

note: with clang I measured ~24: https://tc-imba.github.io/posts/cpp-sso

laanwj commented at 6:07 pm on November 4, 2024:

interesting! that also suggests that on 32-bit platforms, the size would still be 15 bytes for g++ (because it’s the minimum bound), but 10 (3*4 minus 2 for size byte and zero byte) for clang so it would trigger an allocation for message types of 11/12 chars there 😄

laanwj commented at 11:16 am on November 5, 2024:

i don’t think there’s anything to do here; the code is agnostic to the size and implementation of the sso area; the comment gives an example for one C++ library, to illustrate the point adding more specifics adds more information that could go stale 😄

laanwj force-pushed on Nov 4, 2024

in src/memusage.h:100 in c6594c0b14 outdated

 95+{
 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))) {

l0rinc commented at 5:53 pm on November 4, 2024:

Since I think these are related pointers (unlike #31164 (review)), can we use simple pointer comparisons here? Or do I misunderstand how this works?

0if (s.data() >= s_ptr && s.data() + s.size() <= s_ptr + sizeof(s)) {

laanwj commented at 6:11 pm on November 4, 2024:

as i understand it, in the case that sso is not used, these are not related pointers (they point to different data structures that are allocated seperately)

sipa commented at 6:11 pm on November 4, 2024:

In case the small string optimization is in effect, then s.data() and s_ptr are possibly related (the real question is whether they point into the same allocated array), but if sso is not in effect then they are definitely unrelated.

l0rinc commented at 6:13 pm on November 4, 2024:

Thanks for clarifying.

laanwj commented at 6:25 pm on November 4, 2024:

fwiw the underlying reason for this pointer absurdity is to support platforms with segmented memory (such as GPUs), where pointers to different types of allocations might refer to different address spaces and won’t be comparable probably, std::greater adds some address space prefix to give them a defined order this is a super edge case and unlikely to come up for any platform people compile bitcoind for, but better to be safe i guess…

in src/memusage.h:94 in c6594c0b14 outdated

90@@ -90,6 +91,18 @@ static inline size_t DynamicUsage(const std::vector<T, Allocator>& v)
91     return MallocUsage(v.capacity() * sizeof(T));
92 }
93 
94+static inline size_t DynamicUsage(const std::string& s)

l0rinc commented at 5:54 pm on November 4, 2024:

Could you please add a test in this commit that demonstrates the intended behavior? Or would that tie the implementation down too much?

laanwj commented at 6:17 pm on November 4, 2024:

i guess we could do some basic generalizable test like “the memory usage increases or stays equal when the string becomes longer” but i’m not sure how useful that is, and there are currently no memusage tests to add it to

edit: i think we should leave adding a memusage test suite (if we do) to a seperate PR

laanwj commented at 7:40 pm on November 4, 2024:

i’ve tested it manually (gcc, x86_64) with the following patch:

 0diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp
 1index 8a4c1746161c30c04dee1f41a811740d0038df0b..f8c688dd3617123548af13611519adba59eef775 100644
 2--- a/src/bitcoin-cli.cpp
 3+++ b/src/bitcoin-cli.cpp
 4@@ -23,6 +23,7 @@
 5 #include <util/strencodings.h>
 6 #include <util/time.h>
 7 #include <util/translation.h>
 8+#include <memusage.h>
 9
10 #include <algorithm>
11 #include <chrono>
12@@ -1281,6 +1282,13 @@ static int CommandLineRPC(int argc, char *argv[])
13
14 MAIN_FUNCTION
15 {
16+    std::string s;
17+    std::cout << "length (b)" << "," << "dynsize (b)" << std::endl;
18+    for (int x=0; x<=200; ++x) {
19+        std::cout << x << "," << memusage::DynamicUsage(s) << std::endl;
20+        s += 'a';
21+    }
22+    exit(0);
23 #ifdef WIN32
24     common::WinCmdLineArgs winArgs;
25     std::tie(argc, argv) = winArgs.get();

as expected, for the first 15 bytes it stays flat at 0, then it allocates and the first allocation is 48 bytes, and it increases stepwise as the size of the string grows:

l0rinc commented at 5:59 pm on November 4, 2024: contributor

Concept ACK

l0rinc commented at 2:44 pm on November 5, 2024: contributor

ACK d22a234ed270286b483aec2db1e2f716b9756231

DrahtBot requested review from theStack on Nov 5, 2024

DrahtBot requested review from i-am-yuvi on Nov 5, 2024

i-am-yuvi approved

i-am-yuvi commented at 9:20 am on November 6, 2024: none

ACK d22a234ed270286b483aec2db1e2f716b9756231

danielabrozzoni approved

danielabrozzoni commented at 2:42 pm on November 6, 2024: contributor

Light ACK d22a234ed270286b483aec2db1e2f716b9756231 - code looks good to me, but I’m not very familiar with C++ memory management specifics

achow101 commented at 8:48 pm on November 6, 2024: member

ACK d22a234ed270286b483aec2db1e2f716b9756231

achow101 merged this on Nov 6, 2024

achow101 closed this on Nov 6, 2024

theStack commented at 7:34 am on November 7, 2024: contributor

post-merge ACK d22a234ed270286b483aec2db1e2f716b9756231

net: Use actual memory size in receive buffer accounting #31164

Code Coverage & Benchmarks

Reviews

Conflicts