Call ReallocateCache() on each Flush()

martinus · martinus · commit 5e4ac5abf54f · 2023-03-23T19:38:38.000+01:00
This frees up all associated memory with the map, not only the nodes.
This is necessary in preparation for using the PoolAllocator for
CCoinsMap, which does not actually free any memory on clear().
diff --git a/src/coins.cpp b/src/coins.cpp
@@ -253,9 +253,12 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn
 
 bool CCoinsViewCache::Flush() {
     bool fOk = base->BatchWrite(cacheCoins, hashBlock, /*erase=*/true);
-    if (fOk && !cacheCoins.empty()) {
-        /* BatchWrite must erase all cacheCoins elements when erase=true. */
-        throw std::logic_error("Not all cached coins were erased");
+    if (fOk) {
+        if (!cacheCoins.empty()) {
+            /* BatchWrite must erase all cacheCoins elements when erase=true. */
+            throw std::logic_error("Not all cached coins were erased");
+        }
+        ReallocateCache();
     }
     cachedCoinsUsage = 0;
     return fOk;
diff --git a/src/test/validation_flush_tests.cpp b/src/test/validation_flush_tests.cpp
@@ -131,8 +131,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate)
             CoinsCacheSizeState::OK);
     }
 
-    // Flushing the view doesn't take us back to OK because cacheCoins has
-    // preallocated memory that doesn't get reclaimed even after flush.
+    // Flushing the view does take us back to OK because ReallocateCache() is called
 
     BOOST_CHECK_EQUAL(
         chainstate.GetCoinsCacheSizeState(MAX_COINS_CACHE_BYTES, 0),
@@ -144,7 +143,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate)
 
     BOOST_CHECK_EQUAL(
         chainstate.GetCoinsCacheSizeState(MAX_COINS_CACHE_BYTES, 0),
-        CoinsCacheSizeState::CRITICAL);
+        CoinsCacheSizeState::OK);
 }
 
 BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/validation.cpp b/src/validation.cpp
@@ -4930,7 +4930,6 @@ bool Chainstate::ResizeCoinsCaches(size_t coinstip_size, size_t coinsdb_size)
     } else {
         // Otherwise, flush state to disk and deallocate the in-memory coins map.
         ret = FlushStateToDisk(state, FlushStateMode::ALWAYS);
-        CoinsTip().ReallocateCache();
     }
     return ret;
 }

Original file line number	Diff line number	Diff line change
`@@ -131,8 +131,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate)`
`131`	`131`	`CoinsCacheSizeState::OK);`
`132`	`132`	`}`
`133`	`133`
`134`		`- // Flushing the view doesn't take us back to OK because cacheCoins has`
`135`		`- // preallocated memory that doesn't get reclaimed even after flush.`
	`134`	`+ // Flushing the view does take us back to OK because ReallocateCache() is called`
`136`	`135`
`137`	`136`	`BOOST_CHECK_EQUAL(`
`138`	`137`	`chainstate.GetCoinsCacheSizeState(MAX_COINS_CACHE_BYTES, 0),`
`@@ -144,7 +143,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate)`
`144`	`143`
`145`	`144`	`BOOST_CHECK_EQUAL(`
`146`	`145`	`chainstate.GetCoinsCacheSizeState(MAX_COINS_CACHE_BYTES, 0),`
`147`		`- CoinsCacheSizeState::CRITICAL);`
	`146`	`+ CoinsCacheSizeState::OK);`
`148`	`147`	`}`
`149`	`148`
`150`	`149`	`BOOST_AUTO_TEST_SUITE_END()`
Original file line number	Diff line number	Diff line change
`@@ -4930,7 +4930,6 @@ bool Chainstate::ResizeCoinsCaches(size_t coinstip_size, size_t coinsdb_size)`
`4930`	`4930`	`} else {`
`4931`	`4931`	`// Otherwise, flush state to disk and deallocate the in-memory coins map.`
`4932`	`4932`	`ret = FlushStateToDisk(state, FlushStateMode::ALWAYS);`
`4933`		`- CoinsTip().ReallocateCache();`
`4934`	`4933`	`}`
`4935`	`4934`	`return ret;`
`4936`	`4935`	`}`