From 3fcd7fc7ff563bdc0e2bba66b4cbe72d898c876e Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 11 Sep 2023 13:54:32 -0400 Subject: Do not use std::vector = {} to release memory --- src/headerssync.cpp | 5 +++-- src/net.cpp | 23 +++++++++++------------ src/test/util_tests.cpp | 25 +++++++++++++++++++++++++ src/util/vector.h | 18 ++++++++++++++++++ 4 files changed, 57 insertions(+), 14 deletions(-) (limited to 'src') diff --git a/src/headerssync.cpp b/src/headerssync.cpp index a3adfb4f70..f891063cd2 100644 --- a/src/headerssync.cpp +++ b/src/headerssync.cpp @@ -7,6 +7,7 @@ #include #include #include +#include // The two constants below are computed using the simulation script on // https://gist.github.com/sipa/016ae445c132cdf65a2791534dfb7ae1 @@ -51,9 +52,9 @@ HeadersSyncState::HeadersSyncState(NodeId id, const Consensus::Params& consensus void HeadersSyncState::Finalize() { Assume(m_download_state != State::FINAL); - m_header_commitments = {}; + ClearShrink(m_header_commitments); m_last_header_received.SetNull(); - m_redownloaded_headers = {}; + ClearShrink(m_redownloaded_headers); m_redownload_buffer_last_hash.SetNull(); m_redownload_buffer_first_prev_hash.SetNull(); m_process_all_remaining_headers = false; diff --git a/src/net.cpp b/src/net.cpp index af2855932d..13397c3d81 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -35,6 +35,7 @@ #include #include #include +#include #ifdef WIN32 #include @@ -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); } } diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 26677bfa55..67f71bd266 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -1791,4 +1791,29 @@ BOOST_AUTO_TEST_CASE(util_WriteBinaryFile) BOOST_CHECK(valid); BOOST_CHECK_EQUAL(actual_text, expected_text); } + +BOOST_AUTO_TEST_CASE(clearshrink_test) +{ + { + std::vector v = {1, 2, 3}; + ClearShrink(v); + BOOST_CHECK_EQUAL(v.size(), 0); + BOOST_CHECK_EQUAL(v.capacity(), 0); + } + + { + std::vector v = {false, true, false, false, true, true}; + ClearShrink(v); + BOOST_CHECK_EQUAL(v.size(), 0); + BOOST_CHECK_EQUAL(v.capacity(), 0); + } + + { + std::deque v = {1, 3, 3, 7}; + ClearShrink(v); + BOOST_CHECK_EQUAL(v.size(), 0); + // std::deque has no capacity() we can observe. + } +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/util/vector.h b/src/util/vector.h index 9b9218e54f..40ff73c293 100644 --- a/src/util/vector.h +++ b/src/util/vector.h @@ -49,4 +49,22 @@ inline V Cat(V v1, const V& v2) return v1; } +/** Clear a vector (or std::deque) and release its allocated memory. */ +template +inline void ClearShrink(V& v) noexcept +{ + // There are various ways to clear a vector and release its memory: + // + // 1. V{}.swap(v) + // 2. v = V{} + // 3. v = {}; v.shrink_to_fit(); + // 4. v.clear(); v.shrink_to_fit(); + // + // (2) does not appear to release memory in glibc debug mode, even if v.shrink_to_fit() + // follows. (3) and (4) rely on std::vector::shrink_to_fit, which is only a non-binding + // request. Therefore, we use method (1). + + V{}.swap(v); +} + #endif // BITCOIN_UTIL_VECTOR_H -- cgit v1.2.3