coins: reduce lookups in dbcache layer propagation
What the developer wrote
Authored by Lőrinc
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 <martin.ankerl@gmail.com>
Co-authored-by: Andrew Toth <andrewstoth@gmail.com>
Co-authored-by: optout <13562139+optout21@users.noreply.github.com>
What changed, and why it matters
This is a small internal performance cleanup in Bitcoin Core's coin cache code. It replaces two hash-map operations with one, reducing CPU work when copying coin data from a child cache into a parent cache. The commit explicitly says behavior is unchanged; no security vulnerability is described or evident in the diff.
No security action required. Treat as normal code-review/performance improvement. If reviewing, verify the FRESH && SPENT erase path preserves invariants and does not introduce iterator invalidation issues in the surrounding loop.
Security signals we found
No security-relevant signals in diff or commit message
Performance optimization only
Explicit claim of unchanged semantics
Evidence from the diff
In CCoinsViewCache::BatchWrite, the old code did cacheCoins.find() then cacheCoins.try_emplace() when the parent lacked an entry. The patch performs a single try_emplace and branches on the returned inserted flag. For the FRESH && SPENT case it immediately erases the placeholder. The logic for all valid parent/child states is intended to be identical; the change is purely a reduction in SipHash/bucket-traversal work on the insert path.
Changed components
src/coins.cppCCoinsViewCache::BatchWriteUTXO cache layer propagationInspect captured patch +8 / −10
diff --git a/src/coins.cpp b/src/coins.cpp
index 090d36dd..e650b81f 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)) {
Why this scored 18/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.