Skip to content

Commit 8bd3959

Browse files
andrewtothandrewtoth
committed
refactor: require self and sentinel parameters for AddFlags
No behavior change. Prepares for adding the CoinsCachePairs to a linked list when flagged.
1 parent 75f36d2 commit 8bd3959

File tree

4 files changed

+41
-16
lines changed

4 files changed

+41
-16
lines changed

‎src/coins.cpp

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,9 @@ size_t CCoinsViewBacked::EstimateSize() const { return base->EstimateSize(); }
3434
CCoinsViewCache::CCoinsViewCache(CCoinsView* baseIn, bool deterministic) :
3535
CCoinsViewBacked(baseIn), m_deterministic(deterministic),
3636
cacheCoins(0, SaltedOutpointHasher(/*deterministic=*/deterministic), CCoinsMap::key_equal{}, &m_cache_coins_memory_resource)
37-
{}
37+
{
38+
m_sentinel.second.SelfRef(m_sentinel);
39+
}
3840

3941
size_t CCoinsViewCache::DynamicMemoryUsage() const {
4042
return memusage::DynamicUsage(cacheCoins) + cachedCoinsUsage;
@@ -51,7 +53,7 @@ CCoinsMap::iterator CCoinsViewCache::FetchCoin(const COutPoint &outpoint) const
5153
if (ret->second.coin.IsSpent()) {
5254
// The parent only has an empty entry for this outpoint; we can consider our
5355
// version as fresh.
54-
ret->second.AddFlags(CCoinsCacheEntry::FRESH);
56+
ret->second.AddFlags(CCoinsCacheEntry::FRESH, *ret, m_sentinel);
5557
}
5658
cachedCoinsUsage += ret->second.coin.DynamicMemoryUsage();
5759
return ret;
@@ -96,7 +98,7 @@ void CCoinsViewCache::AddCoin(const COutPoint &outpoint, Coin&& coin, bool possi
9698
fresh = !it->second.IsDirty();
9799
}
98100
it->second.coin = std::move(coin);
99-
it->second.AddFlags(CCoinsCacheEntry::DIRTY | (fresh ? CCoinsCacheEntry::FRESH : 0));
101+
it->second.AddFlags(CCoinsCacheEntry::DIRTY | (fresh ? CCoinsCacheEntry::FRESH : 0), *it, m_sentinel);
100102
cachedCoinsUsage += it->second.coin.DynamicMemoryUsage();
101103
TRACE5(utxocache, add,
102104
outpoint.hash.data(),
@@ -113,7 +115,7 @@ void CCoinsViewCache::EmplaceCoinInternalDANGER(COutPoint&& outpoint, Coin&& coi
113115
std::forward_as_tuple(std::move(outpoint)),
114116
std::forward_as_tuple(std::move(coin)));
115117
if (inserted) {
116-
it->second.AddFlags(CCoinsCacheEntry::DIRTY);
118+
it->second.AddFlags(CCoinsCacheEntry::DIRTY, *it, m_sentinel);
117119
}
118120
}
119121

@@ -144,7 +146,7 @@ bool CCoinsViewCache::SpendCoin(const COutPoint &outpoint, Coin* moveout) {
144146
if (it->second.IsFresh()) {
145147
cacheCoins.erase(it);
146148
} else {
147-
it->second.AddFlags(CCoinsCacheEntry::DIRTY);
149+
it->second.AddFlags(CCoinsCacheEntry::DIRTY, *it, m_sentinel);
148150
it->second.coin.Clear();
149151
}
150152
return true;
@@ -196,7 +198,8 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn
196198
if (!(it->second.IsFresh() && it->second.coin.IsSpent())) {
197199
// Create the coin in the parent cache, move the data up
198200
// and mark it as dirty.
199-
CCoinsCacheEntry& entry = cacheCoins[it->first];
201+
itUs = cacheCoins.try_emplace(it->first).first;
202+
CCoinsCacheEntry& entry{itUs->second};
200203
if (erase) {
201204
// The `move` call here is purely an optimization; we rely on the
202205
// `mapCoins.erase` call in the `for` expression to actually remove
@@ -206,12 +209,12 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn
206209
entry.coin = it->second.coin;
207210
}
208211
cachedCoinsUsage += entry.coin.DynamicMemoryUsage();
209-
entry.AddFlags(CCoinsCacheEntry::DIRTY);
212+
entry.AddFlags(CCoinsCacheEntry::DIRTY, *itUs, m_sentinel);
210213
// We can mark it FRESH in the parent if it was FRESH in the child
211214
// Otherwise it might have just been flushed from the parent's cache
212215
// and already exist in the grandparent
213216
if (it->second.IsFresh()) {
214-
entry.AddFlags(CCoinsCacheEntry::FRESH);
217+
entry.AddFlags(CCoinsCacheEntry::FRESH, *itUs, m_sentinel);
215218
}
216219
}
217220
} else {
@@ -241,7 +244,7 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn
241244
itUs->second.coin = it->second.coin;
242245
}
243246
cachedCoinsUsage += itUs->second.coin.DynamicMemoryUsage();
244-
itUs->second.AddFlags(CCoinsCacheEntry::DIRTY);
247+
itUs->second.AddFlags(CCoinsCacheEntry::DIRTY, *itUs, m_sentinel);
245248
// NOTE: It isn't safe to mark the coin as FRESH in the parent
246249
// cache. If it already existed and was spent in the parent
247250
// cache then marking it FRESH would prevent that spentness

‎src/coins.h

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include <serialize.h>
1414
#include <support/allocators/pool.h>
1515
#include <uint256.h>
16+
#include <util/check.h>
1617
#include <util/hasher.h>
1718

1819
#include <assert.h>
@@ -136,14 +137,27 @@ struct CCoinsCacheEntry
136137
CCoinsCacheEntry() noexcept = default;
137138
explicit CCoinsCacheEntry(Coin&& coin_) noexcept : coin(std::move(coin_)) {}
138139

139-
inline void AddFlags(uint8_t flags) noexcept { m_flags |= flags; }
140+
//! Adding a flag also requires a self reference to the pair that contains
141+
//! this entry in the CCoinsCache map and a reference to the sentinel of the
142+
//! flagged pair linked list.
143+
inline void AddFlags(uint8_t flags, CoinsCachePair& self, CoinsCachePair& sentinel) noexcept
144+
{
145+
Assume(&self.second == this);
146+
m_flags |= flags;
147+
}
140148
inline void ClearFlags() noexcept
141149
{
142150
m_flags = 0;
143151
}
144152
inline uint8_t GetFlags() const noexcept { return m_flags; }
145153
inline bool IsDirty() const noexcept { return m_flags & DIRTY; }
146154
inline bool IsFresh() const noexcept { return m_flags & FRESH; }
155+
156+
//! Only use this for initializing the linked list sentinel
157+
inline void SelfRef(CoinsCachePair& self) noexcept
158+
{
159+
Assume(&self.second == this);
160+
}
147161
};
148162

149163
/**
@@ -251,6 +265,8 @@ class CCoinsViewCache : public CCoinsViewBacked
251265
*/
252266
mutable uint256 hashBlock;
253267
mutable CCoinsMapMemoryResource m_cache_coins_memory_resource{};
268+
/* The starting sentinel of the flagged entry circular doubly linked list. */
269+
mutable CoinsCachePair m_sentinel;
254270
mutable CCoinsMap cacheCoins;
255271

256272
/* Cached dynamic memory usage for the inner Coin objects. */

‎src/test/coins_tests.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ class CCoinsViewCacheTest : public CCoinsViewCache
9292
}
9393

9494
CCoinsMap& map() const { return cacheCoins; }
95+
CoinsCachePair& sentinel() const { return m_sentinel; }
9596
size_t& usage() const { return cachedCoinsUsage; }
9697
};
9798

@@ -576,18 +577,18 @@ static void SetCoinsValue(CAmount value, Coin& coin)
576577
}
577578
}
578579

579-
static size_t InsertCoinsMapEntry(CCoinsMap& map, CAmount value, char flags)
580+
static size_t InsertCoinsMapEntry(CCoinsMap& map, CoinsCachePair& sentinel, CAmount value, char flags)
580581
{
581582
if (value == ABSENT) {
582583
assert(flags == NO_ENTRY);
583584
return 0;
584585
}
585586
assert(flags != NO_ENTRY);
586587
CCoinsCacheEntry entry;
587-
entry.AddFlags(flags);
588588
SetCoinsValue(value, entry.coin);
589589
auto inserted = map.emplace(OUTPOINT, std::move(entry));
590590
assert(inserted.second);
591+
inserted.first->second.AddFlags(flags, *inserted.first, sentinel);
591592
return inserted.first->second.coin.DynamicMemoryUsage();
592593
}
593594

@@ -610,9 +611,11 @@ void GetCoinsMapEntry(const CCoinsMap& map, CAmount& value, char& flags, const C
610611

611612
void WriteCoinsViewEntry(CCoinsView& view, CAmount value, char flags)
612613
{
614+
CoinsCachePair sentinel{};
615+
sentinel.second.SelfRef(sentinel);
613616
CCoinsMapMemoryResource resource;
614617
CCoinsMap map{0, CCoinsMap::hasher{}, CCoinsMap::key_equal{}, &resource};
615-
InsertCoinsMapEntry(map, value, flags);
618+
InsertCoinsMapEntry(map, sentinel, value, flags);
616619
BOOST_CHECK(view.BatchWrite(map, {}));
617620
}
618621

@@ -622,7 +625,7 @@ class SingleEntryCacheTest
622625
SingleEntryCacheTest(CAmount base_value, CAmount cache_value, char cache_flags)
623626
{
624627
WriteCoinsViewEntry(base, base_value, base_value == ABSENT ? NO_ENTRY : DIRTY);
625-
cache.usage() += InsertCoinsMapEntry(cache.map(), cache_value, cache_flags);
628+
cache.usage() += InsertCoinsMapEntry(cache.map(), cache.sentinel(), cache_value, cache_flags);
626629
}
627630

628631
CCoinsView root;

‎src/test/fuzz/coins_view.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,12 +120,14 @@ FUZZ_TARGET(coins_view, .init = initialize_coins_view)
120120
random_mutable_transaction = *opt_mutable_transaction;
121121
},
122122
[&] {
123+
CoinsCachePair sentinel{};
124+
sentinel.second.SelfRef(sentinel);
123125
CCoinsMapMemoryResource resource;
124126
CCoinsMap coins_map{0, SaltedOutpointHasher{/*deterministic=*/true}, CCoinsMap::key_equal{}, &resource};
125127
LIMITED_WHILE(good_data && fuzzed_data_provider.ConsumeBool(), 10'000)
126128
{
127129
CCoinsCacheEntry coins_cache_entry;
128-
coins_cache_entry.AddFlags(fuzzed_data_provider.ConsumeIntegral<uint8_t>());
130+
const auto flags{fuzzed_data_provider.ConsumeIntegral<uint8_t>()};
129131
if (fuzzed_data_provider.ConsumeBool()) {
130132
coins_cache_entry.coin = random_coin;
131133
} else {
@@ -136,7 +138,8 @@ FUZZ_TARGET(coins_view, .init = initialize_coins_view)
136138
}
137139
coins_cache_entry.coin = *opt_coin;
138140
}
139-
coins_map.emplace(random_out_point, std::move(coins_cache_entry));
141+
auto it{coins_map.emplace(random_out_point, std::move(coins_cache_entry)).first};
142+
it->second.AddFlags(flags, *it, sentinel);
140143
}
141144
bool expected_code_path = false;
142145
try {

0 commit comments

Comments
 (0)