Skip to content

Commit 287640f

Browse files
sipaPiRK
authored andcommitted
Do not use std::vector = {} to release memory
Summary: > 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). For now the net.cpp changes are not applicable, as they depend on V2Transport backports. The change is useful anyway even if we just apply it to the headersync data. IIUC we could save multiple megabytes of memory, that would otherwsise not be freed during the entire lifetime of the connection, per peer involved in header pre-sync . `m_redownloaded_headers` is a buffer that can stores block headers during the redownload phase of header synchronization. This is a backport of [[bitcoin/bitcoin#28452 | core#28452]] Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D18446
1 parent 0314c7b commit 287640f

File tree

3 files changed

+43
-2
lines changed

3 files changed

+43
-2
lines changed

src/headerssync.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#include <pow/pow.h>
88
#include <timedata.h>
99
#include <util/check.h>
10+
#include <util/vector.h>
1011

1112
// The two constants below are computed using the simulation script on
1213
// https://gist.github.com/sipa/016ae445c132cdf65a2791534dfb7ae1
@@ -65,9 +66,9 @@ HeadersSyncState::HeadersSyncState(NodeId id,
6566
*/
6667
void HeadersSyncState::Finalize() {
6768
Assume(m_download_state != State::FINAL);
68-
m_header_commitments = {};
69+
ClearShrink(m_header_commitments);
6970
m_last_header_received.SetNull();
70-
m_redownloaded_headers = {};
71+
ClearShrink(m_redownloaded_headers);
7172
m_redownload_buffer_last_hash.SetNull();
7273
m_redownload_buffer_first_prev_hash.SetNull();
7374
m_process_all_remaining_headers = false;

src/test/util_tests.cpp

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2924,4 +2924,27 @@ BOOST_AUTO_TEST_CASE(remove_prefix) {
29242924
BOOST_CHECK_EQUAL(RemovePrefix("", ""), "");
29252925
}
29262926

2927+
BOOST_AUTO_TEST_CASE(clearshrink_test) {
2928+
{
2929+
std::vector<uint8_t> v = {1, 2, 3};
2930+
ClearShrink(v);
2931+
BOOST_CHECK_EQUAL(v.size(), 0);
2932+
BOOST_CHECK_EQUAL(v.capacity(), 0);
2933+
}
2934+
2935+
{
2936+
std::vector<bool> v = {false, true, false, false, true, true};
2937+
ClearShrink(v);
2938+
BOOST_CHECK_EQUAL(v.size(), 0);
2939+
BOOST_CHECK_EQUAL(v.capacity(), 0);
2940+
}
2941+
2942+
{
2943+
std::deque<int> v = {1, 3, 3, 7};
2944+
ClearShrink(v);
2945+
BOOST_CHECK_EQUAL(v.size(), 0);
2946+
// std::deque has no capacity() we can observe.
2947+
}
2948+
}
2949+
29272950
BOOST_AUTO_TEST_SUITE_END()

src/util/vector.h

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,23 @@ template <typename V> inline V Cat(V v1, const V &v2) {
4848
return v1;
4949
}
5050

51+
/** Clear a vector (or std::deque) and release its allocated memory. */
52+
template <typename V> inline void ClearShrink(V &v) noexcept {
53+
// There are various ways to clear a vector and release its memory:
54+
//
55+
// 1. V{}.swap(v)
56+
// 2. v = V{}
57+
// 3. v = {}; v.shrink_to_fit();
58+
// 4. v.clear(); v.shrink_to_fit();
59+
//
60+
// (2) does not appear to release memory in glibc debug mode, even if
61+
// v.shrink_to_fit() follows. (3) and (4) rely on
62+
// std::vector::shrink_to_fit, which is only a non-binding request.
63+
// Therefore, we use method (1).
64+
65+
V{}.swap(v);
66+
}
67+
5168
template <typename V, typename L>
5269
inline std::optional<V> FindFirst(const std::vector<V> &vec, const L fnc) {
5370
for (const auto &el : vec) {

0 commit comments

Comments
 (0)