Merge #11353: Small refactor of CCoinsViewCache::BatchWrite()

5b9748f97 Small refactor of CCoinsViewCache::BatchWrite() (Dan Raviv)

Pull request description:

  `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()`.

Tree-SHA512: 00abc838ad91771cfcddd45688841c9414869b75289d09b483a7f0ba835614fe189e9c8aca8a80e3de78ee397ec14083ae52e2e92b7863b3b6eb0d0cb892c9dd

src/coins.cpp

@@ -146,8 +146,11 @@ void CCoinsViewCache::SetBestBlock(const uint256 &hashBlockIn) {
 }
 
 bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn) {
-    for (CCoinsMap::iterator it = mapCoins.begin(); it != mapCoins.end();) {
+    for (CCoinsMap::iterator it = mapCoins.begin(); it != mapCoins.end(); it = mapCoins.erase(it)) {
-        if (it->second.flags & CCoinsCacheEntry::DIRTY) { // Ignore non-dirty entries (optimization).
+        // Ignore non-dirty entries (optimization).
+        if (!(it->second.flags & CCoinsCacheEntry::DIRTY)) {
+            continue;
+        }
         CCoinsMap::iterator itUs = cacheCoins.find(it->first);
         if (itUs == cacheCoins.end()) {
             // The parent cache does not have an entry, while the child does
@@ -162,16 +165,18 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn
                 // 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.flags & CCoinsCacheEntry::FRESH)
+                if (it->second.flags & CCoinsCacheEntry::FRESH) {
                     entry.flags |= CCoinsCacheEntry::FRESH;
                 }
+            }
         } else {
             // Assert that the child cache entry was not marked FRESH if the
             // parent cache entry has unspent outputs. If this ever happens,
             // it means the FRESH flag was misapplied and there is a logic
             // error in the calling code.
-                if ((it->second.flags & CCoinsCacheEntry::FRESH) && !itUs->second.coin.IsSpent())
+            if ((it->second.flags & CCoinsCacheEntry::FRESH) && !itUs->second.coin.IsSpent()) {
                 throw std::logic_error("FRESH flag misapplied to cache entry for base transaction with spendable outputs");
+            }
 
             // Found the entry in the parent cache
             if ((itUs->second.flags & CCoinsCacheEntry::FRESH) && it->second.coin.IsSpent()) {
@@ -194,9 +199,6 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn
             }
         }
     }
-        CCoinsMap::iterator itOld = it++;
-        mapCoins.erase(itOld);
-    }
     hashBlock = hashBlockIn;
     return true;
 }