From c8f5e446dc95712a63e4dd88786e2f7cb697b986 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Mon, 1 Sep 2025 21:29:37 -0700 Subject: [PATCH] coins: reduce lookups in dbcache layer propagation Previously, when the parent coins cache had no entry and the child did, `BatchWrite` performed a find followed by `try_emplace`, which resulted in multiple `SipHash` computations and bucket traversals on the common insert path. This change uses a single leading `try_emplace` and branches on the returned `inserted` flag. In the `FRESH && SPENT` case (only exercised by tests), we erase the just-inserted placeholder (which is constant time with no rehash anyway). Semantics are unchanged for all valid parent/child state combinations. This change is a minimal version of https://github.com/bitcoin/bitcoin/pull/32128/commits/723c49b63bb10da843fbb6efc6928dca415cc47f and draws simplification ideas https://github.com/bitcoin/bitcoin/pull/30673/commits/ae76ec7bcff0a08a61f294882a71e46d177b009f. Added TODO versions for related pre-existing issues that should be fixed in follow-ups. Co-authored-by: Martin Ankerl Co-authored-by: Andrew Toth Co-authored-by: optout <13562139+optout21@users.noreply.github.com> --- src/coins.cpp | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index 090d36dd041..e650b81fe4b 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -185,18 +185,16 @@ void CCoinsViewCache::SetBestBlock(const uint256 &hashBlockIn) { bool CCoinsViewCache::BatchWrite(CoinsViewCacheCursor& cursor, const uint256 &hashBlockIn) { for (auto it{cursor.Begin()}; it != cursor.End(); it = cursor.NextAndMaybeErase(*it)) { - // Ignore non-dirty entries (optimization). - if (!it->second.IsDirty()) { + if (!it->second.IsDirty()) { // TODO a cursor can only contain dirty entries continue; } - CCoinsMap::iterator itUs = cacheCoins.find(it->first); - if (itUs == cacheCoins.end()) { - // The parent cache does not have an entry, while the child cache does. - // We can ignore it if it's both spent and FRESH in the child - if (!(it->second.IsFresh() && it->second.coin.IsSpent())) { - // Create the coin in the parent cache, move the data up - // and mark it as dirty. - itUs = cacheCoins.try_emplace(it->first).first; + auto [itUs, inserted]{cacheCoins.try_emplace(it->first)}; + if (inserted) { + if (it->second.IsFresh() && it->second.coin.IsSpent()) { + cacheCoins.erase(itUs); // TODO fresh coins should have been removed at spend + } else { + // The parent cache does not have an entry, while the child cache does. + // Move the data up and mark it as dirty. CCoinsCacheEntry& entry{itUs->second}; assert(entry.coin.DynamicMemoryUsage() == 0); if (cursor.WillErase(*it)) {