Skip to content

Commit 5b9748f

Browse files
danradanra
committed
Small refactor of CCoinsViewCache::BatchWrite()
std::unordered_map::erase( const_iterator pos ) returns an iterator to the element following the removed one. Use that to optimize (probably minor-performance-wise, and definitely code-structure-wise) the implementation of CCoinsViewCache::BatchWrite().
1 parent e278f86 commit 5b9748f

File tree

1 file changed

+48
-46
lines changed

1 file changed

+48
-46
lines changed

‎src/coins.cpp‎

Lines changed: 48 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -146,56 +146,58 @@ void CCoinsViewCache::SetBestBlock(const uint256 &hashBlockIn) {
146146
}
147147

148148
bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn) {
149-
for (CCoinsMap::iterator it = mapCoins.begin(); it != mapCoins.end();) {
150-
if (it->second.flags & CCoinsCacheEntry::DIRTY) { // Ignore non-dirty entries (optimization).
151-
CCoinsMap::iterator itUs = cacheCoins.find(it->first);
152-
if (itUs == cacheCoins.end()) {
153-
// The parent cache does not have an entry, while the child does
154-
// We can ignore it if it's both FRESH and pruned in the child
155-
if (!(it->second.flags & CCoinsCacheEntry::FRESH && it->second.coin.IsSpent())) {
156-
// Otherwise we will need to create it in the parent
157-
// and move the data up and mark it as dirty
158-
CCoinsCacheEntry& entry = cacheCoins[it->first];
159-
entry.coin = std::move(it->second.coin);
160-
cachedCoinsUsage += entry.coin.DynamicMemoryUsage();
161-
entry.flags = CCoinsCacheEntry::DIRTY;
162-
// We can mark it FRESH in the parent if it was FRESH in the child
163-
// Otherwise it might have just been flushed from the parent's cache
164-
// and already exist in the grandparent
165-
if (it->second.flags & CCoinsCacheEntry::FRESH)
166-
entry.flags |= CCoinsCacheEntry::FRESH;
149+
for (CCoinsMap::iterator it = mapCoins.begin(); it != mapCoins.end(); it = mapCoins.erase(it)) {
150+
// Ignore non-dirty entries (optimization).
151+
if (!(it->second.flags & CCoinsCacheEntry::DIRTY)) {
152+
continue;
153+
}
154+
CCoinsMap::iterator itUs = cacheCoins.find(it->first);
155+
if (itUs == cacheCoins.end()) {
156+
// The parent cache does not have an entry, while the child does
157+
// We can ignore it if it's both FRESH and pruned in the child
158+
if (!(it->second.flags & CCoinsCacheEntry::FRESH && it->second.coin.IsSpent())) {
159+
// Otherwise we will need to create it in the parent
160+
// and move the data up and mark it as dirty
161+
CCoinsCacheEntry& entry = cacheCoins[it->first];
162+
entry.coin = std::move(it->second.coin);
163+
cachedCoinsUsage += entry.coin.DynamicMemoryUsage();
164+
entry.flags = CCoinsCacheEntry::DIRTY;
165+
// We can mark it FRESH in the parent if it was FRESH in the child
166+
// Otherwise it might have just been flushed from the parent's cache
167+
// and already exist in the grandparent
168+
if (it->second.flags & CCoinsCacheEntry::FRESH) {
169+
entry.flags |= CCoinsCacheEntry::FRESH;
167170
}
168-
} else {
169-
// Assert that the child cache entry was not marked FRESH if the
170-
// parent cache entry has unspent outputs. If this ever happens,
171-
// it means the FRESH flag was misapplied and there is a logic
172-
// error in the calling code.
173-
if ((it->second.flags & CCoinsCacheEntry::FRESH) && !itUs->second.coin.IsSpent())
174-
throw std::logic_error("FRESH flag misapplied to cache entry for base transaction with spendable outputs");
171+
}
172+
} else {
173+
// Assert that the child cache entry was not marked FRESH if the
174+
// parent cache entry has unspent outputs. If this ever happens,
175+
// it means the FRESH flag was misapplied and there is a logic
176+
// error in the calling code.
177+
if ((it->second.flags & CCoinsCacheEntry::FRESH) && !itUs->second.coin.IsSpent()) {
178+
throw std::logic_error("FRESH flag misapplied to cache entry for base transaction with spendable outputs");
179+
}
175180

176-
// Found the entry in the parent cache
177-
if ((itUs->second.flags & CCoinsCacheEntry::FRESH) && it->second.coin.IsSpent()) {
178-
// The grandparent does not have an entry, and the child is
179-
// modified and being pruned. This means we can just delete
180-
// it from the parent.
181-
cachedCoinsUsage -= itUs->second.coin.DynamicMemoryUsage();
182-
cacheCoins.erase(itUs);
183-
} else {
184-
// A normal modification.
185-
cachedCoinsUsage -= itUs->second.coin.DynamicMemoryUsage();
186-
itUs->second.coin = std::move(it->second.coin);
187-
cachedCoinsUsage += itUs->second.coin.DynamicMemoryUsage();
188-
itUs->second.flags |= CCoinsCacheEntry::DIRTY;
189-
// NOTE: It is possible the child has a FRESH flag here in
190-
// the event the entry we found in the parent is pruned. But
191-
// we must not copy that FRESH flag to the parent as that
192-
// pruned state likely still needs to be communicated to the
193-
// grandparent.
194-
}
181+
// Found the entry in the parent cache
182+
if ((itUs->second.flags & CCoinsCacheEntry::FRESH) && it->second.coin.IsSpent()) {
183+
// The grandparent does not have an entry, and the child is
184+
// modified and being pruned. This means we can just delete
185+
// it from the parent.
186+
cachedCoinsUsage -= itUs->second.coin.DynamicMemoryUsage();
187+
cacheCoins.erase(itUs);
188+
} else {
189+
// A normal modification.
190+
cachedCoinsUsage -= itUs->second.coin.DynamicMemoryUsage();
191+
itUs->second.coin = std::move(it->second.coin);
192+
cachedCoinsUsage += itUs->second.coin.DynamicMemoryUsage();
193+
itUs->second.flags |= CCoinsCacheEntry::DIRTY;
194+
// NOTE: It is possible the child has a FRESH flag here in
195+
// the event the entry we found in the parent is pruned. But
196+
// we must not copy that FRESH flag to the parent as that
197+
// pruned state likely still needs to be communicated to the
198+
// grandparent.
195199
}
196200
}
197-
CCoinsMap::iterator itOld = it++;
198-
mapCoins.erase(itOld);
199201
}
200202
hashBlock = hashBlockIn;
201203
return true;

0 commit comments

Comments
 (0)