diff --git a/src/coins.cpp b/src/coins.cpp index 554a3ebe962..1ca5fca3e69 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -185,18 +185,16 @@ void CCoinsViewCache::SetBestBlock(const uint256 &hashBlockIn) { bool CCoinsViewCache::BatchWrite(CoinsViewCacheCursor& cursor, const uint256 &hashBlockIn) { for (auto it{cursor.Begin()}; it != cursor.End(); it = cursor.NextAndMaybeErase(*it)) { - // Ignore non-dirty entries (optimization). - if (!it->second.IsDirty()) { + if (!it->second.IsDirty()) { // TODO a cursor can only contain dirty entries continue; } - CCoinsMap::iterator itUs = cacheCoins.find(it->first); - if (itUs == cacheCoins.end()) { - // The parent cache does not have an entry, while the child cache does. - // We can ignore it if it's both spent and FRESH in the child - if (!(it->second.IsFresh() && it->second.coin.IsSpent())) { - // Create the coin in the parent cache, move the data up - // and mark it as dirty. - itUs = cacheCoins.try_emplace(it->first).first; + auto [itUs, inserted]{cacheCoins.try_emplace(it->first)}; + if (inserted) { + if (it->second.IsFresh() && it->second.coin.IsSpent()) { + cacheCoins.erase(itUs); // TODO fresh coins should have been removed at spend + } else { + // The parent cache does not have an entry, while the child cache does. + // Move the data up and mark it as dirty. CCoinsCacheEntry& entry{itUs->second}; assert(entry.coin.DynamicMemoryUsage() == 0); if (cursor.WillErase(*it)) { @@ -251,12 +249,14 @@ bool CCoinsViewCache::BatchWrite(CoinsViewCacheCursor& cursor, const uint256 &ha return true; } -bool CCoinsViewCache::Flush() { +bool CCoinsViewCache::Flush(bool will_reuse_cache) { auto cursor{CoinsViewCacheCursor(m_sentinel, cacheCoins, /*will_erase=*/true)}; bool fOk = base->BatchWrite(cursor, hashBlock); if (fOk) { cacheCoins.clear(); - ReallocateCache(); + if (will_reuse_cache) { + ReallocateCache(); + } cachedCoinsUsage = 0; } return fOk; diff --git a/src/coins.h b/src/coins.h index 2fcc764a3fd..8d07f7fe9fd 100644 --- a/src/coins.h +++ b/src/coins.h @@ -439,9 +439,11 @@ public: * Push the modifications applied to this cache to its base and wipe local state. * Failure to call this method or Sync() before destruction will cause the changes * to be forgotten. + * If will_reuse_cache is false, the cache will retain the same memory footprint + * after flushing and should be destroyed to deallocate. * If false is returned, the state of this cache (and its backing view) will be undefined. */ - bool Flush(); + bool Flush(bool will_reuse_cache = true); /** * Push the modifications applied to this cache to its base while retaining diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp index dceec3d2a2a..9990e9cb4bc 100644 --- a/src/test/fuzz/coins_view.cpp +++ b/src/test/fuzz/coins_view.cpp @@ -74,7 +74,7 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsView& backend } }, [&] { - (void)coins_view_cache.Flush(); + (void)coins_view_cache.Flush(/*will_reuse_cache=*/fuzzed_data_provider.ConsumeBool()); }, [&] { (void)coins_view_cache.Sync(); diff --git a/src/test/fuzz/coinscache_sim.cpp b/src/test/fuzz/coinscache_sim.cpp index 30845c2ef9c..82e5f1fc437 100644 --- a/src/test/fuzz/coinscache_sim.cpp +++ b/src/test/fuzz/coinscache_sim.cpp @@ -392,7 +392,7 @@ FUZZ_TARGET(coinscache_sim) // Apply to simulation data. flush(); // Apply to real caches. - caches.back()->Flush(); + caches.back()->Flush(/*will_reuse_cache=*/provider.ConsumeBool()); }, [&]() { // Sync. @@ -402,14 +402,6 @@ FUZZ_TARGET(coinscache_sim) caches.back()->Sync(); }, - [&]() { // Flush + ReallocateCache. - // Apply to simulation data. - flush(); - // Apply to real caches. - caches.back()->Flush(); - caches.back()->ReallocateCache(); - }, - [&]() { // GetCacheSize (void)caches.back()->GetCacheSize(); }, diff --git a/src/validation.cpp b/src/validation.cpp index 50732965581..c74ff8c9aea 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2966,7 +2966,7 @@ bool Chainstate::DisconnectTip(BlockValidationState& state, DisconnectedBlockTra LogError("DisconnectTip(): DisconnectBlock %s failed\n", pindexDelete->GetBlockHash().ToString()); return false; } - bool flushed = view.Flush(); + bool flushed = view.Flush(/*will_reuse_cache=*/false); // local CCoinsViewCache goes out of scope assert(flushed); } LogDebug(BCLog::BENCH, "- Disconnect block: %.2fms\n", @@ -3101,7 +3101,7 @@ bool Chainstate::ConnectTip( Ticks(time_3 - time_2), Ticks(m_chainman.time_connect_total), Ticks(m_chainman.time_connect_total) / m_chainman.num_blocks_total); - bool flushed = view.Flush(); + bool flushed = view.Flush(/*will_reuse_cache=*/false); // local CCoinsViewCache goes out of scope assert(flushed); } const auto time_4{SteadyClock::now()}; @@ -4895,7 +4895,7 @@ bool Chainstate::ReplayBlocks() } cache.SetBestBlock(pindexNew->GetBlockHash()); - cache.Flush(); + cache.Flush(/*will_reuse_cache=*/false); // local CCoinsViewCache goes out of scope m_chainman.GetNotifications().progress(bilingual_str{}, 100, false); return true; }