From f28aec014edd29cfc669cf1c3f795c0f1e2ae7e2 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Wed, 3 Sep 2014 09:01:24 +0200 Subject: Use ModifyCoins instead of mutable GetCoins. Replace the mutable non-copying GetCoins method with a ModifyCoins, which returns an encapsulated iterator, so we can keep track of concurrent modifications (as iterators can be invalidated by those) and run cleanup code after a modification is finished. This also removes the overloading of the 'GetCoins' name. --- src/coins.cpp | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) (limited to 'src/coins.cpp') diff --git a/src/coins.cpp b/src/coins.cpp index 34485db2bd..2b93c74c3a 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -73,7 +73,7 @@ bool CCoinsViewBacked::GetStats(CCoinsStats &stats) const { return base->GetStat CCoinsKeyHasher::CCoinsKeyHasher() : salt(GetRandHash()) {} -CCoinsViewCache::CCoinsViewCache(CCoinsView &baseIn, bool fDummy) : CCoinsViewBacked(baseIn), hashBlock(0) { } +CCoinsViewCache::CCoinsViewCache(CCoinsView &baseIn, bool fDummy) : CCoinsViewBacked(baseIn), hasModifier(false), hashBlock(0) { } bool CCoinsViewCache::GetCoins(const uint256 &txid, CCoins &coins) const { if (cacheCoins.count(txid)) { @@ -87,7 +87,12 @@ bool CCoinsViewCache::GetCoins(const uint256 &txid, CCoins &coins) const { return false; } -CCoinsMap::iterator CCoinsViewCache::FetchCoins(const uint256 &txid) { +CCoinsViewCache::~CCoinsViewCache() +{ + assert(!hasModifier); +} + +CCoinsMap::const_iterator CCoinsViewCache::FetchCoins(const uint256 &txid) const { CCoinsMap::iterator it = cacheCoins.find(txid); if (it != cacheCoins.end()) return it; @@ -99,15 +104,15 @@ CCoinsMap::iterator CCoinsViewCache::FetchCoins(const uint256 &txid) { return ret; } -CCoinsMap::const_iterator CCoinsViewCache::FetchCoins(const uint256 &txid) const { - /* Avoid redundant implementation with the const-cast. */ - return const_cast(this)->FetchCoins(txid); -} - -CCoins &CCoinsViewCache::GetCoins(const uint256 &txid) { - CCoinsMap::iterator it = FetchCoins(txid); - assert(it != cacheCoins.end()); - return it->second; +CCoinsModifier CCoinsViewCache::ModifyCoins(const uint256 &txid) { + assert(!hasModifier); + hasModifier = true; + std::pair ret = cacheCoins.insert(std::make_pair(txid, CCoins())); + if (ret.second) { + if (!base->GetCoins(txid, ret.first->second)) + ret.first->second.Clear(); + } + return CCoinsModifier(*this, ret.first); } const CCoins* CCoinsViewCache::AccessCoins(const uint256 &txid) const { @@ -145,6 +150,7 @@ bool CCoinsViewCache::SetBestBlock(const uint256 &hashBlockIn) { } bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn) { + assert(!hasModifier); for (CCoinsMap::iterator it = mapCoins.begin(); it != mapCoins.end();) { cacheCoins[it->first].swap(it->second); CCoinsMap::iterator itOld = it++; @@ -213,3 +219,11 @@ double CCoinsViewCache::GetPriority(const CTransaction &tx, int nHeight) const } return tx.ComputePriority(dResult); } + +CCoinsModifier::CCoinsModifier(CCoinsViewCache& cache_, CCoinsMap::iterator it_) : cache(cache_), it(it_) {} + +CCoinsModifier::~CCoinsModifier() { + assert(cache.hasModifier); + cache.hasModifier = false; + it->second.Cleanup(); +} -- cgit v1.2.3 From c9d1a81ce76737a73c9706e074a4fe8440c8277e Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Wed, 3 Sep 2014 09:25:32 +0200 Subject: Get rid of CCoinsView's SetCoins and SetBestBlock. All direct modifications are now done through ModifyCoins, and BatchWrite is used for pushing batches of queued modifications up, so we don't need the low-level SetCoins and SetBestBlock anymore in the top-level CCoinsView class. --- src/coins.cpp | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) (limited to 'src/coins.cpp') diff --git a/src/coins.cpp b/src/coins.cpp index 2b93c74c3a..9632e67f20 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -53,20 +53,16 @@ bool CCoins::Spend(int nPos) { bool CCoinsView::GetCoins(const uint256 &txid, CCoins &coins) const { return false; } -bool CCoinsView::SetCoins(const uint256 &txid, const CCoins &coins) { return false; } bool CCoinsView::HaveCoins(const uint256 &txid) const { return false; } uint256 CCoinsView::GetBestBlock() const { return uint256(0); } -bool CCoinsView::SetBestBlock(const uint256 &hashBlock) { return false; } bool CCoinsView::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) { return false; } bool CCoinsView::GetStats(CCoinsStats &stats) const { return false; } CCoinsViewBacked::CCoinsViewBacked(CCoinsView &viewIn) : base(&viewIn) { } bool CCoinsViewBacked::GetCoins(const uint256 &txid, CCoins &coins) const { return base->GetCoins(txid, coins); } -bool CCoinsViewBacked::SetCoins(const uint256 &txid, const CCoins &coins) { return base->SetCoins(txid, coins); } bool CCoinsViewBacked::HaveCoins(const uint256 &txid) const { return base->HaveCoins(txid); } uint256 CCoinsViewBacked::GetBestBlock() const { return base->GetBestBlock(); } -bool CCoinsViewBacked::SetBestBlock(const uint256 &hashBlock) { return base->SetBestBlock(hashBlock); } void CCoinsViewBacked::SetBackend(CCoinsView &viewIn) { base = &viewIn; } bool CCoinsViewBacked::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) { return base->BatchWrite(mapCoins, hashBlock); } bool CCoinsViewBacked::GetStats(CCoinsStats &stats) const { return base->GetStats(stats); } @@ -124,11 +120,6 @@ const CCoins* CCoinsViewCache::AccessCoins(const uint256 &txid) const { } } -bool CCoinsViewCache::SetCoins(const uint256 &txid, const CCoins &coins) { - cacheCoins[txid] = coins; - return true; -} - bool CCoinsViewCache::HaveCoins(const uint256 &txid) const { CCoinsMap::const_iterator it = FetchCoins(txid); // We're using vtx.empty() instead of IsPruned here for performance reasons, @@ -144,9 +135,8 @@ uint256 CCoinsViewCache::GetBestBlock() const { return hashBlock; } -bool CCoinsViewCache::SetBestBlock(const uint256 &hashBlockIn) { +void CCoinsViewCache::SetBestBlock(const uint256 &hashBlockIn) { hashBlock = hashBlockIn; - return true; } bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn) { -- cgit v1.2.3 From 058b08c147a6d56b57221faa5b6fcdb83b4140b2 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Wed, 3 Sep 2014 09:37:47 +0200 Subject: Do not keep fully spent but unwritten CCoins entries cached. Instead of storing CCoins entries directly in CCoinsMap, store a CCoinsCacheEntry which additionally keeps track of whether a particular entry is: * dirty: potentially different from its parent view. * fresh: the parent view is known to not have a non-pruned version. This allows us to skip non-dirty cache entries when pushing batches of changes up, and to remove CCoins entries about transactions that are fully spent before the parent cache learns about them. --- src/coins.cpp | 85 +++++++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 62 insertions(+), 23 deletions(-) (limited to 'src/coins.cpp') diff --git a/src/coins.cpp b/src/coins.cpp index 9632e67f20..9d60089bf5 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -71,18 +71,6 @@ CCoinsKeyHasher::CCoinsKeyHasher() : salt(GetRandHash()) {} CCoinsViewCache::CCoinsViewCache(CCoinsView &baseIn, bool fDummy) : CCoinsViewBacked(baseIn), hasModifier(false), hashBlock(0) { } -bool CCoinsViewCache::GetCoins(const uint256 &txid, CCoins &coins) const { - if (cacheCoins.count(txid)) { - coins = cacheCoins[txid]; - return true; - } - if (base->GetCoins(txid, coins)) { - cacheCoins[txid] = coins; - return true; - } - return false; -} - CCoinsViewCache::~CCoinsViewCache() { assert(!hasModifier); @@ -93,21 +81,43 @@ CCoinsMap::const_iterator CCoinsViewCache::FetchCoins(const uint256 &txid) const if (it != cacheCoins.end()) return it; CCoins tmp; - if (!base->GetCoins(txid,tmp)) + if (!base->GetCoins(txid, tmp)) return cacheCoins.end(); - CCoinsMap::iterator ret = cacheCoins.insert(it, std::make_pair(txid, CCoins())); - tmp.swap(ret->second); + CCoinsMap::iterator ret = cacheCoins.insert(std::make_pair(txid, CCoinsCacheEntry())).first; + tmp.swap(ret->second.coins); + if (ret->second.coins.IsPruned()) { + // The parent only has an empty entry for this txid; we can consider our + // version as fresh. + ret->second.flags = CCoinsCacheEntry::FRESH; + } return ret; } +bool CCoinsViewCache::GetCoins(const uint256 &txid, CCoins &coins) const { + CCoinsMap::const_iterator it = FetchCoins(txid); + if (it != cacheCoins.end()) { + coins = it->second.coins; + return true; + } + return false; +} + CCoinsModifier CCoinsViewCache::ModifyCoins(const uint256 &txid) { assert(!hasModifier); hasModifier = true; - std::pair ret = cacheCoins.insert(std::make_pair(txid, CCoins())); + std::pair ret = cacheCoins.insert(std::make_pair(txid, CCoinsCacheEntry())); if (ret.second) { - if (!base->GetCoins(txid, ret.first->second)) - ret.first->second.Clear(); + if (!base->GetCoins(txid, ret.first->second.coins)) { + // The parent view does not have this entry; mark it as fresh. + ret.first->second.coins.Clear(); + ret.first->second.flags = CCoinsCacheEntry::FRESH; + } else if (ret.first->second.coins.IsPruned()) { + // The parent view only has a pruned entry for this; mark it as fresh. + ret.first->second.flags = CCoinsCacheEntry::FRESH; + } } + // Assume that whenever ModifyCoins is called, the entry will be modified. + ret.first->second.flags |= CCoinsCacheEntry::DIRTY; return CCoinsModifier(*this, ret.first); } @@ -116,7 +126,7 @@ const CCoins* CCoinsViewCache::AccessCoins(const uint256 &txid) const { if (it == cacheCoins.end()) { return NULL; } else { - return &it->second; + return &it->second.coins; } } @@ -126,7 +136,7 @@ bool CCoinsViewCache::HaveCoins(const uint256 &txid) const { // as we only care about the case where an transaction was replaced entirely // in a reorganization (which wipes vout entirely, as opposed to spending // which just cleans individual outputs). - return (it != cacheCoins.end() && !it->second.vout.empty()); + return (it != cacheCoins.end() && !it->second.coins.vout.empty()); } uint256 CCoinsViewCache::GetBestBlock() const { @@ -142,7 +152,32 @@ void CCoinsViewCache::SetBestBlock(const uint256 &hashBlockIn) { bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn) { assert(!hasModifier); for (CCoinsMap::iterator it = mapCoins.begin(); it != mapCoins.end();) { - cacheCoins[it->first].swap(it->second); + if (it->second.flags & CCoinsCacheEntry::DIRTY) { // Ignore non-dirty entries (optimization). + CCoinsMap::iterator itUs = cacheCoins.find(it->first); + if (itUs == cacheCoins.end()) { + if (!it->second.coins.IsPruned()) { + // The parent cache does not have an entry, while the child + // cache does have (a non-pruned) one. Move the data up, and + // mark it as fresh (if the grandparent did have it, we + // would have pulled it in at first GetCoins). + assert(it->second.flags & CCoinsCacheEntry::FRESH); + CCoinsCacheEntry& entry = cacheCoins[it->first]; + entry.coins.swap(it->second.coins); + entry.flags = CCoinsCacheEntry::DIRTY | CCoinsCacheEntry::FRESH; + } + } else { + if ((itUs->second.flags & CCoinsCacheEntry::FRESH) && it->second.coins.IsPruned()) { + // The grandparent does not have an entry, and the child is + // modified and being pruned. This means we can just delete + // it from the parent. + cacheCoins.erase(itUs); + } else { + // A normal modification. + itUs->second.coins.swap(it->second.coins); + itUs->second.flags |= CCoinsCacheEntry::DIRTY; + } + } + } CCoinsMap::iterator itOld = it++; mapCoins.erase(itOld); } @@ -212,8 +247,12 @@ double CCoinsViewCache::GetPriority(const CTransaction &tx, int nHeight) const CCoinsModifier::CCoinsModifier(CCoinsViewCache& cache_, CCoinsMap::iterator it_) : cache(cache_), it(it_) {} -CCoinsModifier::~CCoinsModifier() { +CCoinsModifier::~CCoinsModifier() +{ assert(cache.hasModifier); cache.hasModifier = false; - it->second.Cleanup(); + it->second.coins.Cleanup(); + if ((it->second.flags & CCoinsCacheEntry::FRESH) && it->second.coins.IsPruned()) { + cache.cacheCoins.erase(it); + } } -- cgit v1.2.3 From 7c70438dc67547e83953ba0343a071fae304ce65 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Wed, 24 Sep 2014 03:19:04 +0200 Subject: Get rid of the dummy CCoinsViewCache constructor arg --- src/coins.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/coins.cpp') diff --git a/src/coins.cpp b/src/coins.cpp index 9d60089bf5..6b7ebf6078 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -59,7 +59,7 @@ bool CCoinsView::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) { ret bool CCoinsView::GetStats(CCoinsStats &stats) const { return false; } -CCoinsViewBacked::CCoinsViewBacked(CCoinsView &viewIn) : base(&viewIn) { } +CCoinsViewBacked::CCoinsViewBacked(CCoinsView *viewIn) : base(viewIn) { } bool CCoinsViewBacked::GetCoins(const uint256 &txid, CCoins &coins) const { return base->GetCoins(txid, coins); } bool CCoinsViewBacked::HaveCoins(const uint256 &txid) const { return base->HaveCoins(txid); } uint256 CCoinsViewBacked::GetBestBlock() const { return base->GetBestBlock(); } @@ -69,7 +69,7 @@ bool CCoinsViewBacked::GetStats(CCoinsStats &stats) const { return base->GetStat CCoinsKeyHasher::CCoinsKeyHasher() : salt(GetRandHash()) {} -CCoinsViewCache::CCoinsViewCache(CCoinsView &baseIn, bool fDummy) : CCoinsViewBacked(baseIn), hasModifier(false), hashBlock(0) { } +CCoinsViewCache::CCoinsViewCache(CCoinsView *baseIn) : CCoinsViewBacked(baseIn), hasModifier(false), hashBlock(0) { } CCoinsViewCache::~CCoinsViewCache() { -- cgit v1.2.3