diff options
-rw-r--r-- | src/coins.cpp | 6 | ||||
-rw-r--r-- | src/coins.h | 20 | ||||
-rw-r--r-- | src/test/coins_tests.cpp | 39 | ||||
-rw-r--r-- | src/test/fuzz/coins_view.cpp | 3 | ||||
-rw-r--r-- | src/test/validation_flush_tests.cpp | 28 |
5 files changed, 79 insertions, 17 deletions
diff --git a/src/coins.cpp b/src/coins.cpp index f55932f302..0fe642e46b 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -34,7 +34,7 @@ size_t CCoinsViewBacked::EstimateSize() const { return base->EstimateSize(); } CCoinsViewCache::CCoinsViewCache(CCoinsView* baseIn, bool deterministic) : CCoinsViewBacked(baseIn), m_deterministic(deterministic), - cacheCoins(0, SaltedOutpointHasher(/*deterministic=*/deterministic)) + cacheCoins(0, SaltedOutpointHasher(/*deterministic=*/deterministic), CCoinsMap::key_equal{}, &m_cache_coins_memory_resource) {} size_t CCoinsViewCache::DynamicMemoryUsage() const { @@ -317,7 +317,9 @@ void CCoinsViewCache::ReallocateCache() // Cache should be empty when we're calling this. assert(cacheCoins.size() == 0); cacheCoins.~CCoinsMap(); - ::new (&cacheCoins) CCoinsMap(0, SaltedOutpointHasher(/*deterministic=*/m_deterministic)); + m_cache_coins_memory_resource.~CCoinsMapMemoryResource(); + ::new (&m_cache_coins_memory_resource) CCoinsMapMemoryResource{}; + ::new (&cacheCoins) CCoinsMap{0, SaltedOutpointHasher{/*deterministic=*/m_deterministic}, CCoinsMap::key_equal{}, &m_cache_coins_memory_resource}; } void CCoinsViewCache::SanityCheck() const diff --git a/src/coins.h b/src/coins.h index dd336b210a..039a07054d 100644 --- a/src/coins.h +++ b/src/coins.h @@ -11,6 +11,7 @@ #include <memusage.h> #include <primitives/transaction.h> #include <serialize.h> +#include <support/allocators/pool.h> #include <uint256.h> #include <util/hasher.h> @@ -131,7 +132,23 @@ struct CCoinsCacheEntry CCoinsCacheEntry(Coin&& coin_, unsigned char flag) : coin(std::move(coin_)), flags(flag) {} }; -typedef std::unordered_map<COutPoint, CCoinsCacheEntry, SaltedOutpointHasher> CCoinsMap; +/** + * PoolAllocator's MAX_BLOCK_SIZE_BYTES parameter here uses sizeof the data, and adds the size + * of 4 pointers. We do not know the exact node size used in the std::unordered_node implementation + * because it is implementation defined. Most implementations have an overhead of 1 or 2 pointers, + * so nodes can be connected in a linked list, and in some cases the hash value is stored as well. + * Using an additional sizeof(void*)*4 for MAX_BLOCK_SIZE_BYTES should thus be sufficient so that + * all implementations can allocate the nodes from the PoolAllocator. + */ +using CCoinsMap = std::unordered_map<COutPoint, + CCoinsCacheEntry, + SaltedOutpointHasher, + std::equal_to<COutPoint>, + PoolAllocator<std::pair<const COutPoint, CCoinsCacheEntry>, + sizeof(std::pair<const COutPoint, CCoinsCacheEntry>) + sizeof(void*) * 4, + alignof(void*)>>; + +using CCoinsMapMemoryResource = CCoinsMap::allocator_type::ResourceType; /** Cursor for iterating over CoinsView state */ class CCoinsViewCursor @@ -220,6 +237,7 @@ protected: * declared as "const". */ mutable uint256 hashBlock; + mutable CCoinsMapMemoryResource m_cache_coins_memory_resource{}; mutable CCoinsMap cacheCoins; /* Cached dynamic memory usage for the inner Coin objects. */ diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index e082800fc3..853dc6dc1e 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -6,6 +6,7 @@ #include <coins.h> #include <script/standard.h> #include <streams.h> +#include <test/util/poolresourcetester.h> #include <test/util/random.h> #include <test/util/setup_common.h> #include <txdb.h> @@ -612,7 +613,8 @@ void GetCoinsMapEntry(const CCoinsMap& map, CAmount& value, char& flags, const C void WriteCoinsViewEntry(CCoinsView& view, CAmount value, char flags) { - CCoinsMap map; + CCoinsMapMemoryResource resource; + CCoinsMap map{0, CCoinsMap::hasher{}, CCoinsMap::key_equal{}, &resource}; InsertCoinsMapEntry(map, value, flags); BOOST_CHECK(view.BatchWrite(map, {})); } @@ -911,6 +913,7 @@ void TestFlushBehavior( CAmount value; char flags; size_t cache_usage; + size_t cache_size; auto flush_all = [&all_caches](bool erase) { // Flush in reverse order to ensure that flushes happen from children up. @@ -935,6 +938,8 @@ void TestFlushBehavior( view->AddCoin(outp, Coin(coin), false); cache_usage = view->DynamicMemoryUsage(); + cache_size = view->map().size(); + // `base` shouldn't have coin (no flush yet) but `view` should have cached it. BOOST_CHECK(!base.HaveCoin(outp)); BOOST_CHECK(view->HaveCoin(outp)); @@ -949,6 +954,7 @@ void TestFlushBehavior( // CoinsMap usage should be unchanged since we didn't erase anything. BOOST_CHECK_EQUAL(cache_usage, view->DynamicMemoryUsage()); + BOOST_CHECK_EQUAL(cache_size, view->map().size()); // --- 3. Ensuring the entry still exists in the cache and has been written to parent // @@ -965,8 +971,10 @@ void TestFlushBehavior( // flush_all(/*erase=*/ true); - // Memory usage should have gone down. - BOOST_CHECK(view->DynamicMemoryUsage() < cache_usage); + // Memory does not necessarily go down due to the map using a memory pool + BOOST_TEST(view->DynamicMemoryUsage() <= cache_usage); + // Size of the cache must go down though + BOOST_TEST(view->map().size() < cache_size); // --- 5. Ensuring the entry is no longer in the cache // @@ -1076,4 +1084,29 @@ BOOST_AUTO_TEST_CASE(ccoins_flush_behavior) } } +BOOST_AUTO_TEST_CASE(coins_resource_is_used) +{ + CCoinsMapMemoryResource resource; + PoolResourceTester::CheckAllDataAccountedFor(resource); + + { + CCoinsMap map{0, CCoinsMap::hasher{}, CCoinsMap::key_equal{}, &resource}; + BOOST_TEST(memusage::DynamicUsage(map) >= resource.ChunkSizeBytes()); + + map.reserve(1000); + + // The resource has preallocated a chunk, so we should have space for at several nodes without the need to allocate anything else. + const auto usage_before = memusage::DynamicUsage(map); + + COutPoint out_point{}; + for (size_t i = 0; i < 1000; ++i) { + out_point.n = i; + map[out_point]; + } + BOOST_TEST(usage_before == memusage::DynamicUsage(map)); + } + + PoolResourceTester::CheckAllDataAccountedFor(resource); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp index e80c772aa4..5843b80c0a 100644 --- a/src/test/fuzz/coins_view.cpp +++ b/src/test/fuzz/coins_view.cpp @@ -115,7 +115,8 @@ FUZZ_TARGET_INIT(coins_view, initialize_coins_view) random_mutable_transaction = *opt_mutable_transaction; }, [&] { - CCoinsMap coins_map; + CCoinsMapMemoryResource resource; + CCoinsMap coins_map{0, SaltedOutpointHasher{/*deterministic=*/true}, CCoinsMap::key_equal{}, &resource}; LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 10000) { CCoinsCacheEntry coins_cache_entry; coins_cache_entry.flags = fuzzed_data_provider.ConsumeIntegral<unsigned char>(); diff --git a/src/test/validation_flush_tests.cpp b/src/test/validation_flush_tests.cpp index 205164b94c..7398091215 100644 --- a/src/test/validation_flush_tests.cpp +++ b/src/test/validation_flush_tests.cpp @@ -36,12 +36,12 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate) BOOST_TEST_MESSAGE("CCoinsViewCache memory usage: " << view.DynamicMemoryUsage()); }; - constexpr size_t MAX_COINS_CACHE_BYTES = 1024; + // PoolResource defaults to 256 KiB that will be allocated, so we'll take that and make it a bit larger. + constexpr size_t MAX_COINS_CACHE_BYTES = 262144 + 512; // Without any coins in the cache, we shouldn't need to flush. - BOOST_CHECK_EQUAL( - chainstate.GetCoinsCacheSizeState(MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes=*/0), - CoinsCacheSizeState::OK); + BOOST_TEST( + chainstate.GetCoinsCacheSizeState(MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes=*/ 0) != CoinsCacheSizeState::CRITICAL); // If the initial memory allocations of cacheCoins don't match these common // cases, we can't really continue to make assertions about memory usage. @@ -71,13 +71,21 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate) // cacheCoins (unordered_map) preallocates. constexpr int COINS_UNTIL_CRITICAL{3}; + // no coin added, so we have plenty of space left. + BOOST_CHECK_EQUAL( + chainstate.GetCoinsCacheSizeState(MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0), + CoinsCacheSizeState::OK); + for (int i{0}; i < COINS_UNTIL_CRITICAL; ++i) { const COutPoint res = AddTestCoin(view); print_view_mem_usage(view); BOOST_CHECK_EQUAL(view.AccessCoin(res).DynamicMemoryUsage(), COIN_SIZE); + + // adding first coin causes the MemoryResource to allocate one 256 KiB chunk of memory, + // pushing us immediately over to LARGE BOOST_CHECK_EQUAL( - chainstate.GetCoinsCacheSizeState(MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes=*/0), - CoinsCacheSizeState::OK); + chainstate.GetCoinsCacheSizeState(MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes=*/ 0), + CoinsCacheSizeState::LARGE); } // Adding some additional coins will push us over the edge to CRITICAL. @@ -94,16 +102,16 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate) chainstate.GetCoinsCacheSizeState(MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes=*/0), CoinsCacheSizeState::CRITICAL); - // Passing non-zero max mempool usage should allow us more headroom. + // Passing non-zero max mempool usage (512 KiB) should allow us more headroom. BOOST_CHECK_EQUAL( - chainstate.GetCoinsCacheSizeState(MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes=*/1 << 10), + chainstate.GetCoinsCacheSizeState(MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes=*/ 1 << 19), CoinsCacheSizeState::OK); for (int i{0}; i < 3; ++i) { AddTestCoin(view); print_view_mem_usage(view); BOOST_CHECK_EQUAL( - chainstate.GetCoinsCacheSizeState(MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes=*/1 << 10), + chainstate.GetCoinsCacheSizeState(MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes=*/ 1 << 19), CoinsCacheSizeState::OK); } @@ -119,7 +127,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate) BOOST_CHECK(usage_percentage >= 0.9); BOOST_CHECK(usage_percentage < 1); BOOST_CHECK_EQUAL( - chainstate.GetCoinsCacheSizeState(MAX_COINS_CACHE_BYTES, 1 << 10), + chainstate.GetCoinsCacheSizeState(MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 1 << 10), // 1024 CoinsCacheSizeState::LARGE); } |