refactor: remove redundant usage tracking from CoinsViewCacheCursor

When a coin is spent via `SpendCoin()`, `cachedCoinsUsage` is already decremented and the coin's `scriptPubKey` is cleared, so `DynamicMemoryUsage()` is `0`.

`CoinsViewCacheCursor::NextAndMaybeErase()` was subtracting usage again when erasing spent entries.
Replace it with an assert that documents spent coins have zero dynamic memory usage by the time the cursor encounters them.

Remove the now-unnecessary `usage` reference from the cursor's constructor and member variables.
This commit is contained in:
Lőrinc committed 2025-10-11 21:48:04 -04:00
1 parent 67cff8bec9
commit 39cf8bb3d0
4 files changed
+8 -12

No files matched your search

+2 -2
View File
@@ -249,7 +249,7 @@ bool CCoinsViewCache::BatchWrite(CoinsViewCacheCursor& cursor, const uint256 &ha
} }
bool CCoinsViewCache::Flush() { bool CCoinsViewCache::Flush() {
auto cursor{CoinsViewCacheCursor(cachedCoinsUsage, m_sentinel, cacheCoins, /*will_erase=*/true)}; auto cursor{CoinsViewCacheCursor(m_sentinel, cacheCoins, /*will_erase=*/true)};
bool fOk = base->BatchWrite(cursor, hashBlock); bool fOk = base->BatchWrite(cursor, hashBlock);
if (fOk) { if (fOk) {
cacheCoins.clear(); cacheCoins.clear();
@@ -261,7 +261,7 @@ bool CCoinsViewCache::Flush() {
bool CCoinsViewCache::Sync() bool CCoinsViewCache::Sync()
{ {
auto cursor{CoinsViewCacheCursor(cachedCoinsUsage, m_sentinel, cacheCoins, /*will_erase=*/false)}; auto cursor{CoinsViewCacheCursor(m_sentinel, cacheCoins, /*will_erase=*/false)};
bool fOk = base->BatchWrite(cursor, hashBlock); bool fOk = base->BatchWrite(cursor, hashBlock);
if (fOk) { if (fOk) {
if (m_sentinel.second.Next() != &m_sentinel) { if (m_sentinel.second.Next() != &m_sentinel) {
+3 -5
View File
@@ -271,11 +271,10 @@ struct CoinsViewCacheCursor
//! This is an optimization compared to erasing all entries as the cursor iterates them when will_erase is set. //! This is an optimization compared to erasing all entries as the cursor iterates them when will_erase is set.
//! Calling CCoinsMap::clear() afterwards is faster because a CoinsCachePair cannot be coerced back into a //! Calling CCoinsMap::clear() afterwards is faster because a CoinsCachePair cannot be coerced back into a
//! CCoinsMap::iterator to be erased, and must therefore be looked up again by key in the CCoinsMap before being erased. //! CCoinsMap::iterator to be erased, and must therefore be looked up again by key in the CCoinsMap before being erased.
CoinsViewCacheCursor(size_t& usage LIFETIMEBOUND, CoinsViewCacheCursor(CoinsCachePair& sentinel LIFETIMEBOUND,
CoinsCachePair& sentinel LIFETIMEBOUND,
CCoinsMap& map LIFETIMEBOUND, CCoinsMap& map LIFETIMEBOUND,
bool will_erase) noexcept bool will_erase) noexcept
: m_usage(usage), m_sentinel(sentinel), m_map(map), m_will_erase(will_erase) {} : m_sentinel(sentinel), m_map(map), m_will_erase(will_erase) {}
inline CoinsCachePair* Begin() const noexcept { return m_sentinel.second.Next(); } inline CoinsCachePair* Begin() const noexcept { return m_sentinel.second.Next(); }
inline CoinsCachePair* End() const noexcept { return &m_sentinel; } inline CoinsCachePair* End() const noexcept { return &m_sentinel; }
@@ -288,7 +287,7 @@ struct CoinsViewCacheCursor
// Otherwise, clear the state of the entry. // Otherwise, clear the state of the entry.
if (!m_will_erase) { if (!m_will_erase) {
if (current.second.coin.IsSpent()) { if (current.second.coin.IsSpent()) {
m_usage -= current.second.coin.DynamicMemoryUsage(); assert(current.second.coin.DynamicMemoryUsage() == 0); // scriptPubKey was already cleared in SpendCoin
m_map.erase(current.first); m_map.erase(current.first);
} else { } else {
current.second.SetClean(); current.second.SetClean();
@@ -299,7 +298,6 @@ struct CoinsViewCacheCursor
inline bool WillErase(CoinsCachePair& current) const noexcept { return m_will_erase || current.second.coin.IsSpent(); } inline bool WillErase(CoinsCachePair& current) const noexcept { return m_will_erase || current.second.coin.IsSpent(); }
private: private:
size_t& m_usage;
CoinsCachePair& m_sentinel; CoinsCachePair& m_sentinel;
CCoinsMap& m_map; CCoinsMap& m_map;
bool m_will_erase; bool m_will_erase;
+2 -2
View File
@@ -662,8 +662,8 @@ static void WriteCoinsViewEntry(CCoinsView& view, const MaybeCoin& cache_coin)
sentinel.second.SelfRef(sentinel); sentinel.second.SelfRef(sentinel);
CCoinsMapMemoryResource resource; CCoinsMapMemoryResource resource;
CCoinsMap map{0, CCoinsMap::hasher{}, CCoinsMap::key_equal{}, &resource}; CCoinsMap map{0, CCoinsMap::hasher{}, CCoinsMap::key_equal{}, &resource};
auto usage{cache_coin ? InsertCoinsMapEntry(map, sentinel, *cache_coin) : 0}; if (cache_coin) InsertCoinsMapEntry(map, sentinel, *cache_coin);
auto cursor{CoinsViewCacheCursor(usage, sentinel, map, /*will_erase=*/true)}; auto cursor{CoinsViewCacheCursor(sentinel, map, /*will_erase=*/true)};
BOOST_CHECK(view.BatchWrite(cursor, {})); BOOST_CHECK(view.BatchWrite(cursor, {}));
} }
+1 -3
View File
@@ -131,7 +131,6 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsView& backend
[&] { [&] {
CoinsCachePair sentinel{}; CoinsCachePair sentinel{};
sentinel.second.SelfRef(sentinel); sentinel.second.SelfRef(sentinel);
size_t usage{0};
CCoinsMapMemoryResource resource; CCoinsMapMemoryResource resource;
CCoinsMap coins_map{0, SaltedOutpointHasher{/*deterministic=*/true}, CCoinsMap::key_equal{}, &resource}; CCoinsMap coins_map{0, SaltedOutpointHasher{/*deterministic=*/true}, CCoinsMap::key_equal{}, &resource};
LIMITED_WHILE(good_data && fuzzed_data_provider.ConsumeBool(), 10'000) LIMITED_WHILE(good_data && fuzzed_data_provider.ConsumeBool(), 10'000)
@@ -152,11 +151,10 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsView& backend
auto it{coins_map.emplace(random_out_point, std::move(coins_cache_entry)).first}; auto it{coins_map.emplace(random_out_point, std::move(coins_cache_entry)).first};
if (dirty) CCoinsCacheEntry::SetDirty(*it, sentinel); if (dirty) CCoinsCacheEntry::SetDirty(*it, sentinel);
if (fresh) CCoinsCacheEntry::SetFresh(*it, sentinel); if (fresh) CCoinsCacheEntry::SetFresh(*it, sentinel);
usage += it->second.coin.DynamicMemoryUsage();
} }
bool expected_code_path = false; bool expected_code_path = false;
try { try {
auto cursor{CoinsViewCacheCursor(usage, sentinel, coins_map, /*will_erase=*/true)}; auto cursor{CoinsViewCacheCursor(sentinel, coins_map, /*will_erase=*/true)};
uint256 best_block{coins_view_cache.GetBestBlock()}; uint256 best_block{coins_view_cache.GetBestBlock()};
if (fuzzed_data_provider.ConsumeBool()) best_block = ConsumeUInt256(fuzzed_data_provider); if (fuzzed_data_provider.ConsumeBool()) best_block = ConsumeUInt256(fuzzed_data_provider);
// Set best block hash to non-null to satisfy the assertion in CCoinsViewDB::BatchWrite(). // Set best block hash to non-null to satisfy the assertion in CCoinsViewDB::BatchWrite().