Skip to content

[draft] Remove the dirty linked list in CCoinsViewCache #13

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
optout21 wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

[draft] Remove the dirty linked list in CCoinsViewCache #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 16 additions & 24 deletions src/coins.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,7 @@ std::optional<Coin> CCoinsViewCache::PeekCoin(const COutPoint& outpoint) const
CCoinsViewCache::CCoinsViewCache(CCoinsView* in_base, bool deterministic) :
CCoinsViewBacked(in_base), m_deterministic(deterministic),
cacheCoins(0, SaltedOutpointHasher(/*deterministic=*/deterministic), CCoinsMap::key_equal{}, &m_cache_coins_memory_resource)
{
m_sentinel.second.SelfRef(m_sentinel);
}
{}

size_t CCoinsViewCache::DynamicMemoryUsage() const {
return memusage::DynamicUsage(cacheCoins) + cachedCoinsUsage;
Expand Down Expand Up @@ -96,9 +94,9 @@ void CCoinsViewCache::AddCoin(const COutPoint &outpoint, Coin&& coin, bool possi
Assume(TrySub(cachedCoinsUsage, it->second.coin.DynamicMemoryUsage()));
}
it->second.coin = std::move(coin);
CCoinsCacheEntry::SetDirty(*it, m_sentinel);
CCoinsCacheEntry::SetDirty(*it);
++m_dirty_count;
if (fresh) CCoinsCacheEntry::SetFresh(*it, m_sentinel);
if (fresh) CCoinsCacheEntry::SetFresh(*it);
cachedCoinsUsage += it->second.coin.DynamicMemoryUsage();
TRACEPOINT(utxocache, add,
outpoint.hash.data(),
Expand All @@ -112,7 +110,7 @@ void CCoinsViewCache::EmplaceCoinInternalDANGER(COutPoint&& outpoint, Coin&& coi
const auto mem_usage{coin.DynamicMemoryUsage()};
auto [it, inserted] = cacheCoins.try_emplace(std::move(outpoint), std::move(coin));
if (inserted) {
CCoinsCacheEntry::SetDirty(*it, m_sentinel);
CCoinsCacheEntry::SetDirty(*it);
++m_dirty_count;
cachedCoinsUsage += mem_usage;
}
Expand Down Expand Up @@ -146,7 +144,7 @@ bool CCoinsViewCache::SpendCoin(const COutPoint &outpoint, Coin* moveout) {
if (it->second.IsFresh()) {
cacheCoins.erase(it);
} else {
CCoinsCacheEntry::SetDirty(*it, m_sentinel);
CCoinsCacheEntry::SetDirty(*it);
++m_dirty_count;
it->second.coin.Clear();
}
Expand Down Expand Up @@ -188,7 +186,7 @@ void CCoinsViewCache::SetBestBlock(const uint256& in_block_hash)

void CCoinsViewCache::BatchWrite(CoinsViewCacheCursor& cursor, const uint256& in_block_hash)
{
for (auto it{cursor.Begin()}; it != cursor.End(); it = cursor.NextAndMaybeErase(*it)) {
for (auto it{cursor.Begin()}; it != cursor.End(); it.NextAndMaybeErase()) {
if (!it->second.IsDirty()) { // TODO a cursor can only contain dirty entries
continue;
}
Expand All @@ -201,20 +199,20 @@ void CCoinsViewCache::BatchWrite(CoinsViewCacheCursor& cursor, const uint256& in
// Move the data up and mark it as dirty.
CCoinsCacheEntry& entry{itUs->second};
assert(entry.coin.DynamicMemoryUsage() == 0);
if (cursor.WillErase(*it)) {
if (cursor.WillErase(it)) {
// Since this entry will be erased,
// we can move the coin into us instead of copying it
entry.coin = std::move(it->second.coin);
} else {
entry.coin = it->second.coin;
}
CCoinsCacheEntry::SetDirty(*itUs, m_sentinel);
CCoinsCacheEntry::SetDirty(*itUs);
++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
if (it->second.IsFresh()) CCoinsCacheEntry::SetFresh(*itUs, m_sentinel);
if (it->second.IsFresh()) CCoinsCacheEntry::SetFresh(*itUs);
}
} else {
// Found the entry in the parent cache
Expand All @@ -235,7 +233,7 @@ void CCoinsViewCache::BatchWrite(CoinsViewCacheCursor& cursor, const uint256& in
} else {
// A normal modification.
Assume(TrySub(cachedCoinsUsage, itUs->second.coin.DynamicMemoryUsage()));
if (cursor.WillErase(*it)) {
if (cursor.WillErase(it)) {
// Since this entry will be erased,
// we can move the coin into us instead of copying it
itUs->second.coin = std::move(it->second.coin);
Expand All @@ -244,7 +242,7 @@ void CCoinsViewCache::BatchWrite(CoinsViewCacheCursor& cursor, const uint256& in
}
cachedCoinsUsage += itUs->second.coin.DynamicMemoryUsage();
if (!itUs->second.IsDirty()) {
CCoinsCacheEntry::SetDirty(*itUs, m_sentinel);
CCoinsCacheEntry::SetDirty(*itUs);
++m_dirty_count;
}
// NOTE: It isn't safe to mark the coin as FRESH in the parent
Expand All @@ -259,7 +257,7 @@ void CCoinsViewCache::BatchWrite(CoinsViewCacheCursor& cursor, const uint256& in

void CCoinsViewCache::Flush(bool reallocate_cache)
{
auto cursor{CoinsViewCacheCursor(m_dirty_count, m_sentinel, cacheCoins, /*will_erase=*/true)};
auto cursor{CoinsViewCacheCursor(m_dirty_count, cacheCoins, /*will_erase=*/true)};
base->BatchWrite(cursor, m_block_hash);
Assume(m_dirty_count == 0);
cacheCoins.clear();
Expand All @@ -271,13 +269,9 @@ void CCoinsViewCache::Flush(bool reallocate_cache)

void CCoinsViewCache::Sync()
{
auto cursor{CoinsViewCacheCursor(m_dirty_count, m_sentinel, cacheCoins, /*will_erase=*/false)};
auto cursor{CoinsViewCacheCursor(m_dirty_count, cacheCoins, /*will_erase=*/false)};
base->BatchWrite(cursor, m_block_hash);
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");
}
}

void CCoinsViewCache::Reset() noexcept
Expand Down Expand Up @@ -346,12 +340,10 @@ void CCoinsViewCache::SanityCheck() const
// Count the number of entries we expect in the linked list.
if (entry.IsDirty()) ++count_dirty;
}
// Iterate over the linked list of flagged entries.
// Iterate over the flagged entries.
auto cursor{CoinsViewCacheCursor(m_dirty_count, cacheCoins, /*will_erase=*/true)};
size_t count_linked = 0;
for (auto it = m_sentinel.second.Next(); it != &m_sentinel; it = it->second.Next()) {
// Verify linked list integrity.
assert(it->second.Next()->second.Prev() == it);
assert(it->second.Prev()->second.Next() == it);
for (auto it{cursor.Begin()}; it != cursor.End(); it.Next()) {
// Verify they are actually flagged.
assert(it->second.IsDirty());
// Count the number of entries actually in the list.
Expand Down
125 changes: 49 additions & 76 deletions src/coins.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,32 +109,11 @@ using CoinsCachePair = std::pair<const COutPoint, CCoinsCacheEntry>;
struct CCoinsCacheEntry
{
private:
/**
* These are used to create a doubly linked list of flagged entries.
* They are set in SetDirty, SetFresh, and unset in SetClean.
* A flagged entry is any entry that is either DIRTY, FRESH, or both.
*
* DIRTY entries are tracked so that only modified entries can be passed to
* the parent cache for batch writing. This is a performance optimization
* compared to giving all entries in the cache to the parent and having the
* parent scan for only modified entries.
*/
CoinsCachePair* m_prev{nullptr};
CoinsCachePair* m_next{nullptr};
uint8_t m_flags{0};

//! Adding a flag requires a reference to the sentinel of the flagged pair linked list.
static void AddFlags(uint8_t flags, CoinsCachePair& pair, CoinsCachePair& sentinel) noexcept
static void AddFlags(uint8_t flags, CoinsCachePair& pair) noexcept
{
Assume(flags & (DIRTY | FRESH));
if (!pair.second.m_flags) {
Assume(!pair.second.m_prev && !pair.second.m_next);
pair.second.m_prev = sentinel.second.m_prev;
pair.second.m_next = &sentinel;
sentinel.second.m_prev = &pair;
pair.second.m_prev->second.m_next = &pair;
}
Assume(pair.second.m_prev && pair.second.m_next);
pair.second.m_flags |= flags;
}

Expand Down Expand Up @@ -169,43 +148,16 @@ struct CCoinsCacheEntry
SetClean();
}

static void SetDirty(CoinsCachePair& pair, CoinsCachePair& sentinel) noexcept { AddFlags(DIRTY, pair, sentinel); }
static void SetFresh(CoinsCachePair& pair, CoinsCachePair& sentinel) noexcept { AddFlags(FRESH, pair, sentinel); }
static void SetDirty(CoinsCachePair& pair) noexcept { AddFlags(DIRTY, pair); }
static void SetFresh(CoinsCachePair& pair) noexcept { AddFlags(FRESH, pair); }

void SetClean() noexcept
{
if (!m_flags) return;
m_next->second.m_prev = m_prev;
m_prev->second.m_next = m_next;
m_flags = 0;
m_prev = m_next = nullptr;
}
bool IsDirty() const noexcept { return m_flags & DIRTY; }
bool IsFresh() const noexcept { return m_flags & FRESH; }

//! Only call Next when this entry is DIRTY, FRESH, or both
CoinsCachePair* Next() const noexcept
{
Assume(m_flags);
return m_next;
}

//! Only call Prev when this entry is DIRTY, FRESH, or both
CoinsCachePair* Prev() const noexcept
{
Assume(m_flags);
return m_prev;
}

//! Only use this for initializing the linked list sentinel
void SelfRef(CoinsCachePair& pair) noexcept
{
Assume(&pair.second == this);
m_prev = &pair;
m_next = &pair;
// Set sentinel to DIRTY so we can call Next on it
m_flags = DIRTY;
}
bool IsFlagged() const noexcept { return bool(m_flags); }
};

/**
Expand Down Expand Up @@ -247,6 +199,8 @@ class CCoinsViewCursor
/**
* Cursor for iterating over the linked list of flagged entries in CCoinsViewCache.
*
* Wraps an iterator, and it skips over non-flagged entries.
*
* This is a helper struct to encapsulate the diverging logic between a non-erasing
* CCoinsViewCache::Sync and an erasing CCoinsViewCache::Flush. This allows the receiver
* of CCoinsView::BatchWrite to iterate through the flagged entries without knowing
Expand All @@ -267,38 +221,59 @@ 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& dirty_count LIFETIMEBOUND,
CoinsCachePair& sentinel LIFETIMEBOUND,
CCoinsMap& map LIFETIMEBOUND,
bool will_erase) noexcept
: m_dirty_count(dirty_count), m_sentinel(sentinel), m_map(map), m_will_erase(will_erase) {}
: m_dirty_count(dirty_count), 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; }

//! Return the next entry after current, possibly erasing current
inline CoinsCachePair* NextAndMaybeErase(CoinsCachePair& current) noexcept
{
const auto next_entry{current.second.Next()};
Assume(TrySub(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) {
if (current.second.coin.IsSpent()) {
assert(current.second.coin.DynamicMemoryUsage() == 0); // scriptPubKey was already cleared in SpendCoin
m_map.erase(current.first);
} else {
current.second.SetClean();
class Iterator {
private:
CCoinsMap::iterator m_iter;
const CoinsViewCacheCursor& m_cursor;
public:
Iterator(CCoinsMap::iterator iter, const CoinsViewCacheCursor& cursor) : m_iter(std::move(iter)), m_cursor(cursor) {}
inline void SkipOverNonFlagged() noexcept {
while (m_iter != m_cursor.m_map.end() && !m_iter->second.IsFlagged()) {
++m_iter;
}
}
return next_entry;
inline void Next() noexcept {
++m_iter;
SkipOverNonFlagged();
}
//! Return the next entry after current, possibly erasing current
inline void NextAndMaybeErase() noexcept
{
const auto current{m_iter};
// go to next
++m_iter;
SkipOverNonFlagged();
Assume(TrySub(m_cursor.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_cursor.m_will_erase) {
if (current->second.coin.IsSpent()) {
assert(current->second.coin.DynamicMemoryUsage() == 0); // scriptPubKey was already cleared in SpendCoin
m_cursor.m_map.erase(current->first);
} else {
current->second.SetClean();
}
}
}
inline const CCoinsMap::iterator& operator->() const { return m_iter; }
inline bool operator==(Iterator& it2) { return it2.m_iter == m_iter; }
};
inline Iterator Begin() const noexcept {
auto iter{Iterator(m_map.begin(), *this)};
iter.SkipOverNonFlagged();
return iter;
}
inline Iterator End() const noexcept { return Iterator(m_map.end(), *this); }

inline bool WillErase(CoinsCachePair& current) const noexcept { return m_will_erase || current.second.coin.IsSpent(); }
inline bool WillErase(Iterator& it) const noexcept { return m_will_erase || it->second.coin.IsSpent(); }
size_t GetDirtyCount() const noexcept { return m_dirty_count; }
size_t GetTotalCount() const noexcept { return m_map.size(); }
private:
size_t& m_dirty_count;
CoinsCachePair& m_sentinel;
CCoinsMap& m_map;
bool m_will_erase;
};
Expand Down Expand Up @@ -361,7 +336,7 @@ class CoinsViewEmpty : public CCoinsView
std::vector<uint256> GetHeadBlocks() const override { return {}; }
void BatchWrite(CoinsViewCacheCursor& cursor, const uint256&) override
{
for (auto it{cursor.Begin()}; it != cursor.End(); it = cursor.NextAndMaybeErase(*it)) { }
for (auto it{cursor.Begin()}; it != cursor.End(); it.NextAndMaybeErase()) { }
}
std::unique_ptr<CCoinsViewCursor> Cursor() const override { return {}; }
size_t EstimateSize() const override { return 0; }
Expand Down Expand Up @@ -402,8 +377,6 @@ class CCoinsViewCache : public CCoinsViewBacked
*/
mutable uint256 m_block_hash;
mutable CCoinsMapMemoryResource m_cache_coins_memory_resource{};
/* The starting sentinel of the flagged entry circular doubly linked list. */
mutable CoinsCachePair m_sentinel;
mutable CCoinsMap cacheCoins;

/* Cached dynamic memory usage for the inner Coin objects. */
Expand Down
17 changes: 7 additions & 10 deletions src/test/coins_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class CCoinsViewTest : public CoinsViewEmpty

void BatchWrite(CoinsViewCacheCursor& cursor, const uint256& block_hash) override
{
for (auto it{cursor.Begin()}; it != cursor.End(); it = cursor.NextAndMaybeErase(*it)){
for (auto it{cursor.Begin()}; it != cursor.End(); it.NextAndMaybeErase()){
if (it->second.IsDirty()) {
// Same optimization used in CCoinsViewDB is to only write dirty entries.
map_[it->first] = it->second.coin;
Expand Down Expand Up @@ -95,7 +95,6 @@ class CCoinsViewCacheTest : public CCoinsViewCache
}

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; }
};
Expand Down Expand Up @@ -631,14 +630,14 @@ static void SetCoinsValue(const CAmount value, Coin& coin)
}
}

static size_t InsertCoinsMapEntry(CCoinsMap& map, CoinsCachePair& sentinel, const CoinEntry& cache_coin)
static size_t InsertCoinsMapEntry(CCoinsMap& map, const CoinEntry& cache_coin)
{
CCoinsCacheEntry entry;
SetCoinsValue(cache_coin.value, entry.coin);
auto [iter, inserted] = map.emplace(OUTPOINT, std::move(entry));
assert(inserted);
if (cache_coin.IsDirty()) CCoinsCacheEntry::SetDirty(*iter, sentinel);
if (cache_coin.IsFresh()) CCoinsCacheEntry::SetFresh(*iter, sentinel);
if (cache_coin.IsDirty()) CCoinsCacheEntry::SetDirty(*iter);
if (cache_coin.IsFresh()) CCoinsCacheEntry::SetFresh(*iter);
return iter->second.coin.DynamicMemoryUsage();
}

Expand All @@ -654,13 +653,11 @@ static MaybeCoin GetCoinsMapEntry(const CCoinsMap& map, const COutPoint& outp =

static void WriteCoinsViewEntry(CCoinsView& view, const MaybeCoin& cache_coin)
{
CoinsCachePair sentinel{};
sentinel.second.SelfRef(sentinel);
CCoinsMapMemoryResource resource;
CCoinsMap map{0, CCoinsMap::hasher{}, CCoinsMap::key_equal{}, &resource};
if (cache_coin) InsertCoinsMapEntry(map, sentinel, *cache_coin);
if (cache_coin) InsertCoinsMapEntry(map, *cache_coin);
size_t dirty_count{cache_coin && cache_coin->IsDirty()};
auto cursor{CoinsViewCacheCursor(dirty_count, sentinel, map, /*will_erase=*/true)};
auto cursor{CoinsViewCacheCursor(dirty_count, map, /*will_erase=*/true)};
view.BatchWrite(cursor, {});
BOOST_CHECK_EQUAL(dirty_count, 0U);
}
Expand All @@ -673,7 +670,7 @@ class SingleEntryCacheTest
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);
cache.usage() += InsertCoinsMapEntry(cache.map(), *cache_coin);
cache.dirty() += cache_coin->IsDirty();
}
}
Expand Down
Loading
Loading