validation: don't reallocate cache for short-lived CCoinsViewCache
What the developer wrote
Authored by Lőrinc
validation: don't reallocate cache for short-lived CCoinsViewCache
A few temporary `CCoinsViewCache`'s are destructed right after the `Flush()`, therefore it is not necessary to call `ReallocateCache` to recreate them right before they're killed anyway.
* `Flush()` - retains existing functionality;
* `Flush(/*will_reuse_cache=*/false)` - skips destruction and reallocation of the parent cache since it will soon go out of scope anyway;
For the `will_reuse_cache` parameter we want to see exactly which ones will reallocate memory and which won't - since both can be valid usages.
This change was based on a subset of https://github.com/bitcoin/bitcoin/pull/28945.
Co-authored-by: Martin Ankerl <martin.ankerl@gmail.com>
What changed, and why it matters
This commit is a small performance optimization for Bitcoin Core. It changes how temporary in-memory data caches are cleaned up after use. Previously, these short-lived caches were always emptied and then immediately re-created, only to be destroyed moments later. The patch adds an option to skip that pointless re-creation step. It is not a security fix and does not change what data is stored or how it is validated.
No security action required. Treat as a routine performance/refactoring commit. Reviewers may verify that all short-lived CCoinsViewCache users are correctly passing will_reuse_cache=false and that long-lived caches still default to true.
Security signals we found
No security-relevant behavioral change: Flush still writes all pending modifications to the base view and clears local entries.
No new untrusted inputs or parsing surfaces introduced.
Default parameter value preserves prior behavior for all existing callers except the three explicitly updated sites.
No bounds checks, cryptographic operations, or network handling modified.
Evidence from the diff
CCoinsViewCache::Flush() is modified to accept a bool will_reuse_cache parameter defaulting to true. When false, Flush() skips the ReallocateCache() call that rebuilds the internal hash table. Several call sites in validation.cpp (DisconnectTip, ConnectTip, ReplayBlocks) now pass false because their local CCoinsViewCache objects go out of scope immediately after flushing. Fuzz tests are updated to cover both values. The change reduces unnecessary memory allocation/deallocation work without altering consensus behavior.
Changed components
src/coins.cppsrc/coins.hsrc/validation.cppsrc/test/fuzz/coins_view.cppsrc/test/fuzz/coinscache_sim.cppInspect captured patch +12 / −16
diff --git a/src/coins.cpp b/src/coins.cpp
index e650b81f..b5dd7c62 100644
--- a/src/coins.cpp
+++ b/src/coins.cpp
@@ -249,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 2fcc764a..8d07f7fe 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 c6870654..f277eadf 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 30845c2e..82e5f1fc 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 af523b06..5906eef8 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -3036,7 +3036,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",
@@ -3171,7 +3171,7 @@ bool Chainstate::ConnectTip(
Ticks<MillisecondsDouble>(time_3 - time_2),
Ticks<SecondsDouble>(m_chainman.time_connect_total),
Ticks<MillisecondsDouble>(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()};
@@ -4950,7 +4950,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;
}
Why this scored 19/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.