highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-02-04 06:12:07 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#28452: Do not use std::vector = {} to release memory

Browse Source 
3fcd7fc7ff Do not use std::vector = {} to release memory (Pieter Wuille)

Pull request description:

  It appears that invoking `v = {};` for an `std::vector<...> v` is equivalent to `v.clear()`, which does not release its allocated memory. There are a number of places in the codebase where it appears to be used for that purpose however (mostly written by me). Replace those with `std::vector<...>{}.swap(v);` (using a helper function `ClearShrink` in util/vector.h).

  To explain what is going on: `v = {...};` is equivalent in general to `v.operator=({...});`. For many types, the `{}` is converted to the type of `v`, and then assigned to `v` - which for `std::vector` would ordinarily have the effect of clearing its memory (constructing a new empty vector, and then move-assigning it to `v`). However, since `std::vector<T>` has an `operator=(std::initializer_list<T>)` defined, it has precedence (since no implicit conversion is needed), and with an empty list, that is equivalent to `clear()`.

  I did consider using `v = std::vector<T>{};` as replacement for `v = {};` instances where memory releasing is desired, but it appears that it does not actually work universally either. `V{}.swap(v);` does.

ACKs for top commit:
  ajtowns:
    utACK 3fcd7fc7ff
  stickies-v:
    ACK 3fcd7fc7ff
  theStack:
    Code-review ACK 3fcd7fc7ff

Tree-SHA512: 6148558126ec3c8cfd6daee167ec1c67b360cf1dff2cbc132bd71768337cf9bc4dda3e5a9cf7da4f7457d2123288eeba77dd78f3a17fa2cfd9c6758262950cc5
This commit is contained in:
fanquake
2023-09-15 09:47:59 +01:00
parent 459272d639 3fcd7fc7ff
commit 8ef672937e
4 changed files with 57 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
src/net.cpp
View File

@@ -35,6 +35,7 @@
#include <util/threadinterrupt.h>
#include <util/trace.h>
#include <util/translation.h>
#include <util/vector.h>
#ifdef WIN32
#include <string.h>
@@ -899,8 +900,7 @@ void V1Transport::MarkBytesSent(size_t bytes_sent) noexcept
m_bytes_sent = 0;
} else if (!m_sending_header && m_bytes_sent == m_message_to_send.data.size()) {
// We're done sending a message's data. Wipe the data vector to reduce memory consumption.
m_message_to_send.data.clear();
m_message_to_send.data.shrink_to_fit();
ClearShrink(m_message_to_send.data);
m_bytes_sent = 0;
}
}
@@ -1123,8 +1123,8 @@ void V2Transport::ProcessReceivedMaybeV1Bytes() noexcept
SetReceiveState(RecvState::V1);
SetSendState(SendState::V1);
// Reset v2 transport buffers to save memory.
m_recv_buffer = {};
m_send_buffer = {};
ClearShrink(m_recv_buffer);
ClearShrink(m_send_buffer);
} else {
// We have not received enough to distinguish v1 from v2 yet. Wait until more bytes come.
}
@@ -1184,8 +1184,7 @@ bool V2Transport::ProcessReceivedKeyBytes() noexcept
/*ignore=*/false,
/*output=*/MakeWritableByteSpan(m_send_buffer).last(BIP324Cipher::EXPANSION));
// We no longer need the garbage.
m_send_garbage.clear();
m_send_garbage.shrink_to_fit();
ClearShrink(m_send_garbage);
// Construct version packet in the send buffer.
m_send_buffer.resize(m_send_buffer.size() + BIP324Cipher::EXPANSION + VERSION_CONTENTS.size());
@@ -1275,7 +1274,7 @@ bool V2Transport::ProcessReceivedPacketBytes() noexcept
// Ignore flag does not matter for garbage authentication. Any valid packet functions
// as authentication. Receive and process the version packet next.
SetReceiveState(RecvState::VERSION);
m_recv_garbage = {};
ClearShrink(m_recv_garbage);
break;
case RecvState::VERSION:
if (!ignore) {
@@ -1295,9 +1294,9 @@ bool V2Transport::ProcessReceivedPacketBytes() noexcept
Assume(false);
}
// Wipe the receive buffer where the next packet will be received into.
m_recv_buffer = {};
ClearShrink(m_recv_buffer);
// In all but APP_READY state, we can wipe the decoded contents.
if (m_recv_state != RecvState::APP_READY) m_recv_decode_buffer = {};
if (m_recv_state != RecvState::APP_READY) ClearShrink(m_recv_decode_buffer);
} else {
// We either have less than 3 bytes, so we don't know the packet's length yet, or more
// than 3 bytes but less than the packet's full ciphertext. Wait until those arrive.
@@ -1511,7 +1510,7 @@ CNetMessage V2Transport::GetReceivedMessage(std::chrono::microseconds time, bool
LogPrint(BCLog::NET, "V2 transport error: invalid message type (%u bytes contents), peer=%d\n", m_recv_decode_buffer.size(), m_nodeid);
reject_message = true;
}
m_recv_decode_buffer = {};
ClearShrink(m_recv_decode_buffer);
SetReceiveState(RecvState::APP);
return msg;
@@ -1545,7 +1544,7 @@ bool V2Transport::SetMessageToSend(CSerializedNetMsg& msg) noexcept
m_cipher.Encrypt(MakeByteSpan(contents), {}, false, MakeWritableByteSpan(m_send_buffer));
m_send_type = msg.m_type;
// Release memory
msg.data = {};
ClearShrink(msg.data);
return true;
}
@@ -1577,7 +1576,7 @@ void V2Transport::MarkBytesSent(size_t bytes_sent) noexcept
// Wipe the buffer when everything is sent.
if (m_send_pos == m_send_buffer.size()) {
m_send_pos = 0;
m_send_buffer = {};
ClearShrink(m_send_buffer);
}
}