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

pull laanwj wants to merge 5 commits into bitcoin:master from laanwj:2024-10-p2p-receive-buffer changing 5 files +36 −9
  1. 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
    
  2. laanwj added the label P2P on Oct 27, 2024
  3. 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.

  4. jarolrod commented at 8:50 am on October 29, 2024: member
    Concept ACK
  5. theStack commented at 2:58 pm on October 30, 2024: contributor
    Concept ACK
  6. theStack approved
  7. 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);
    
  8. DrahtBot requested review from jarolrod on Nov 1, 2024
  9. i-am-yuvi approved
  10. 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
    
  11. 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

  12. 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

  13. 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
  14. 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
  15. 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.
  16. 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!
  17. memusage: Allow counting usage of vectors with different allocators 7596282a55
  18. laanwj force-pushed on Nov 4, 2024
  19. 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
  20. 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
  21. streams: add DataStream::GetMemoryUsage 047b5e2af1
  22. 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
  23. 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 😄
  24. laanwj force-pushed on Nov 4, 2024
  25. 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…
  26. 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: image

  27. l0rinc commented at 5:59 pm on November 4, 2024: contributor
    Concept ACK
  28. l0rinc commented at 2:44 pm on November 5, 2024: contributor
    ACK d22a234ed270286b483aec2db1e2f716b9756231
  29. DrahtBot requested review from theStack on Nov 5, 2024
  30. DrahtBot requested review from i-am-yuvi on Nov 5, 2024
  31. i-am-yuvi approved
  32. i-am-yuvi commented at 9:20 am on November 6, 2024: none
    ACK d22a234ed270286b483aec2db1e2f716b9756231
  33. danielabrozzoni approved
  34. 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
  35. achow101 commented at 8:48 pm on November 6, 2024: member
    ACK d22a234ed270286b483aec2db1e2f716b9756231
  36. achow101 merged this on Nov 6, 2024
  37. achow101 closed this on Nov 6, 2024

  38. theStack commented at 7:34 am on November 7, 2024: contributor
    post-merge ACK d22a234ed270286b483aec2db1e2f716b9756231

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-11 06:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me