Merge #19085: Refactor: clean up PeriodicFlush()

e846a2a1d9e8aa37abfa55bd4d2a0a9f55ba6c8e refactor: clean up PeriodicFlush() (John Newbery)

Pull request description:

  `PeriodicFlush()` is much more convoluted than it needs to be: it has triple nesting, local variables counting refs and return values, and increments the `mapFileUseCount` iterator unnecessarily. Removing all of that makes the function much easier to understand.

ACKs for top commit:
  MarcoFalke:
    ACK e846a2a1d9e8aa37abfa55bd4d2a0a9f55ba6c8e 🎁
  jonatack:
    re-ACK e846a2a per `git range-diff f7c19e8 7c10020 e846a2a`
  promag:
    ACK e846a2a1d9e8aa37abfa55bd4d2a0a9f55ba6c8e.

Tree-SHA512: 22bc600a5268b139c0a2c16b5a9f14837b262670ec24aef00643fcedd1c3ebcbf46dea1633e76adc8acf78e8840b776e17127307c5ee95308caa94239dad5b88

1 files changed, 22 insertions, 31 deletions

diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp
index fa4a505982..5f823d5906 100644
--- a/src/wallet/bdb.cpp
+++ b/src/wallet/bdb.cpp
@@ -615,42 +615,33 @@ void BerkeleyEnvironment::Flush(bool fShutdown)
 
 bool BerkeleyDatabase::PeriodicFlush()
 {
-    if (IsDummy()) {
-        return true;
-    }
-    bool ret = false;
+    // There's nothing to do for dummy databases. Return true.
+    if (IsDummy()) return true;
+
+    // Don't flush if we can't acquire the lock.
     TRY_LOCK(cs_db, lockDb);
-    if (lockDb)
-    {
-        // Don't do this if any databases are in use
-        int nRefCount = 0;
-        std::map<std::string, int>::iterator mit = env->mapFileUseCount.begin();
-        while (mit != env->mapFileUseCount.end())
-        {
-            nRefCount += (*mit).second;
-            mit++;
-        }
+    if (!lockDb) return false;
 
-        if (nRefCount == 0)
-        {
-            std::map<std::string, int>::iterator mi = env->mapFileUseCount.find(strFile);
-            if (mi != env->mapFileUseCount.end())
-            {
-                LogPrint(BCLog::WALLETDB, "Flushing %s\n", strFile);
-                int64_t nStart = GetTimeMillis();
+    // Don't flush if any databases are in use
+    for (auto it = env->mapFileUseCount.begin() ; it != env->mapFileUseCount.end(); it++) {
+        if ((*it).second > 0) return false;
+    }
 
-                // Flush wallet file so it's self contained
-                env->CloseDb(strFile);
-                env->CheckpointLSN(strFile);
+    // Don't flush if there haven't been any batch writes for this database.
+    auto it = env->mapFileUseCount.find(strFile);
+    if (it == env->mapFileUseCount.end()) return false;
 
-                env->mapFileUseCount.erase(mi++);
-                LogPrint(BCLog::WALLETDB, "Flushed %s %dms\n", strFile, GetTimeMillis() - nStart);
-                ret = true;
-            }
-        }
-    }
+    LogPrint(BCLog::WALLETDB, "Flushing %s\n", strFile);
+    int64_t nStart = GetTimeMillis();
+
+    // Flush wallet file so it's self contained
+    env->CloseDb(strFile);
+    env->CheckpointLSN(strFile);
+    env->mapFileUseCount.erase(it);
 
-    return ret;
+    LogPrint(BCLog::WALLETDB, "Flushed %s %dms\n", strFile, GetTimeMillis() - nStart);
+
+    return true;
 }
 
 bool BerkeleyDatabase::Backup(const std::string& strDest) const