From a6f50e791e9c47a0e3b7a149dcdd9424daa88cd1 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Thu, 23 May 2024 01:29:08 +0200 Subject: [PATCH 1/3] test: ensure that `fill_mempool` leaves some room in mempool Without this change, fill_mempool() may leave the mempool very close to its memory size limit (-maxmempool). This can cause tests to incorrectly fail when they submit another transaction expecting it to succeed. Note that without this change, the same test that fails on one platform may succeed on another, because their memory allocation accounting algorithms (how they calculate memory usage, that is, MallocUsage()) may be slightly different. --- test/functional/test_framework/mempool_util.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/test/functional/test_framework/mempool_util.py b/test/functional/test_framework/mempool_util.py index 0e9c821e2ea..54b734fb6de 100644 --- a/test/functional/test_framework/mempool_util.py +++ b/test/functional/test_framework/mempool_util.py @@ -57,16 +57,17 @@ def fill_mempool(test_framework, node, *, tx_sync_fun=None): # Generate UTXOs to flood the mempool # 1 to create a tx initially that will be evicted from the mempool later # 75 transactions each with a fee rate higher than the previous one + # 1 to create a final middle-sized tx to evict a filler tx and create room (if needed) ephemeral_miniwallet = MiniWallet(node, tag_name="fill_mempool_ephemeral_wallet") - test_framework.generate(ephemeral_miniwallet, 1 + num_of_batches * tx_batch_size) + test_framework.generate(ephemeral_miniwallet, 2 + num_of_batches * tx_batch_size) # Mine enough blocks so that the UTXOs are allowed to be spent test_framework.generate(node, COINBASE_MATURITY - 1) # Get all UTXOs up front to ensure none of the transactions spend from each other, as that may # change their effective feerate and thus the order in which they are selected for eviction. - confirmed_utxos = [ephemeral_miniwallet.get_utxo(confirmed_only=True) for _ in range(num_of_batches * tx_batch_size + 1)] - assert_equal(len(confirmed_utxos), num_of_batches * tx_batch_size + 1) + confirmed_utxos = [ephemeral_miniwallet.get_utxo(confirmed_only=True) for _ in range(num_of_batches * tx_batch_size + 2)] + assert_equal(len(confirmed_utxos), num_of_batches * tx_batch_size + 2) test_framework.log.debug("Create a mempool tx that will be evicted") tx_to_be_evicted_id = ephemeral_miniwallet.send_self_transfer( @@ -92,6 +93,14 @@ def fill_mempool(test_framework, node, *, tx_sync_fun=None): send_batch(fee) tx_sync_fun() if tx_sync_fun else test_framework.sync_mempools() # sync after all evictions + # If the mempool is almost full (<10k usage bytes left), submit one extra middle-sized tx, + # in order to evict a large filler tx and leave some room; this will enable tests to submit + # txs with just the mempoolminfee without immediate eviction ("mempool full" error). + if node.getmempoolinfo()['usage'] >= 4_990_000: + ephemeral_miniwallet.send_self_transfer(from_node=node, fee=num_of_batches * (base_fee / 2), + utxo_to_spend=confirmed_utxos.pop(0), target_weight = 32500 * 4) + assert_greater_than(4_990_000, node.getmempoolinfo()['usage']) + test_framework.log.debug("The tx should be evicted by now") # The number of transactions created should be greater than the ones present in the mempool assert_greater_than(tx_batch_size * num_of_batches, len(node.getrawmempool())) From f389bd8358828836aa58bed4ca67076306e4279b Mon Sep 17 00:00:00 2001 From: Larry Ruane Date: Thu, 27 Mar 2025 13:23:25 -0600 Subject: [PATCH 2/3] test: fill_mempool() should call sync_mempools() before returning We saw a case where a test (p2p_1p1c_network.py) called raise_network_minfee(), which called fill_mempool() using node 0. Then raise_network_minfee() returned, and the test called rescan_utxos(), which called getrawmempool() using a different node (node 1) followed by getrawtransaction() on each returned transaction, and the test asserted because a transaction was not found. This was caused by the timing window between the call to getrawmempool() and fetching the individual transactions; the transactions were still being propagated on the P2P network. During this window, a transaction (returned by getrawmempool()) was evicted (the mempool is close to full during this test), and did not exist in the mempool by the time it was attempted to be fetched. It might make more sense for rescan_utxos() to call sync_mempools() just before calling getrawmempool(), but it can't because rescan_utxos() is part of the MiniWallet class, which doesn't have access to test_framework (but that could probably be changed). --- test/functional/test_framework/mempool_util.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/functional/test_framework/mempool_util.py b/test/functional/test_framework/mempool_util.py index 54b734fb6de..bffb297399c 100644 --- a/test/functional/test_framework/mempool_util.py +++ b/test/functional/test_framework/mempool_util.py @@ -110,6 +110,7 @@ def fill_mempool(test_framework, node, *, tx_sync_fun=None): test_framework.log.debug("Check that mempoolminfee is larger than minrelaytxfee") assert_equal(node.getmempoolinfo()['minrelaytxfee'], Decimal('0.00001000')) assert_greater_than(node.getmempoolinfo()['mempoolminfee'], Decimal('0.00001000')) + test_framework.sync_mempools() def tx_in_orphanage(node, tx: CTransaction) -> bool: """Returns true if the transaction is in the orphanage.""" From 77ebad9651cd9138cf385c84402fd6042db8536e Mon Sep 17 00:00:00 2001 From: Larry Ruane Date: Mon, 25 Sep 2023 14:14:39 -0600 Subject: [PATCH 3/3] improve MallocUsage() accuracy Co-authored-by: Pieter Wuille Co-authored-by: Martin Leitner-Ankerl Co-authored-by: l0rinc --- src/memusage.h | 45 +++++++++++-------- src/test/validation_flush_tests.cpp | 2 +- .../functional/test_framework/mempool_util.py | 2 +- 3 files changed, 29 insertions(+), 20 deletions(-) diff --git a/src/memusage.h b/src/memusage.h index 9d9e549ef22..8119bc0f8b7 100644 --- a/src/memusage.h +++ b/src/memusage.h @@ -24,9 +24,6 @@ namespace memusage { -/** Compute the total memory used by allocating alloc bytes. */ -static size_t MallocUsage(size_t alloc); - /** Dynamic memory usage for built-in types is zero. */ static inline size_t DynamicUsage(const int8_t& v) { return 0; } static inline size_t DynamicUsage(const uint8_t& v) { return 0; } @@ -48,19 +45,21 @@ template static inline size_t DynamicUsage(const X * const &v) { ret * application data structures require more accurate inner accounting, they should * iterate themselves, or use more efficient caching + updating on modification. */ - -static inline size_t MallocUsage(size_t alloc) +static constexpr size_t MallocUsage(size_t alloc) { - // Measured on libc6 2.19 on Linux. - if (alloc == 0) { - return 0; - } else if (sizeof(void*) == 8) { - return ((alloc + 31) >> 4) << 4; - } else if (sizeof(void*) == 4) { - return ((alloc + 15) >> 3) << 3; - } else { - assert(0); - } + if (alloc == 0) return 0; + +#if defined(__arm__) || SIZE_MAX == UINT64_MAX + constexpr size_t min_alloc{9}; +#else + constexpr size_t min_alloc{0}; +#endif + constexpr size_t overhead{sizeof(size_t)}; + constexpr size_t step{alignof(std::max_align_t)}; + // step should be a nonzero power of 2 (exactly one bit set) + static_assert(step > 0 && (step & (step - 1)) == 0); + + return (std::max(min_alloc, alloc) + overhead + (step - 1)) & ~(step - 1); } // STL data structures @@ -177,23 +176,33 @@ static inline size_t DynamicUsage(const std::list& l) return MallocUsage(sizeof(list_node)) * l.size(); } +// Empirically, an std::unordered_map node has two pointers (likely +// forward and backward pointers) on some platforms (Windows and macOS), +// so be conservative in estimating memory usage by assuming this is +// the case for all platforms. template struct unordered_node : private X { private: void* ptr; + void* ptr2; }; +// The memory used by an unordered_set or unordered map is the sum of the +// sizes of the individual nodes (which are separately allocated) plus +// the size of the bucket array (which is a single allocation). +// Empirically, each element of the bucket array consists of two pointers +// on some platforms (Windows and macOS), so be conservative. template static inline size_t DynamicUsage(const std::unordered_set& s) { - return MallocUsage(sizeof(unordered_node)) * s.size() + MallocUsage(sizeof(void*) * s.bucket_count()); + return MallocUsage(sizeof(unordered_node)) * s.size() + MallocUsage(2 * sizeof(void*) * s.bucket_count()); } template static inline size_t DynamicUsage(const std::unordered_map& m) { - return MallocUsage(sizeof(unordered_node >)) * m.size() + MallocUsage(sizeof(void*) * m.bucket_count()); + return MallocUsage(sizeof(unordered_node >)) * m.size() + MallocUsage(2 * sizeof(void*) * m.bucket_count()); } template @@ -212,7 +221,7 @@ static inline size_t DynamicUsage(const std::unordered_mapNumAllocatedChunks(); size_t usage_chunks = MallocUsage(pool_resource->ChunkSizeBytes()) * pool_resource->NumAllocatedChunks(); - return usage_resource + usage_chunks + MallocUsage(sizeof(void*) * m.bucket_count()); + return usage_resource + usage_chunks + MallocUsage(2 * sizeof(void*) * m.bucket_count()); } } // namespace memusage diff --git a/src/test/validation_flush_tests.cpp b/src/test/validation_flush_tests.cpp index c325f7deb2b..bab0b782d45 100644 --- a/src/test/validation_flush_tests.cpp +++ b/src/test/validation_flush_tests.cpp @@ -30,7 +30,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate) // (prevector<28, unsigned char>) when assigned 56 bytes of data per above. // // See also: Coin::DynamicMemoryUsage(). - constexpr unsigned int COIN_SIZE = is_64_bit ? 80 : 64; + constexpr size_t COIN_SIZE{64}; auto print_view_mem_usage = [](CCoinsViewCache& view) { BOOST_TEST_MESSAGE("CCoinsViewCache memory usage: " << view.DynamicMemoryUsage()); diff --git a/test/functional/test_framework/mempool_util.py b/test/functional/test_framework/mempool_util.py index bffb297399c..441686f4f22 100644 --- a/test/functional/test_framework/mempool_util.py +++ b/test/functional/test_framework/mempool_util.py @@ -98,7 +98,7 @@ def fill_mempool(test_framework, node, *, tx_sync_fun=None): # txs with just the mempoolminfee without immediate eviction ("mempool full" error). if node.getmempoolinfo()['usage'] >= 4_990_000: ephemeral_miniwallet.send_self_transfer(from_node=node, fee=num_of_batches * (base_fee / 2), - utxo_to_spend=confirmed_utxos.pop(0), target_weight = 32500 * 4) + utxo_to_spend=confirmed_utxos.pop(0), target_vsize = 32500) assert_greater_than(4_990_000, node.getmempoolinfo()['usage']) test_framework.log.debug("The tx should be evicted by now")