From fa576b4532814b4bca1936d170cabd01fbc51960 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 25 Jan 2021 12:22:03 +0100 Subject: [PATCH] Move MakeNoLogFileContext to common libtest_util, and use it in bench Can be reviewed with --color-moved=dimmed-zebra --- src/bench/block_assemble.cpp | 14 ++++---------- src/bench/duplicate_inputs.cpp | 8 +------- src/bench/mempool_eviction.cpp | 8 +------- src/bench/mempool_stress.cpp | 2 +- src/bench/rpc_blockchain.cpp | 2 +- src/bench/wallet_balance.cpp | 16 +++++----------- src/test/fuzz/banman.cpp | 1 + src/test/fuzz/coins_view.cpp | 1 + src/test/fuzz/connman.cpp | 1 + src/test/fuzz/data_stream.cpp | 1 + src/test/fuzz/policy_estimator.cpp | 1 + src/test/fuzz/policy_estimator_io.cpp | 1 + src/test/fuzz/signet.cpp | 1 + src/test/fuzz/util.h | 15 --------------- src/test/util/setup_common.h | 18 ++++++++++++++++++ 15 files changed, 38 insertions(+), 52 deletions(-) diff --git a/src/bench/block_assemble.cpp b/src/bench/block_assemble.cpp index 8f656c44d9e..67ab02a5b33 100644 --- a/src/bench/block_assemble.cpp +++ b/src/bench/block_assemble.cpp @@ -16,13 +16,7 @@ static void AssembleBlock(benchmark::Bench& bench) { - TestingSetup test_setup{ - CBaseChainParams::REGTEST, - /* extra_args */ { - "-nodebuglogfile", - "-nodebug", - }, - }; + const auto test_setup = MakeNoLogFileContext(); const std::vector op_true{OP_TRUE}; CScriptWitness witness; @@ -38,7 +32,7 @@ static void AssembleBlock(benchmark::Bench& bench) std::array txs; for (size_t b{0}; b < NUM_BLOCKS; ++b) { CMutableTransaction tx; - tx.vin.push_back(MineBlock(test_setup.m_node, SCRIPT_PUB)); + tx.vin.push_back(MineBlock(test_setup->m_node, SCRIPT_PUB)); tx.vin.back().scriptWitness = witness; tx.vout.emplace_back(1337, SCRIPT_PUB); if (NUM_BLOCKS - b >= COINBASE_MATURITY) @@ -48,13 +42,13 @@ static void AssembleBlock(benchmark::Bench& bench) LOCK(::cs_main); // Required for ::AcceptToMemoryPool. for (const auto& txr : txs) { - const MempoolAcceptResult res = ::AcceptToMemoryPool(::ChainstateActive(), *test_setup.m_node.mempool, txr, false /* bypass_limits */); + const MempoolAcceptResult res = ::AcceptToMemoryPool(::ChainstateActive(), *test_setup->m_node.mempool, txr, false /* bypass_limits */); assert(res.m_result_type == MempoolAcceptResult::ResultType::VALID); } } bench.run([&] { - PrepareBlock(test_setup.m_node, SCRIPT_PUB); + PrepareBlock(test_setup->m_node, SCRIPT_PUB); }); } diff --git a/src/bench/duplicate_inputs.cpp b/src/bench/duplicate_inputs.cpp index 5745e4276c5..25d1a2b56c4 100644 --- a/src/bench/duplicate_inputs.cpp +++ b/src/bench/duplicate_inputs.cpp @@ -14,13 +14,7 @@ static void DuplicateInputs(benchmark::Bench& bench) { - TestingSetup test_setup{ - CBaseChainParams::REGTEST, - /* extra_args */ { - "-nodebuglogfile", - "-nodebug", - }, - }; + const auto testing_setup = MakeNoLogFileContext(); const CScript SCRIPT_PUB{CScript(OP_TRUE)}; diff --git a/src/bench/mempool_eviction.cpp b/src/bench/mempool_eviction.cpp index db9a5661fde..4f49fba7b75 100644 --- a/src/bench/mempool_eviction.cpp +++ b/src/bench/mempool_eviction.cpp @@ -25,13 +25,7 @@ static void AddTx(const CTransactionRef& tx, const CAmount& nFee, CTxMemPool& po // unique transactions for a more meaningful performance measurement. static void MempoolEviction(benchmark::Bench& bench) { - TestingSetup test_setup{ - CBaseChainParams::REGTEST, - /* extra_args */ { - "-nodebuglogfile", - "-nodebug", - }, - }; + const auto testing_setup = MakeNoLogFileContext(); CMutableTransaction tx1 = CMutableTransaction(); tx1.vin.resize(1); diff --git a/src/bench/mempool_stress.cpp b/src/bench/mempool_stress.cpp index 9b862b735c1..f28768efc85 100644 --- a/src/bench/mempool_stress.cpp +++ b/src/bench/mempool_stress.cpp @@ -79,7 +79,7 @@ static void ComplexMemPool(benchmark::Bench& bench) ordered_coins.emplace_back(MakeTransactionRef(tx)); available_coins.emplace_back(ordered_coins.back(), tx_counter++); } - TestingSetup test_setup; + const auto testing_setup = MakeNoLogFileContext(CBaseChainParams::MAIN); CTxMemPool pool; LOCK2(cs_main, pool.cs); bench.run([&]() NO_THREAD_SAFETY_ANALYSIS { diff --git a/src/bench/rpc_blockchain.cpp b/src/bench/rpc_blockchain.cpp index 78f8c6e6dc5..c8886a4c237 100644 --- a/src/bench/rpc_blockchain.cpp +++ b/src/bench/rpc_blockchain.cpp @@ -15,7 +15,7 @@ namespace { struct TestBlockAndIndex { - TestingSetup test_setup{}; + const std::unique_ptr testing_setup{MakeNoLogFileContext(CBaseChainParams::MAIN)}; CBlock block{}; uint256 blockHash{}; CBlockIndex blockindex{}; diff --git a/src/bench/wallet_balance.cpp b/src/bench/wallet_balance.cpp index b385cec085f..c96ef209e36 100644 --- a/src/bench/wallet_balance.cpp +++ b/src/bench/wallet_balance.cpp @@ -14,30 +14,24 @@ static void WalletBalance(benchmark::Bench& bench, const bool set_dirty, const bool add_watchonly, const bool add_mine) { - TestingSetup test_setup{ - CBaseChainParams::REGTEST, - /* extra_args */ { - "-nodebuglogfile", - "-nodebug", - }, - }; + const auto test_setup = MakeNoLogFileContext(); const auto& ADDRESS_WATCHONLY = ADDRESS_BCRT1_UNSPENDABLE; - CWallet wallet{test_setup.m_node.chain.get(), "", CreateMockWalletDatabase()}; + CWallet wallet{test_setup->m_node.chain.get(), "", CreateMockWalletDatabase()}; { wallet.SetupLegacyScriptPubKeyMan(); bool first_run; if (wallet.LoadWallet(first_run) != DBErrors::LOAD_OK) assert(false); } - auto handler = test_setup.m_node.chain->handleNotifications({&wallet, [](CWallet*) {}}); + auto handler = test_setup->m_node.chain->handleNotifications({&wallet, [](CWallet*) {}}); const Optional address_mine{add_mine ? Optional{getnewaddress(wallet)} : nullopt}; if (add_watchonly) importaddress(wallet, ADDRESS_WATCHONLY); for (int i = 0; i < 100; ++i) { - generatetoaddress(test_setup.m_node, address_mine.value_or(ADDRESS_WATCHONLY)); - generatetoaddress(test_setup.m_node, ADDRESS_WATCHONLY); + generatetoaddress(test_setup->m_node, address_mine.value_or(ADDRESS_WATCHONLY)); + generatetoaddress(test_setup->m_node, ADDRESS_WATCHONLY); } SyncWithValidationInterfaceQueue(); diff --git a/src/test/fuzz/banman.cpp b/src/test/fuzz/banman.cpp index 124439559eb..8bf484722c0 100644 --- a/src/test/fuzz/banman.cpp +++ b/src/test/fuzz/banman.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp index 19486365f62..328a31f1dce 100644 --- a/src/test/fuzz/coins_view.cpp +++ b/src/test/fuzz/coins_view.cpp @@ -16,6 +16,7 @@ #include #include #include +#include #include #include diff --git a/src/test/fuzz/connman.cpp b/src/test/fuzz/connman.cpp index 3fb8d8ca06d..f457201091e 100644 --- a/src/test/fuzz/connman.cpp +++ b/src/test/fuzz/connman.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include diff --git a/src/test/fuzz/data_stream.cpp b/src/test/fuzz/data_stream.cpp index d6ef0c6691c..473caec6ffa 100644 --- a/src/test/fuzz/data_stream.cpp +++ b/src/test/fuzz/data_stream.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include diff --git a/src/test/fuzz/policy_estimator.cpp b/src/test/fuzz/policy_estimator.cpp index 311550f0413..116b7a71d93 100644 --- a/src/test/fuzz/policy_estimator.cpp +++ b/src/test/fuzz/policy_estimator.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include diff --git a/src/test/fuzz/policy_estimator_io.cpp b/src/test/fuzz/policy_estimator_io.cpp index c24ad3d49a7..9021d95954f 100644 --- a/src/test/fuzz/policy_estimator_io.cpp +++ b/src/test/fuzz/policy_estimator_io.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include diff --git a/src/test/fuzz/signet.cpp b/src/test/fuzz/signet.cpp index 0d1999731f7..303dcf13e39 100644 --- a/src/test/fuzz/signet.cpp +++ b/src/test/fuzz/signet.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h index f3bc3c78abc..5065cde06bc 100644 --- a/src/test/fuzz/util.h +++ b/src/test/fuzz/util.h @@ -23,11 +23,9 @@ #include #include #include -#include #include #include #include -#include #include #include @@ -324,19 +322,6 @@ inline std::unique_ptr ConsumeNodeAsUniquePtr(FuzzedDataProvider& fdp, co void FillNode(FuzzedDataProvider& fuzzed_data_provider, CNode& node, bool init_version) noexcept; -template -std::unique_ptr MakeNoLogFileContext(const std::string& chain_name = CBaseChainParams::REGTEST, const std::vector& extra_args = {}) -{ - // Prepend default arguments for fuzzing - const std::vector arguments = Cat( - { - "-nodebuglogfile", - }, - extra_args); - - return MakeUnique(chain_name, arguments); -} - class FuzzedFileProvider { FuzzedDataProvider& m_fuzzed_data_provider; diff --git a/src/test/util/setup_common.h b/src/test/util/setup_common.h index 33f24e7c448..7323f1f0b6f 100644 --- a/src/test/util/setup_common.h +++ b/src/test/util/setup_common.h @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -152,6 +153,23 @@ struct TestChain100DeterministicSetup : public TestChain100Setup { TestChain100DeterministicSetup() : TestChain100Setup(true) { } }; +/** + * Make a test setup that has disk access to the debug.log file disabled. Can + * be used in "hot loops", for example fuzzing or benchmarking. + */ +template +std::unique_ptr MakeNoLogFileContext(const std::string& chain_name = CBaseChainParams::REGTEST, const std::vector& extra_args = {}) +{ + const std::vector arguments = Cat( + { + "-nodebuglogfile", + "-nodebug", + }, + extra_args); + + return std::make_unique(chain_name, arguments); +} + class CTxMemPoolEntry; struct TestMemPoolEntryHelper