mirror of https://github.com/bitcoin/bitcoin.git
coins: Keep track of number of dirty entries in `CCoinsViewCache`
Adds `m_dirty_count` member to track the running count of dirty cache entries as follows: * Incremented when entries are marked dirty via `CCoinsCacheEntry::SetDirty` * Decremented when dirty entries are removed or cleaned * Passed through `CoinsViewCacheCursor` and updated during iteration * Validated in `SanityCheck()` by recomputing from scratch The dirty count is needed because after non-wiping flushes (introduced in #28280 and #28233), the percentage of dirty entries in the cache may be far below 100%. Using total cache size for flush warnings and disk space checks is therefore misleading. Updates all test code to properly initialize and maintain the dirty count. Co-authored-by: l0rinc <pap.lorinc@gmail.com>
This commit is contained in:
parent
3dc15c9607
commit
801e7bb2d2
|
@ -96,10 +96,12 @@ void CCoinsViewCache::AddCoin(const COutPoint &outpoint, Coin&& coin, bool possi
|
|||
fresh = !it->second.IsDirty();
|
||||
}
|
||||
if (!inserted) {
|
||||
m_dirty_count -= it->second.IsDirty();
|
||||
cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage();
|
||||
}
|
||||
it->second.coin = std::move(coin);
|
||||
CCoinsCacheEntry::SetDirty(*it, m_sentinel);
|
||||
++m_dirty_count;
|
||||
if (fresh) CCoinsCacheEntry::SetFresh(*it, m_sentinel);
|
||||
cachedCoinsUsage += it->second.coin.DynamicMemoryUsage();
|
||||
TRACEPOINT(utxocache, add,
|
||||
|
@ -115,6 +117,7 @@ void CCoinsViewCache::EmplaceCoinInternalDANGER(COutPoint&& outpoint, Coin&& coi
|
|||
auto [it, inserted] = cacheCoins.try_emplace(std::move(outpoint), std::move(coin));
|
||||
if (inserted) {
|
||||
CCoinsCacheEntry::SetDirty(*it, m_sentinel);
|
||||
++m_dirty_count;
|
||||
cachedCoinsUsage += mem_usage;
|
||||
}
|
||||
}
|
||||
|
@ -133,6 +136,7 @@ void AddCoins(CCoinsViewCache& cache, const CTransaction &tx, int nHeight, bool
|
|||
bool CCoinsViewCache::SpendCoin(const COutPoint &outpoint, Coin* moveout) {
|
||||
CCoinsMap::iterator it = FetchCoin(outpoint);
|
||||
if (it == cacheCoins.end()) return false;
|
||||
m_dirty_count -= it->second.IsDirty();
|
||||
cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage();
|
||||
TRACEPOINT(utxocache, spent,
|
||||
outpoint.hash.data(),
|
||||
|
@ -147,6 +151,7 @@ bool CCoinsViewCache::SpendCoin(const COutPoint &outpoint, Coin* moveout) {
|
|||
cacheCoins.erase(it);
|
||||
} else {
|
||||
CCoinsCacheEntry::SetDirty(*it, m_sentinel);
|
||||
++m_dirty_count;
|
||||
it->second.coin.Clear();
|
||||
}
|
||||
return true;
|
||||
|
@ -205,8 +210,9 @@ bool CCoinsViewCache::BatchWrite(CoinsViewCacheCursor& cursor, const uint256 &ha
|
|||
} else {
|
||||
entry.coin = it->second.coin;
|
||||
}
|
||||
cachedCoinsUsage += entry.coin.DynamicMemoryUsage();
|
||||
CCoinsCacheEntry::SetDirty(*itUs, m_sentinel);
|
||||
++m_dirty_count;
|
||||
cachedCoinsUsage += entry.coin.DynamicMemoryUsage();
|
||||
// We can mark it FRESH in the parent if it was FRESH in the child
|
||||
// Otherwise it might have just been flushed from the parent's cache
|
||||
// and already exist in the grandparent
|
||||
|
@ -225,6 +231,7 @@ bool CCoinsViewCache::BatchWrite(CoinsViewCacheCursor& cursor, const uint256 &ha
|
|||
if (itUs->second.IsFresh() && it->second.coin.IsSpent()) {
|
||||
// The grandparent cache does not have an entry, and the coin
|
||||
// has been spent. We can just delete it from the parent cache.
|
||||
m_dirty_count -= itUs->second.IsDirty();
|
||||
cachedCoinsUsage -= itUs->second.coin.DynamicMemoryUsage();
|
||||
cacheCoins.erase(itUs);
|
||||
} else {
|
||||
|
@ -238,7 +245,10 @@ bool CCoinsViewCache::BatchWrite(CoinsViewCacheCursor& cursor, const uint256 &ha
|
|||
itUs->second.coin = it->second.coin;
|
||||
}
|
||||
cachedCoinsUsage += itUs->second.coin.DynamicMemoryUsage();
|
||||
CCoinsCacheEntry::SetDirty(*itUs, m_sentinel);
|
||||
if (!itUs->second.IsDirty()) {
|
||||
CCoinsCacheEntry::SetDirty(*itUs, m_sentinel);
|
||||
++m_dirty_count;
|
||||
}
|
||||
// NOTE: It isn't safe to mark the coin as FRESH in the parent
|
||||
// cache. If it already existed and was spent in the parent
|
||||
// cache then marking it FRESH would prevent that spentness
|
||||
|
@ -251,21 +261,23 @@ bool CCoinsViewCache::BatchWrite(CoinsViewCacheCursor& cursor, const uint256 &ha
|
|||
}
|
||||
|
||||
bool CCoinsViewCache::Flush() {
|
||||
auto cursor{CoinsViewCacheCursor(cachedCoinsUsage, m_sentinel, cacheCoins, /*will_erase=*/true)};
|
||||
auto cursor{CoinsViewCacheCursor(cachedCoinsUsage, m_dirty_count, m_sentinel, cacheCoins, /*will_erase=*/true)};
|
||||
bool fOk = base->BatchWrite(cursor, hashBlock);
|
||||
if (fOk) {
|
||||
cacheCoins.clear();
|
||||
ReallocateCache();
|
||||
}
|
||||
cachedCoinsUsage = 0;
|
||||
m_dirty_count = 0;
|
||||
return fOk;
|
||||
}
|
||||
|
||||
bool CCoinsViewCache::Sync()
|
||||
{
|
||||
auto cursor{CoinsViewCacheCursor(cachedCoinsUsage, m_sentinel, cacheCoins, /*will_erase=*/false)};
|
||||
auto cursor{CoinsViewCacheCursor(cachedCoinsUsage, m_dirty_count, m_sentinel, cacheCoins, /*will_erase=*/false)};
|
||||
bool fOk = base->BatchWrite(cursor, hashBlock);
|
||||
if (fOk) {
|
||||
Assume(m_dirty_count == 0);
|
||||
if (m_sentinel.second.Next() != &m_sentinel) {
|
||||
/* BatchWrite must clear flags of all entries */
|
||||
throw std::logic_error("Not all unspent flagged entries were cleared");
|
||||
|
@ -309,6 +321,7 @@ void CCoinsViewCache::ReallocateCache()
|
|||
{
|
||||
// Cache should be empty when we're calling this.
|
||||
assert(cacheCoins.size() == 0);
|
||||
Assume(m_dirty_count == 0);
|
||||
cacheCoins.~CCoinsMap();
|
||||
m_cache_coins_memory_resource.~CCoinsMapMemoryResource();
|
||||
::new (&m_cache_coins_memory_resource) CCoinsMapMemoryResource{};
|
||||
|
@ -319,6 +332,7 @@ void CCoinsViewCache::SanityCheck() const
|
|||
{
|
||||
size_t recomputed_usage = 0;
|
||||
size_t count_flagged = 0;
|
||||
size_t dirty_count = 0;
|
||||
for (const auto& [_, entry] : cacheCoins) {
|
||||
unsigned attr = 0;
|
||||
if (entry.IsDirty()) attr |= 1;
|
||||
|
@ -330,6 +344,9 @@ void CCoinsViewCache::SanityCheck() const
|
|||
// Recompute cachedCoinsUsage.
|
||||
recomputed_usage += entry.coin.DynamicMemoryUsage();
|
||||
|
||||
// Recompute dirty_count.
|
||||
dirty_count += entry.IsDirty();
|
||||
|
||||
// Count the number of entries we expect in the linked list.
|
||||
if (entry.IsDirty() || entry.IsFresh()) ++count_flagged;
|
||||
}
|
||||
|
@ -346,6 +363,7 @@ void CCoinsViewCache::SanityCheck() const
|
|||
}
|
||||
assert(count_linked == count_flagged);
|
||||
assert(recomputed_usage == cachedCoinsUsage);
|
||||
assert(dirty_count == m_dirty_count);
|
||||
}
|
||||
|
||||
static const size_t MIN_TRANSACTION_OUTPUT_WEIGHT = WITNESS_SCALE_FACTOR * ::GetSerializeSize(CTxOut());
|
||||
|
|
16
src/coins.h
16
src/coins.h
|
@ -272,10 +272,11 @@ struct CoinsViewCacheCursor
|
|||
//! 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.
|
||||
CoinsViewCacheCursor(size_t& usage LIFETIMEBOUND,
|
||||
CoinsCachePair& sentinel LIFETIMEBOUND,
|
||||
CCoinsMap& map LIFETIMEBOUND,
|
||||
bool will_erase) noexcept
|
||||
: m_usage(usage), m_sentinel(sentinel), m_map(map), m_will_erase(will_erase) {}
|
||||
size_t& dirty_count LIFETIMEBOUND,
|
||||
CoinsCachePair& sentinel LIFETIMEBOUND,
|
||||
CCoinsMap& map LIFETIMEBOUND,
|
||||
bool will_erase) noexcept
|
||||
: m_usage(usage), m_dirty_count(dirty_count), m_sentinel(sentinel), m_map(map), m_will_erase(will_erase) {}
|
||||
|
||||
inline CoinsCachePair* Begin() const noexcept { return m_sentinel.second.Next(); }
|
||||
inline CoinsCachePair* End() const noexcept { return &m_sentinel; }
|
||||
|
@ -284,6 +285,7 @@ struct CoinsViewCacheCursor
|
|||
inline CoinsCachePair* NextAndMaybeErase(CoinsCachePair& current) noexcept
|
||||
{
|
||||
const auto next_entry{current.second.Next()};
|
||||
m_dirty_count -= current.second.IsDirty();
|
||||
// If we are not going to erase the cache, we must still erase spent entries.
|
||||
// Otherwise, clear the state of the entry.
|
||||
if (!m_will_erase) {
|
||||
|
@ -300,6 +302,7 @@ struct CoinsViewCacheCursor
|
|||
inline bool WillErase(CoinsCachePair& current) const noexcept { return m_will_erase || current.second.coin.IsSpent(); }
|
||||
private:
|
||||
size_t& m_usage;
|
||||
size_t& m_dirty_count;
|
||||
CoinsCachePair& m_sentinel;
|
||||
CCoinsMap& m_map;
|
||||
bool m_will_erase;
|
||||
|
@ -377,6 +380,8 @@ protected:
|
|||
|
||||
/* Cached dynamic memory usage for the inner Coin objects. */
|
||||
mutable size_t cachedCoinsUsage{0};
|
||||
/* Running count of dirty Coin cache entries. */
|
||||
mutable size_t m_dirty_count{0};
|
||||
|
||||
public:
|
||||
CCoinsViewCache(CCoinsView *baseIn, bool deterministic = false);
|
||||
|
@ -463,6 +468,9 @@ public:
|
|||
//! Calculate the size of the cache (in number of transaction outputs)
|
||||
unsigned int GetCacheSize() const;
|
||||
|
||||
//! Calculate the number of dirty cache entries (transaction outputs)
|
||||
size_t GetDirtyCount() const noexcept { return m_dirty_count; }
|
||||
|
||||
//! Calculate the size of the cache (in bytes)
|
||||
size_t DynamicMemoryUsage() const;
|
||||
|
||||
|
|
|
@ -100,6 +100,7 @@ public:
|
|||
CCoinsMap& map() const { return cacheCoins; }
|
||||
CoinsCachePair& sentinel() const { return m_sentinel; }
|
||||
size_t& usage() const { return cachedCoinsUsage; }
|
||||
size_t& dirty() const { return m_dirty_count; }
|
||||
};
|
||||
|
||||
} // namespace
|
||||
|
@ -666,7 +667,8 @@ static void WriteCoinsViewEntry(CCoinsView& view, const MaybeCoin& cache_coin)
|
|||
CCoinsMapMemoryResource resource;
|
||||
CCoinsMap map{0, CCoinsMap::hasher{}, CCoinsMap::key_equal{}, &resource};
|
||||
auto usage{cache_coin ? InsertCoinsMapEntry(map, sentinel, *cache_coin) : 0};
|
||||
auto cursor{CoinsViewCacheCursor(usage, sentinel, map, /*will_erase=*/true)};
|
||||
size_t dirty_count{cache_coin && cache_coin->IsDirty() ? 1U : 0U};
|
||||
auto cursor{CoinsViewCacheCursor(usage, dirty_count, sentinel, map, /*will_erase=*/true)};
|
||||
BOOST_CHECK(view.BatchWrite(cursor, {}));
|
||||
}
|
||||
|
||||
|
@ -677,7 +679,10 @@ public:
|
|||
{
|
||||
auto base_cache_coin{base_value == ABSENT ? MISSING : CoinEntry{base_value, CoinEntry::State::DIRTY}};
|
||||
WriteCoinsViewEntry(base, base_cache_coin);
|
||||
if (cache_coin) cache.usage() += InsertCoinsMapEntry(cache.map(), cache.sentinel(), *cache_coin);
|
||||
if (cache_coin) {
|
||||
cache.usage() += InsertCoinsMapEntry(cache.map(), cache.sentinel(), *cache_coin);
|
||||
cache.dirty() += cache_coin->IsDirty();
|
||||
}
|
||||
}
|
||||
|
||||
CCoinsView root;
|
||||
|
|
|
@ -126,6 +126,7 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsView& backend
|
|||
CoinsCachePair sentinel{};
|
||||
sentinel.second.SelfRef(sentinel);
|
||||
size_t usage{0};
|
||||
size_t dirty_count{0};
|
||||
CCoinsMapMemoryResource resource;
|
||||
CCoinsMap coins_map{0, SaltedOutpointHasher{/*deterministic=*/true}, CCoinsMap::key_equal{}, &resource};
|
||||
LIMITED_WHILE(good_data && fuzzed_data_provider.ConsumeBool(), 10'000)
|
||||
|
@ -147,10 +148,11 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsView& backend
|
|||
if (dirty) CCoinsCacheEntry::SetDirty(*it, sentinel);
|
||||
if (fresh) CCoinsCacheEntry::SetFresh(*it, sentinel);
|
||||
usage += it->second.coin.DynamicMemoryUsage();
|
||||
dirty_count += dirty;
|
||||
}
|
||||
bool expected_code_path = false;
|
||||
try {
|
||||
auto cursor{CoinsViewCacheCursor(usage, sentinel, coins_map, /*will_erase=*/true)};
|
||||
auto cursor{CoinsViewCacheCursor(usage, dirty_count, sentinel, coins_map, /*will_erase=*/true)};
|
||||
uint256 best_block{coins_view_cache.GetBestBlock()};
|
||||
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().
|
||||
|
|
Loading…
Reference in New Issue