diff options
author | MarcoFalke <falke.marco@gmail.com> | 2020-07-30 17:30:44 +0200 |
---|---|---|
committer | MarcoFalke <falke.marco@gmail.com> | 2020-07-30 17:30:52 +0200 |
commit | ad2952d17a2af419a04256b10b53c7377f826a27 (patch) | |
tree | 7aefc283a3f710c626ef0eddab02989ce947ac35 /src | |
parent | 62d137ac3b701aae36c1aa3aa93a83fd6357fde6 (diff) | |
parent | fae8c28dae747f9c4c6481049742346d18202fc8 (diff) | |
download | bitcoin-ad2952d17a2af419a04256b10b53c7377f826a27.tar.xz |
Merge #19604: Pass mempool pointer to UnloadBlockIndex/GetCoinsCacheSizeState
fae8c28dae747f9c4c6481049742346d18202fc8 Pass mempool pointer to GetCoinsCacheSizeState (MarcoFalke)
fac674db200e6b2d5b32069335fb24e713d7b69f Pass mempool pointer to UnloadBlockIndex (MarcoFalke)
faec851b6eb7e65e28cdcae50d6dc86fafa0f91c test: Simplify cs_main locks (MarcoFalke)
Pull request description:
Split out from #19556
Instead of relying on the implicit mempool global, pass a mempool pointer (which can be `0`). This helps with testing, code clarity and unlocks the features described in #19556.
ACKs for top commit:
jnewbery:
code review ACK fae8c28dae747f9c4c6481049742346d18202fc8
fjahr:
Code review ACK fae8c28dae747f9c4c6481049742346d18202fc8
darosior:
Tested ACK fae8c28dae747f9c4c6481049742346d18202fc8
jamesob:
ACK fae8c28dae747f9c4c6481049742346d18202fc8 ([`jamesob/ackr/19604.1.MarcoFalke.pass_mempool_pointer_to`](https://github.com/jamesob/bitcoin/tree/ackr/19604.1.MarcoFalke.pass_mempool_pointer_to))
Tree-SHA512: fa687518c8cda4a095bdbdfe56e01fae2fb16c13d51efbb1312cd6dc007611fc47f53f475602e4a843e3973c9410e6af5a81d6847bd2399f8262ca7205975728
Diffstat (limited to 'src')
-rw-r--r-- | src/init.cpp | 2 | ||||
-rw-r--r-- | src/qt/test/apptests.cpp | 2 | ||||
-rw-r--r-- | src/test/util/setup_common.cpp | 2 | ||||
-rw-r--r-- | src/test/validation_chainstate_tests.cpp | 4 | ||||
-rw-r--r-- | src/test/validation_chainstatemanager_tests.cpp | 13 | ||||
-rw-r--r-- | src/test/validation_flush_tests.cpp | 22 | ||||
-rw-r--r-- | src/validation.cpp | 12 | ||||
-rw-r--r-- | src/validation.h | 6 |
8 files changed, 27 insertions, 36 deletions
diff --git a/src/init.cpp b/src/init.cpp index ad500702d7..6cca21f375 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1567,7 +1567,7 @@ bool AppInitMain(const util::Ref& context, NodeContext& node) chainman.m_total_coinstip_cache = nCoinCacheUsage; chainman.m_total_coinsdb_cache = nCoinDBCache; - UnloadBlockIndex(); + UnloadBlockIndex(node.mempool); // new CBlockTreeDB tries to delete the existing file, which // fails if it's still open from the previous loop. Close it first: diff --git a/src/qt/test/apptests.cpp b/src/qt/test/apptests.cpp index 443e2d047d..b880a99baf 100644 --- a/src/qt/test/apptests.cpp +++ b/src/qt/test/apptests.cpp @@ -83,7 +83,7 @@ void AppTests::appTests() // Reset global state to avoid interfering with later tests. LogInstance().DisconnectTestLogger(); AbortShutdown(); - UnloadBlockIndex(); + UnloadBlockIndex(/* mempool */ nullptr); WITH_LOCK(::cs_main, g_chainman.Reset()); } diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 0cd991f453..14f65dcb7c 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -182,9 +182,9 @@ TestingSetup::~TestingSetup() m_node.connman.reset(); m_node.banman.reset(); m_node.args = nullptr; + UnloadBlockIndex(m_node.mempool); m_node.mempool = nullptr; m_node.scheduler.reset(); - UnloadBlockIndex(); m_node.chainman->Reset(); m_node.chainman = nullptr; pblocktree.reset(); diff --git a/src/test/validation_chainstate_tests.cpp b/src/test/validation_chainstate_tests.cpp index f531b393b8..2076a1096a 100644 --- a/src/test/validation_chainstate_tests.cpp +++ b/src/test/validation_chainstate_tests.cpp @@ -34,9 +34,7 @@ BOOST_AUTO_TEST_CASE(validation_chainstate_resize_caches) return outp; }; - ENTER_CRITICAL_SECTION(cs_main); - CChainState& c1 = manager.InitializeChainstate(); - LEAVE_CRITICAL_SECTION(cs_main); + CChainState& c1 = *WITH_LOCK(cs_main, return &manager.InitializeChainstate()); c1.InitCoinsDB( /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false); WITH_LOCK(::cs_main, c1.InitCoinsCache(1 << 23)); diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index f99191b08f..887a48124f 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -54,10 +54,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager) // Create a snapshot-based chainstate. // - ENTER_CRITICAL_SECTION(cs_main); - CChainState& c2 = *WITH_LOCK(::cs_main, - return &manager.InitializeChainstate(GetRandHash())); - LEAVE_CRITICAL_SECTION(cs_main); + CChainState& c2 = *WITH_LOCK(::cs_main, return &manager.InitializeChainstate(GetRandHash())); chainstates.push_back(&c2); c2.InitCoinsDB( /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false); @@ -115,9 +112,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager_rebalance_caches) // Create a legacy (IBD) chainstate. // - ENTER_CRITICAL_SECTION(cs_main); - CChainState& c1 = manager.InitializeChainstate(); - LEAVE_CRITICAL_SECTION(cs_main); + CChainState& c1 = *WITH_LOCK(cs_main, return &manager.InitializeChainstate()); chainstates.push_back(&c1); c1.InitCoinsDB( /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false); @@ -134,9 +129,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager_rebalance_caches) // Create a snapshot-based chainstate. // - ENTER_CRITICAL_SECTION(cs_main); - CChainState& c2 = manager.InitializeChainstate(GetRandHash()); - LEAVE_CRITICAL_SECTION(cs_main); + CChainState& c2 = *WITH_LOCK(cs_main, return &manager.InitializeChainstate(GetRandHash())); chainstates.push_back(&c2); c2.InitCoinsDB( /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false); diff --git a/src/test/validation_flush_tests.cpp b/src/test/validation_flush_tests.cpp index 7111fc3380..8bac914f05 100644 --- a/src/test/validation_flush_tests.cpp +++ b/src/test/validation_flush_tests.cpp @@ -56,7 +56,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate) // Without any coins in the cache, we shouldn't need to flush. BOOST_CHECK_EQUAL( - chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0), + chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0), CoinsCacheSizeState::OK); // If the initial memory allocations of cacheCoins don't match these common @@ -71,7 +71,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate) } BOOST_CHECK_EQUAL( - chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0), + chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0), CoinsCacheSizeState::CRITICAL); BOOST_TEST_MESSAGE("Exiting cache flush tests early due to unsupported arch"); @@ -92,7 +92,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate) print_view_mem_usage(view); BOOST_CHECK_EQUAL(view.AccessCoin(res).DynamicMemoryUsage(), COIN_SIZE); BOOST_CHECK_EQUAL( - chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0), + chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0), CoinsCacheSizeState::OK); } @@ -100,26 +100,26 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate) for (int i{0}; i < 4; ++i) { add_coin(view); print_view_mem_usage(view); - if (chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0) == + if (chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0) == CoinsCacheSizeState::CRITICAL) { break; } } BOOST_CHECK_EQUAL( - chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0), + chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 0), CoinsCacheSizeState::CRITICAL); // Passing non-zero max mempool usage should allow us more headroom. BOOST_CHECK_EQUAL( - chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 1 << 10), + chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 1 << 10), CoinsCacheSizeState::OK); for (int i{0}; i < 3; ++i) { add_coin(view); print_view_mem_usage(view); BOOST_CHECK_EQUAL( - chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 1 << 10), + chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, /*max_mempool_size_bytes*/ 1 << 10), CoinsCacheSizeState::OK); } @@ -135,7 +135,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate) BOOST_CHECK(usage_percentage >= 0.9); BOOST_CHECK(usage_percentage < 1); BOOST_CHECK_EQUAL( - chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, 1 << 10), + chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, 1 << 10), CoinsCacheSizeState::LARGE); } @@ -143,7 +143,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate) for (int i{0}; i < 1000; ++i) { add_coin(view); BOOST_CHECK_EQUAL( - chainstate.GetCoinsCacheSizeState(tx_pool), + chainstate.GetCoinsCacheSizeState(&tx_pool), CoinsCacheSizeState::OK); } @@ -151,7 +151,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate) // preallocated memory that doesn't get reclaimed even after flush. BOOST_CHECK_EQUAL( - chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, 0), + chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, 0), CoinsCacheSizeState::CRITICAL); view.SetBestBlock(InsecureRand256()); @@ -159,7 +159,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate) print_view_mem_usage(view); BOOST_CHECK_EQUAL( - chainstate.GetCoinsCacheSizeState(tx_pool, MAX_COINS_CACHE_BYTES, 0), + chainstate.GetCoinsCacheSizeState(&tx_pool, MAX_COINS_CACHE_BYTES, 0), CoinsCacheSizeState::CRITICAL); } diff --git a/src/validation.cpp b/src/validation.cpp index 545aa0f2d3..3cebf1090e 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2227,7 +2227,7 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, return true; } -CoinsCacheSizeState CChainState::GetCoinsCacheSizeState(const CTxMemPool& tx_pool) +CoinsCacheSizeState CChainState::GetCoinsCacheSizeState(const CTxMemPool* tx_pool) { return this->GetCoinsCacheSizeState( tx_pool, @@ -2236,11 +2236,11 @@ CoinsCacheSizeState CChainState::GetCoinsCacheSizeState(const CTxMemPool& tx_poo } CoinsCacheSizeState CChainState::GetCoinsCacheSizeState( - const CTxMemPool& tx_pool, + const CTxMemPool* tx_pool, size_t max_coins_cache_size_bytes, size_t max_mempool_size_bytes) { - int64_t nMempoolUsage = tx_pool.DynamicMemoryUsage(); + const int64_t nMempoolUsage = tx_pool ? tx_pool->DynamicMemoryUsage() : 0; int64_t cacheSize = CoinsTip().DynamicMemoryUsage(); int64_t nTotalSpace = max_coins_cache_size_bytes + std::max<int64_t>(max_mempool_size_bytes - nMempoolUsage, 0); @@ -2279,7 +2279,7 @@ bool CChainState::FlushStateToDisk( { bool fFlushForPrune = false; bool fDoFullFlush = false; - CoinsCacheSizeState cache_state = GetCoinsCacheSizeState(::mempool); + CoinsCacheSizeState cache_state = GetCoinsCacheSizeState(&::mempool); LOCK(cs_LastBlockFile); if (fPruneMode && (fCheckForPruning || nManualPruneHeight > 0) && !fReindex) { if (nManualPruneHeight > 0) { @@ -4587,13 +4587,13 @@ void CChainState::UnloadBlockIndex() { // May NOT be used after any connections are up as much // of the peer-processing logic assumes a consistent // block index state -void UnloadBlockIndex() +void UnloadBlockIndex(CTxMemPool* mempool) { LOCK(cs_main); g_chainman.Unload(); pindexBestInvalid = nullptr; pindexBestHeader = nullptr; - mempool.clear(); + if (mempool) mempool->clear(); vinfoBlockFile.clear(); nLastBlockFile = 0; setDirtyBlockIndex.clear(); diff --git a/src/validation.h b/src/validation.h index 53503768aa..534162d64a 100644 --- a/src/validation.h +++ b/src/validation.h @@ -160,7 +160,7 @@ void LoadExternalBlockFile(const CChainParams& chainparams, FILE* fileIn, FlatFi /** Ensures we have a genesis block in the block tree, possibly writing one to disk. */ bool LoadGenesisBlock(const CChainParams& chainparams); /** Unload database information */ -void UnloadBlockIndex(); +void UnloadBlockIndex(CTxMemPool* mempool); /** Run an instance of the script checking thread */ void ThreadScriptCheck(int worker_num); /** @@ -674,11 +674,11 @@ public: //! Dictates whether we need to flush the cache to disk or not. //! //! @return the state of the size of the coins cache. - CoinsCacheSizeState GetCoinsCacheSizeState(const CTxMemPool& tx_pool) + CoinsCacheSizeState GetCoinsCacheSizeState(const CTxMemPool* tx_pool) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); CoinsCacheSizeState GetCoinsCacheSizeState( - const CTxMemPool& tx_pool, + const CTxMemPool* tx_pool, size_t max_coins_cache_size_bytes, size_t max_mempool_size_bytes) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); |