laantungir
/
bitcoin
mirror of https://github.com/bitcoin/bitcoin.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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 2025-04-20 16:47:51 +02:00
parent 8d5e323b5f
commit 5c2f073e1e
4 changed files with 10 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/coins.cpp
View File

@ -249,7 +249,7 @@ bool CCoinsViewCache::BatchWrite(CoinsViewCacheCursor& cursor, const uint256 &ha
}
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);
if (fOk) {
cacheCoins.clear();
@ -261,7 +261,7 @@ bool CCoinsViewCache::Flush() {
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);
if (fOk) {
if (m_sentinel.second.Next() != &m_sentinel) {

12
src/coins.h
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.
//! 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) {}
CoinsViewCacheCursor(CoinsCachePair& sentinel LIFETIMEBOUND,
CCoinsMap& map LIFETIMEBOUND,
bool will_erase) noexcept
: 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; }
@ -288,7 +287,7 @@ struct CoinsViewCacheCursor
// Otherwise, clear the state of the entry.
if (!m_will_erase) {
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);
} else {
current.second.SetClean();
@ -299,7 +298,6 @@ struct CoinsViewCacheCursor
inline bool WillErase(CoinsCachePair& current) const noexcept { return m_will_erase || current.second.coin.IsSpent(); }
private:
size_t& m_usage;
CoinsCachePair& m_sentinel;
CCoinsMap& m_map;
bool m_will_erase;

4
src/test/coins_tests.cpp
View File

@ -662,8 +662,8 @@ static void WriteCoinsViewEntry(CCoinsView& view, const MaybeCoin& cache_coin)
sentinel.second.SelfRef(sentinel);
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)};
if (cache_coin) InsertCoinsMapEntry(map, sentinel, *cache_coin);
auto cursor{CoinsViewCacheCursor(sentinel, map, /*will_erase=*/true)};
BOOST_CHECK(view.BatchWrite(cursor, {}));
}

4
src/test/fuzz/coins_view.cpp
View File

@ -131,7 +131,6 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsView& backend
[&] {
CoinsCachePair sentinel{};
sentinel.second.SelfRef(sentinel);
size_t usage{0};
CCoinsMapMemoryResource resource;
CCoinsMap coins_map{0, SaltedOutpointHasher{/*deterministic=*/true}, CCoinsMap::key_equal{}, &resource};
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};
if (dirty) CCoinsCacheEntry::SetDirty(*it, sentinel);
if (fresh) CCoinsCacheEntry::SetFresh(*it, sentinel);
usage += it->second.coin.DynamicMemoryUsage();
}
bool expected_code_path = false;
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()};
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().