From 1b40dc02a6a292239037ac5a44e0d0c9506d5fa2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Sat, 19 Jul 2025 17:19:21 -0700 Subject: [PATCH 1/3] refactor: extract `LargeCoinsCacheThreshold` from `GetCoinsCacheSizeState` Move-only commit, enabled reusing the large cache size calculation logic later. The only difference is the removal of the `static` keyword (since in a constexpr function it's a C++23 extension) --- src/validation.cpp | 7 +------ src/validation.h | 10 ++++++++++ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index b62691ca4c0..c8c496a59ab 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2786,15 +2786,10 @@ CoinsCacheSizeState Chainstate::GetCoinsCacheSizeState( int64_t nTotalSpace = max_coins_cache_size_bytes + std::max(int64_t(max_mempool_size_bytes) - nMempoolUsage, 0); - //! No need to periodic flush if at least this much space still available. - static constexpr int64_t MAX_BLOCK_COINSDB_USAGE_BYTES = 10 * 1024 * 1024; // 10MB - int64_t large_threshold = - std::max((9 * nTotalSpace) / 10, nTotalSpace - MAX_BLOCK_COINSDB_USAGE_BYTES); - if (cacheSize > nTotalSpace) { LogPrintf("Cache size (%s) exceeds total space (%s)\n", cacheSize, nTotalSpace); return CoinsCacheSizeState::CRITICAL; - } else if (cacheSize > large_threshold) { + } else if (cacheSize > LargeCoinsCacheThreshold(nTotalSpace)) { return CoinsCacheSizeState::LARGE; } return CoinsCacheSizeState::OK; diff --git a/src/validation.h b/src/validation.h index c25dd2de2d9..8998dccedcf 100644 --- a/src/validation.h +++ b/src/validation.h @@ -35,6 +35,7 @@ #include #include +#include #include #include #include @@ -503,6 +504,15 @@ enum class CoinsCacheSizeState OK = 0 }; +constexpr int64_t LargeCoinsCacheThreshold(int64_t nTotalSpace) noexcept +{ + //! No need to periodic flush if at least this much space still available. + constexpr int64_t MAX_BLOCK_COINSDB_USAGE_BYTES = 10 * 1024 * 1024; // 10MB + int64_t large_threshold = + std::max((9 * nTotalSpace) / 10, nTotalSpace - MAX_BLOCK_COINSDB_USAGE_BYTES); + return large_threshold; +} + /** * Chainstate stores and provides an API to update our local knowledge of the * current best chain. From 64ed0fa6b7a2b637f236c3abf2f045adf6a067cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Sat, 19 Jul 2025 17:20:33 -0700 Subject: [PATCH 2/3] refactor: modernize `LargeCoinsCacheThreshold` * parameter name uses underscores * commit message typo fixed and compacted * used `10_MiB` to avoid having to comment * swapped order of operands in (9 * x / 10) to make it obvious that we're calculating 90% * inlined return value --- src/validation.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/validation.h b/src/validation.h index 8998dccedcf..6be6ae287fc 100644 --- a/src/validation.h +++ b/src/validation.h @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -504,13 +505,12 @@ enum class CoinsCacheSizeState OK = 0 }; -constexpr int64_t LargeCoinsCacheThreshold(int64_t nTotalSpace) noexcept +constexpr int64_t LargeCoinsCacheThreshold(int64_t total_space) noexcept { - //! No need to periodic flush if at least this much space still available. - constexpr int64_t MAX_BLOCK_COINSDB_USAGE_BYTES = 10 * 1024 * 1024; // 10MB - int64_t large_threshold = - std::max((9 * nTotalSpace) / 10, nTotalSpace - MAX_BLOCK_COINSDB_USAGE_BYTES); - return large_threshold; + // No periodic flush needed if at least this much space is free + constexpr int64_t MAX_BLOCK_COINSDB_USAGE_BYTES{int64_t(10_MiB)}; + return std::max((total_space * 9) / 10, + total_space - MAX_BLOCK_COINSDB_USAGE_BYTES); } /** From 554befd8738ea993b3b555e7366558a9c32c915c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Sat, 19 Jul 2025 17:16:52 -0700 Subject: [PATCH 3/3] test: revive `getcoinscachesizestate` After the changes in https://github.com/bitcoin/bitcoin/pull/25325 `getcoinscachesizestate` always end the test early, see: https://maflcko.github.io/b-c-cov/test_bitcoin.coverage/src/test/validation_flush_tests.cpp.gcov.html The test revival was extracted from a related PR where it was discovered, see: https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2109417797 Co-authored-by: Larry Ruane --- src/test/validation_flush_tests.cpp | 157 ++++++---------------------- 1 file changed, 34 insertions(+), 123 deletions(-) diff --git a/src/test/validation_flush_tests.cpp b/src/test/validation_flush_tests.cpp index 4d6017a0e30..2477cb19d60 100644 --- a/src/test/validation_flush_tests.cpp +++ b/src/test/validation_flush_tests.cpp @@ -1,7 +1,7 @@ -// Copyright (c) 2019-2022 The Bitcoin Core developers +// Copyright (c) 2019-present The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -// + #include #include #include @@ -12,145 +12,56 @@ BOOST_FIXTURE_TEST_SUITE(validation_flush_tests, TestingSetup) -//! Test utilities for detecting when we need to flush the coins cache based -//! on estimated memory usage. -//! -//! @sa Chainstate::GetCoinsCacheSizeState() -//! +//! Verify that Chainstate::GetCoinsCacheSizeState() switches from OK→LARGE→CRITICAL +//! at the expected utilization thresholds, first with *no* mempool head-room, +//! then with additional mempool head-room. BOOST_AUTO_TEST_CASE(getcoinscachesizestate) { Chainstate& chainstate{m_node.chainman->ActiveChainstate()}; - constexpr bool is_64_bit = sizeof(void*) == 8; - LOCK(::cs_main); - auto& view = chainstate.CoinsTip(); + CCoinsViewCache& view{chainstate.CoinsTip()}; - // The number of bytes consumed by coin's heap data, i.e. - // CScript (prevector<36, unsigned char>) when assigned 56 bytes of data per above. - // See also: Coin::DynamicMemoryUsage(). - constexpr unsigned int COIN_SIZE = is_64_bit ? 80 : 64; + // Sanity: an empty cache should be ≲ 1 chunk (~ 256 KiB). + BOOST_CHECK_LT(view.DynamicMemoryUsage() / (256 * 1024.0), 1.1); - auto print_view_mem_usage = [](CCoinsViewCache& view) { - BOOST_TEST_MESSAGE("CCoinsViewCache memory usage: " << view.DynamicMemoryUsage()); - }; + constexpr size_t MAX_COINS_BYTES{8_MiB}; + constexpr size_t MAX_MEMPOOL_BYTES{4_MiB}; + constexpr size_t MAX_ATTEMPTS{50'000}; - // 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; + // Run the same growth-path twice: first with 0 head-room, then with extra head-room + for (size_t max_mempool_size_bytes : {size_t{0}, MAX_MEMPOOL_BYTES}) { + const int64_t full_cap{int64_t(MAX_COINS_BYTES + max_mempool_size_bytes)}; + const int64_t large_cap{LargeCoinsCacheThreshold(full_cap)}; - // Without any coins in the cache, we shouldn't need to flush. - 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. - // End the test early. - if (view.DynamicMemoryUsage() != 32 && view.DynamicMemoryUsage() != 16) { - // Add a bunch of coins to see that we at least flip over to CRITICAL. - - for (int i{0}; i < 1000; ++i) { - const COutPoint res = AddTestCoin(m_rng, view); - BOOST_CHECK_EQUAL(view.AccessCoin(res).DynamicMemoryUsage(), COIN_SIZE); + // OK → LARGE + auto state{chainstate.GetCoinsCacheSizeState(MAX_COINS_BYTES, max_mempool_size_bytes)}; + for (size_t i{0}; i < MAX_ATTEMPTS && int64_t(view.DynamicMemoryUsage()) <= large_cap; ++i) { + BOOST_CHECK_EQUAL(state, CoinsCacheSizeState::OK); + AddTestCoin(m_rng, view); + state = chainstate.GetCoinsCacheSizeState(MAX_COINS_BYTES, max_mempool_size_bytes); } - BOOST_CHECK_EQUAL( - chainstate.GetCoinsCacheSizeState(MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes=*/0), - CoinsCacheSizeState::CRITICAL); - - BOOST_TEST_MESSAGE("Exiting cache flush tests early due to unsupported arch"); - return; - } - - print_view_mem_usage(view); - BOOST_CHECK_EQUAL(view.DynamicMemoryUsage(), is_64_bit ? 32U : 16U); - - // We should be able to add COINS_UNTIL_CRITICAL coins to the cache before going CRITICAL. - // This is contingent not only on the dynamic memory usage of the Coins - // that we're adding (COIN_SIZE bytes per), but also on how much memory the - // 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(m_rng, 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::LARGE); - } - - // Adding some additional coins will push us over the edge to CRITICAL. - for (int i{0}; i < 4; ++i) { - AddTestCoin(m_rng, view); - print_view_mem_usage(view); - if (chainstate.GetCoinsCacheSizeState(MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes=*/0) == - CoinsCacheSizeState::CRITICAL) { - break; + // LARGE → CRITICAL + for (size_t i{0}; i < MAX_ATTEMPTS && int64_t(view.DynamicMemoryUsage()) <= full_cap; ++i) { + BOOST_CHECK_EQUAL(state, CoinsCacheSizeState::LARGE); + AddTestCoin(m_rng, view); + state = chainstate.GetCoinsCacheSizeState(MAX_COINS_BYTES, max_mempool_size_bytes); } + BOOST_CHECK_EQUAL(state, CoinsCacheSizeState::CRITICAL); } - BOOST_CHECK_EQUAL( - chainstate.GetCoinsCacheSizeState(MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes=*/0), - CoinsCacheSizeState::CRITICAL); - - // 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 << 19), - CoinsCacheSizeState::OK); - - for (int i{0}; i < 3; ++i) { + // Default thresholds (no explicit limits) permit many more coins. + for (int i{0}; i < 1'000; ++i) { AddTestCoin(m_rng, view); - print_view_mem_usage(view); - BOOST_CHECK_EQUAL( - chainstate.GetCoinsCacheSizeState(MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes=*/ 1 << 19), - CoinsCacheSizeState::OK); + BOOST_CHECK_EQUAL(chainstate.GetCoinsCacheSizeState(), CoinsCacheSizeState::OK); } - // Adding another coin with the additional mempool room will put us >90% - // but not yet critical. - AddTestCoin(m_rng, view); - print_view_mem_usage(view); - - // Only perform these checks on 64 bit hosts; I haven't done the math for 32. - if (is_64_bit) { - float usage_percentage = (float)view.DynamicMemoryUsage() / (MAX_COINS_CACHE_BYTES + (1 << 10)); - BOOST_TEST_MESSAGE("CoinsTip usage percentage: " << usage_percentage); - BOOST_CHECK(usage_percentage >= 0.9); - BOOST_CHECK(usage_percentage < 1); - BOOST_CHECK_EQUAL( - chainstate.GetCoinsCacheSizeState(MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 1 << 10), // 1024 - CoinsCacheSizeState::LARGE); - } - - // Using the default max_* values permits way more coins to be added. - for (int i{0}; i < 1000; ++i) { - AddTestCoin(m_rng, view); - BOOST_CHECK_EQUAL( - chainstate.GetCoinsCacheSizeState(), - CoinsCacheSizeState::OK); - } - - // Flushing the view does take us back to OK because ReallocateCache() is called - - BOOST_CHECK_EQUAL( - chainstate.GetCoinsCacheSizeState(MAX_COINS_CACHE_BYTES, 0), - CoinsCacheSizeState::CRITICAL); - + // CRITICAL → OK via Flush + BOOST_CHECK_EQUAL(chainstate.GetCoinsCacheSizeState(MAX_COINS_BYTES, /*max_mempool_size_bytes=*/0), CoinsCacheSizeState::CRITICAL); view.SetBestBlock(m_rng.rand256()); - BOOST_CHECK(view.Flush()); - print_view_mem_usage(view); - - BOOST_CHECK_EQUAL( - chainstate.GetCoinsCacheSizeState(MAX_COINS_CACHE_BYTES, 0), - CoinsCacheSizeState::OK); + BOOST_REQUIRE(view.Flush()); + BOOST_CHECK_EQUAL(chainstate.GetCoinsCacheSizeState(MAX_COINS_BYTES, /*max_mempool_size_bytes=*/0), CoinsCacheSizeState::OK); } BOOST_AUTO_TEST_SUITE_END()