merge bitcoin#16945: introduce CChainState::GetCoinsCacheSizeState

src/Makefile.test.include

@@ -193,6 +193,7 @@ BITCOIN_TESTS =\
   test/uint256_tests.cpp \
   test/util_tests.cpp \
   test/validation_block_tests.cpp \
+  test/validation_flush_tests.cpp \
   test/versionbits_tests.cpp
 
 if ENABLE_WALLET

src/test/validation_flush_tests.cpp

@@ -0,0 +1,174 @@
+// Copyright (c) 2019 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 <txmempool.h>
+#include <validation.h>
+#include <sync.h>
+#include <test/test_dash.h>
+
+#include <boost/test/unit_test.hpp>
+
+BOOST_FIXTURE_TEST_SUITE(validation_flush_tests, BasicTestingSetup)
+
+//! Test utilities for detecting when we need to flush the coins cache based
+//! on estimated memory usage.
+//!
+//! @sa CChainState::GetCoinsCacheSizeState()
+//!
+BOOST_AUTO_TEST_CASE(getcoinscachesizestate)
+{
+    BlockManager blockman{};
+    CChainState chainstate{blockman};
+    chainstate.InitCoinsDB(/*cache_size_bytes*/ 1 << 10, /*in_memory*/ true, /*should_wipe*/ false);
+    WITH_LOCK(::cs_main, chainstate.InitCoinsCache());
+    CTxMemPool tx_pool{};
+
+    constexpr bool is_64_bit = sizeof(void*) == 8;
+
+    LOCK(::cs_main);
+    auto& view = chainstate.CoinsTip();
+
+    //! Create and add a Coin with DynamicMemoryUsage of 80 bytes to the given view.
+    auto add_coin = [](CCoinsViewCache& coins_view) -> COutPoint {
+        Coin newcoin;
+        uint256 txid = InsecureRand256();
+        COutPoint outp{txid, 0};
+        newcoin.nHeight = 1;
+        newcoin.out.nValue = InsecureRand32();
+        newcoin.out.scriptPubKey.assign((uint32_t)56, 1);
+        coins_view.AddCoin(outp, std::move(newcoin), false);
+
+        return outp;
+    };
+
+    // The number of bytes consumed by coin's heap data, i.e. CScript
+    // (prevector<28, unsigned char>) when assigned 56 bytes of data per above.
+    //
+    // See also: Coin::DynamicMemoryUsage().
+    constexpr int COIN_SIZE = is_64_bit ? 80 : 64;
+
+    auto print_view_mem_usage = [](CCoinsViewCache& view) {
+        BOOST_TEST_MESSAGE("CCoinsViewCache memory usage: " << view.DynamicMemoryUsage());
+    };
+
+    constexpr size_t MAX_COINS_CACHE_BYTES = 1024;
+
+    // Without any coins in the cache, we shouldn't need to flush.
+    BOOST_CHECK_EQUAL(
+        chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0),
+        CoinsCacheSizeState::OK);
+
+    // 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) {
+            COutPoint res = add_coin(view);
+            BOOST_CHECK_EQUAL(view.AccessCoin(res).DynamicMemoryUsage(), COIN_SIZE);
+        }
+
+        BOOST_CHECK_EQUAL(
+            chainstate.GetCoinsCacheSizeState(tx_pool, 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 ? 32 : 16);
+
+    // 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.
+    //
+    // I came up with the count by examining the printed memory usage of the
+    // CCoinsCacheView, so it's sort of arbitrary - but it shouldn't change
+    // unless we somehow change the way the cacheCoins map allocates memory.
+    //
+    constexpr int COINS_UNTIL_CRITICAL = is_64_bit ? 4 : 5;
+
+    for (int i{0}; i < COINS_UNTIL_CRITICAL; ++i) {
+        COutPoint res = add_coin(view);
+        print_view_mem_usage(view);
+        BOOST_CHECK_EQUAL(view.AccessCoin(res).DynamicMemoryUsage(), COIN_SIZE);
+        BOOST_CHECK_EQUAL(
+            chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0),
+            CoinsCacheSizeState::OK);
+    }
+
+    // Adding an additional coin will push us over the edge to CRITICAL.
+    add_coin(view);
+    print_view_mem_usage(view);
+
+    auto size_state = chainstate.GetCoinsCacheSizeState(
+        tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0);
+
+    if (!is_64_bit && size_state == CoinsCacheSizeState::LARGE) {
+        // On 32 bit hosts, we may hit LARGE before CRITICAL.
+        add_coin(view);
+        print_view_mem_usage(view);
+    }
+
+    BOOST_CHECK_EQUAL(
+        chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0),
+        CoinsCacheSizeState::CRITICAL);
+
+    // Passing non-zero max mempool usage should allow us more headroom.
+    BOOST_CHECK_EQUAL(
+        chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 1 << 10),
+        CoinsCacheSizeState::OK);
+
+    for (int i{0}; i < 3; ++i) {
+        add_coin(view);
+        print_view_mem_usage(view);
+        BOOST_CHECK_EQUAL(
+            chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 1 << 10),
+            CoinsCacheSizeState::OK);
+    }
+
+    // Adding another coin with the additional mempool room will put us >90%
+    // but not yet critical.
+    add_coin(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(tx_pool, MAX_COINS_CACHE_BYTES, 1 << 10),
+            CoinsCacheSizeState::LARGE);
+    }
+
+    // Using the default max_* values permits way more coins to be added.
+    for (int i{0}; i < 1000; ++i) {
+        add_coin(view);
+        BOOST_CHECK_EQUAL(
+            chainstate.GetCoinsCacheSizeState(tx_pool),
+            CoinsCacheSizeState::OK);
+    }
+
+    // Flushing the view doesn't take us back to OK because cacheCoins has
+    // preallocated memory that doesn't get reclaimed even after flush.
+
+    BOOST_CHECK_EQUAL(
+        chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, 0),
+        CoinsCacheSizeState::CRITICAL);
+
+    view.SetBestBlock(InsecureRand256());
+    BOOST_CHECK(view.Flush());
+    print_view_mem_usage(view);
+
+    BOOST_CHECK_EQUAL(
+        chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, 0),
+        CoinsCacheSizeState::CRITICAL);
+}
+
+BOOST_AUTO_TEST_SUITE_END()

src/txdb.h

@@ -24,8 +24,6 @@ class CBlockIndex;
 class CCoinsViewDBCursor;
 class uint256;
 
-//! No need to periodic flush if at least this much space still available.
-static constexpr int MAX_BLOCK_COINSDB_USAGE = 10;
 //! -dbcache default (MiB)
 static const int64_t nDefaultDbCache = 300;
 //! -dbbatchsize default (bytes)

src/validation.cpp

@@ -2370,13 +2370,46 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl
     return true;
 }
 
+CoinsCacheSizeState CChainState::GetCoinsCacheSizeState(const CTxMemPool& tx_pool)
+{
+    return this->GetCoinsCacheSizeState(
+        tx_pool,
+        nCoinCacheUsage,
+        gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000);
+}
+
+CoinsCacheSizeState CChainState::GetCoinsCacheSizeState(
+    const CTxMemPool& tx_pool,
+    size_t max_coins_cache_size_bytes,
+    size_t max_mempool_size_bytes)
+{
+    int64_t nMempoolUsage = tx_pool.DynamicMemoryUsage();
+    int64_t cacheSize = CoinsTip().DynamicMemoryUsage();
+    int64_t nTotalSpace =
+        max_coins_cache_size_bytes + std::max<int64_t>(max_mempool_size_bytes - nMempoolUsage, 0);
+
+    cacheSize += evoDb->GetMemoryUsage();
+
+    //! 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) {
+        return CoinsCacheSizeState::LARGE;
+    }
+    return CoinsCacheSizeState::OK;
+}
+
 bool CChainState::FlushStateToDisk(
     const CChainParams& chainparams,
     CValidationState &state,
     FlushStateMode mode,
     int nManualPruneHeight)
 {
-    int64_t nMempoolUsage = mempool.DynamicMemoryUsage();
     LOCK(cs_main);
     assert(this->CanFlushToDisk());
     static int64_t nLastWrite = 0;
@@ -2391,6 +2424,7 @@ bool CChainState::FlushStateToDisk(
     {
         bool fFlushForPrune = false;
         bool fDoFullFlush = false;
+        CoinsCacheSizeState cache_state = GetCoinsCacheSizeState(::mempool);
         LOCK(cs_LastBlockFile);
         if (fPruneMode && (fCheckForPruning || nManualPruneHeight > 0) && !fReindex) {
             if (nManualPruneHeight > 0) {
@@ -2419,14 +2453,10 @@ bool CChainState::FlushStateToDisk(
         if (nLastFlush == 0) {
             nLastFlush = nNow;
         }
-        int64_t nMempoolSizeMax = gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000;
-        int64_t cacheSize = CoinsTip().DynamicMemoryUsage();
-        cacheSize += evoDb->GetMemoryUsage();
-        int64_t nTotalSpace = nCoinCacheUsage + std::max<int64_t>(nMempoolSizeMax - nMempoolUsage, 0);
         // The cache is large and we're within 10% and 10 MiB of the limit, but we have time now (not in the middle of a block processing).
-        bool fCacheLarge = mode == FlushStateMode::PERIODIC && cacheSize > std::max((9 * nTotalSpace) / 10, nTotalSpace - MAX_BLOCK_COINSDB_USAGE * 1024 * 1024);
+        bool fCacheLarge = mode == FlushStateMode::PERIODIC && cache_state >= CoinsCacheSizeState::LARGE;
         // The cache is over the limit, we have to write now.
-        bool fCacheCritical = mode == FlushStateMode::IF_NEEDED && cacheSize > nCoinCacheUsage;
+        bool fCacheCritical = mode == FlushStateMode::IF_NEEDED && cache_state >= CoinsCacheSizeState::CRITICAL;
         // It's been a while since we wrote the block index to disk. Do this frequently, so we don't need to redownload after a crash.
         bool fPeriodicWrite = mode == FlushStateMode::PERIODIC && nNow > nLastWrite + (int64_t)DATABASE_WRITE_INTERVAL * 1000000;
         // It's been very long since we flushed the cache. Do this infrequently, to optimize cache usage.

src/validation.h

@@ -513,6 +513,15 @@ class CoinsViews {
     void InitCache() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 };
 
+enum class CoinsCacheSizeState
+{
+    //! The coins cache is in immediate need of a flush.
+    CRITICAL = 2,
+    //! The cache is at >= 90% capacity.
+    LARGE = 1,
+    OK = 0
+};
+
 /**
  * CChainState stores and provides an API to update our local knowledge of the
  * current best chain.
@@ -705,6 +714,17 @@ class CChainState
     /** Update the chain tip based on database information, i.e. CoinsTip()'s best block. */
     bool LoadChainTip(const CChainParams& chainparams) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 
+    //! Dictates whether we need to flush the cache to disk or not.
+    //!
+    //! @return the state of the size of the coins cache.
+    CoinsCacheSizeState GetCoinsCacheSizeState(const CTxMemPool& tx_pool)
+        EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+
+    CoinsCacheSizeState GetCoinsCacheSizeState(
+        const CTxMemPool& tx_pool,
+        size_t max_coins_cache_size_bytes,
+        size_t max_mempool_size_bytes) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+
 private:
     bool ActivateBestChainStep(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
     bool ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions& disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main);