diff options
author | James O'Beirne <james.obeirne@gmail.com> | 2019-12-03 13:25:35 -0500 |
---|---|---|
committer | James O'Beirne <james.obeirne@pm.me> | 2023-01-20 10:36:48 -0500 |
commit | 79cedc36afe2e72e42839d861734d73d545d21b8 (patch) | |
tree | b828b92dababbe42ced74b7ec7ff6484a995ccd7 | |
parent | c0b6c40bb084752f686ff77e6883ee6fd16eba26 (diff) | |
download | bitcoin-79cedc36afe2e72e42839d861734d73d545d21b8.tar.xz |
coins: add Sync() method to allow flush without cacheCoins drop
In certain circumstances, we may want to flush to disk without
emptying `cacheCoins`, which affects performance. UTXO snapshot
activation is one such case.
This method is currently unused and this commit does not
change any behavior.
Incorporates feedback from John Newbery.
Co-authored-by: Suhas Daftuar <sdaftuar@gmail.com>
-rw-r--r-- | src/coins.cpp | 47 | ||||
-rw-r--r-- | src/coins.h | 20 | ||||
-rw-r--r-- | src/test/coins_tests.cpp | 2 | ||||
-rw-r--r-- | src/txdb.cpp | 5 | ||||
-rw-r--r-- | src/txdb.h | 2 |
5 files changed, 59 insertions, 17 deletions
diff --git a/src/coins.cpp b/src/coins.cpp index 976118e23c..ecbd625cd7 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -13,7 +13,7 @@ bool CCoinsView::GetCoin(const COutPoint &outpoint, Coin &coin) const { return false; } uint256 CCoinsView::GetBestBlock() const { return uint256(); } std::vector<uint256> CCoinsView::GetHeadBlocks() const { return std::vector<uint256>(); } -bool CCoinsView::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) { return false; } +bool CCoinsView::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock, bool erase) { return false; } std::unique_ptr<CCoinsViewCursor> CCoinsView::Cursor() const { return nullptr; } bool CCoinsView::HaveCoin(const COutPoint &outpoint) const @@ -28,7 +28,7 @@ bool CCoinsViewBacked::HaveCoin(const COutPoint &outpoint) const { return base-> uint256 CCoinsViewBacked::GetBestBlock() const { return base->GetBestBlock(); } std::vector<uint256> CCoinsViewBacked::GetHeadBlocks() const { return base->GetHeadBlocks(); } void CCoinsViewBacked::SetBackend(CCoinsView &viewIn) { base = &viewIn; } -bool CCoinsViewBacked::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) { return base->BatchWrite(mapCoins, hashBlock); } +bool CCoinsViewBacked::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock, bool erase) { return base->BatchWrite(mapCoins, hashBlock, erase); } std::unique_ptr<CCoinsViewCursor> CCoinsViewBacked::Cursor() const { return base->Cursor(); } size_t CCoinsViewBacked::EstimateSize() const { return base->EstimateSize(); } @@ -176,8 +176,10 @@ void CCoinsViewCache::SetBestBlock(const uint256 &hashBlockIn) { hashBlock = hashBlockIn; } -bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn) { - for (CCoinsMap::iterator it = mapCoins.begin(); it != mapCoins.end(); it = mapCoins.erase(it)) { +bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn, bool erase) { + for (CCoinsMap::iterator it = mapCoins.begin(); + it != mapCoins.end(); + it = erase ? mapCoins.erase(it) : std::next(it)) { // Ignore non-dirty entries (optimization). if (!(it->second.flags & CCoinsCacheEntry::DIRTY)) { continue; @@ -190,7 +192,14 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn // Create the coin in the parent cache, move the data up // and mark it as dirty. CCoinsCacheEntry& entry = cacheCoins[it->first]; - entry.coin = std::move(it->second.coin); + if (erase) { + // The `move` call here is purely an optimization; we rely on the + // `mapCoins.erase` call in the `for` expression to actually remove + // the entry from the child map. + entry.coin = std::move(it->second.coin); + } else { + entry.coin = it->second.coin; + } cachedCoinsUsage += entry.coin.DynamicMemoryUsage(); entry.flags = CCoinsCacheEntry::DIRTY; // We can mark it FRESH in the parent if it was FRESH in the child @@ -218,7 +227,14 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn } else { // A normal modification. cachedCoinsUsage -= itUs->second.coin.DynamicMemoryUsage(); - itUs->second.coin = std::move(it->second.coin); + if (erase) { + // The `move` call here is purely an optimization; we rely on the + // `mapCoins.erase` call in the `for` expression to actually remove + // the entry from the child map. + itUs->second.coin = std::move(it->second.coin); + } else { + itUs->second.coin = it->second.coin; + } cachedCoinsUsage += itUs->second.coin.DynamicMemoryUsage(); itUs->second.flags |= CCoinsCacheEntry::DIRTY; // NOTE: It isn't safe to mark the coin as FRESH in the parent @@ -233,12 +249,29 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn } bool CCoinsViewCache::Flush() { - bool fOk = base->BatchWrite(cacheCoins, hashBlock); + bool fOk = base->BatchWrite(cacheCoins, hashBlock, /*erase=*/ true); cacheCoins.clear(); cachedCoinsUsage = 0; return fOk; } +bool CCoinsViewCache::Sync() +{ + bool fOk = base->BatchWrite(cacheCoins, hashBlock, /*erase=*/ false); + // Instead of clearing `cacheCoins` as we would in Flush(), just clear the + // FRESH/DIRTY flags of any coin that isn't spent. + for (auto it = cacheCoins.begin(); it != cacheCoins.end(); ) { + if (it->second.coin.IsSpent()) { + cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage(); + it = cacheCoins.erase(it); + } else { + it->second.flags = 0; + ++it; + } + } + return fOk; +} + void CCoinsViewCache::Uncache(const COutPoint& hash) { CCoinsMap::iterator it = cacheCoins.find(hash); diff --git a/src/coins.h b/src/coins.h index b0d6bdf333..4edc146a14 100644 --- a/src/coins.h +++ b/src/coins.h @@ -176,7 +176,7 @@ public: //! Do a bulk modification (multiple Coin changes + BestBlock change). //! The passed mapCoins can be modified. - virtual bool BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock); + virtual bool BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock, bool erase = true); //! Get a cursor to iterate over the whole state virtual std::unique_ptr<CCoinsViewCursor> Cursor() const; @@ -202,7 +202,7 @@ public: uint256 GetBestBlock() const override; std::vector<uint256> GetHeadBlocks() const override; void SetBackend(CCoinsView &viewIn); - bool BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) override; + bool BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock, bool erase = true) override; std::unique_ptr<CCoinsViewCursor> Cursor() const override; size_t EstimateSize() const override; }; @@ -235,7 +235,7 @@ public: bool HaveCoin(const COutPoint &outpoint) const override; uint256 GetBestBlock() const override; void SetBestBlock(const uint256 &hashBlock); - bool BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) override; + bool BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock, bool erase = true) override; std::unique_ptr<CCoinsViewCursor> Cursor() const override { throw std::logic_error("CCoinsViewCache cursor iteration not supported."); } @@ -282,13 +282,23 @@ public: bool SpendCoin(const COutPoint &outpoint, Coin* moveto = nullptr); /** - * Push the modifications applied to this cache to its base. - * Failure to call this method before destruction will cause the changes to be forgotten. + * Push the modifications applied to this cache to its base and wipe local state. + * Failure to call this method or Sync() before destruction will cause the changes + * to be forgotten. * If false is returned, the state of this cache (and its backing view) will be undefined. */ bool Flush(); /** + * Push the modifications applied to this cache to its base while retaining + * the contents of this cache (except for spent coins, which we erase). + * Failure to call this method or Flush() before destruction will cause the changes + * to be forgotten. + * If false is returned, the state of this cache (and its backing view) will be undefined. + */ + bool Sync(); + + /** * Removes the UTXO with the given outpoint from the cache, if it is * not modified. */ diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index b5f961a239..d55dea4f83 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -53,7 +53,7 @@ public: uint256 GetBestBlock() const override { return hashBestBlock_; } - bool BatchWrite(CCoinsMap& mapCoins, const uint256& hashBlock) override + bool BatchWrite(CCoinsMap& mapCoins, const uint256& hashBlock, bool erase = true) override { for (CCoinsMap::iterator it = mapCoins.begin(); it != mapCoins.end(); ) { if (it->second.flags & CCoinsCacheEntry::DIRTY) { diff --git a/src/txdb.cpp b/src/txdb.cpp index f04a4e9800..c12b540b9b 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -111,7 +111,7 @@ std::vector<uint256> CCoinsViewDB::GetHeadBlocks() const { return vhashHeadBlocks; } -bool CCoinsViewDB::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) { +bool CCoinsViewDB::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock, bool erase) { CDBBatch batch(*m_db); size_t count = 0; size_t changed = 0; @@ -146,8 +146,7 @@ bool CCoinsViewDB::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) { changed++; } count++; - CCoinsMap::iterator itOld = it++; - mapCoins.erase(itOld); + it = erase ? mapCoins.erase(it) : std::next(it); if (batch.SizeEstimate() > batch_size) { LogPrint(BCLog::COINDB, "Writing partial batch of %.2f MiB\n", batch.SizeEstimate() * (1.0 / 1048576.0)); m_db->WriteBatch(batch); diff --git a/src/txdb.h b/src/txdb.h index 5a409d7dcc..e3422846c0 100644 --- a/src/txdb.h +++ b/src/txdb.h @@ -62,7 +62,7 @@ public: bool HaveCoin(const COutPoint &outpoint) const override; uint256 GetBestBlock() const override; std::vector<uint256> GetHeadBlocks() const override; - bool BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) override; + bool BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock, bool erase = true) override; std::unique_ptr<CCoinsViewCursor> Cursor() const override; //! Whether an unsupported database format is used. |