Merge #21003: test: Move MakeNoLogFileContext to libtest_util, and use it in bench

fanquake · fanquake · commit 83bdbbd300bb · 2021-03-04T20:29:09.000+08:00
fa576b4 Move MakeNoLogFileContext to common libtest_util, and use it in bench (MarcoFalke) Pull request description: To avoid verbose code duplication, which may lead to accidental mishaps https://github.com/bitcoin/bitcoin/pull/20998/files#r563624041. Also fix a nit I found in bitcoin/bitcoin#20946 (comment). ACKs for top commit: dongcarl: Light Code-Review ACK fa576b4 fanquake: ACK fa576b4 Tree-SHA512: d39ac9c0957813ebb20ed13bd25a8ef8469377ce2651245638bd761c796feac2a17e30dd16f1e5c8db57737fb918c81d56a3d784c33258275e426a1b11e69eb2
diff --git 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 TestingSetup>();
 
     const std::vector<unsigned char> op_true{OP_TRUE};
     CScriptWitness witness;
@@ -38,7 +32,7 @@ static void AssembleBlock(benchmark::Bench& bench)
     std::array<CTransactionRef, NUM_BLOCKS - COINBASE_MATURITY + 1> 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
@@ -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 TestingSetup>();
 
     const CScript SCRIPT_PUB{CScript(OP_TRUE)};
 
diff --git 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<const TestingSetup>();
 
     CMutableTransaction tx1 = CMutableTransaction();
     tx1.vin.resize(1);
diff --git 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<const TestingSetup>(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
@@ -15,7 +15,7 @@
 namespace {
 
 struct TestBlockAndIndex {
-    TestingSetup test_setup{};
+    const std::unique_ptr<const TestingSetup> testing_setup{MakeNoLogFileContext<const TestingSetup>(CBaseChainParams::MAIN)};
     CBlock block{};
     uint256 blockHash{};
     CBlockIndex blockindex{};
diff --git 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 TestingSetup>();
 
     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<std::string> address_mine{add_mine ? Optional<std::string>{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
@@ -8,6 +8,7 @@
 #include <test/fuzz/FuzzedDataProvider.h>
 #include <test/fuzz/fuzz.h>
 #include <test/fuzz/util.h>
+#include <test/util/setup_common.h>
 #include <util/system.h>
 
 #include <cstdint>
diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp
@@ -16,6 +16,7 @@
 #include <test/fuzz/FuzzedDataProvider.h>
 #include <test/fuzz/fuzz.h>
 #include <test/fuzz/util.h>
+#include <test/util/setup_common.h>
 #include <validation.h>
 
 #include <cstdint>
diff --git a/src/test/fuzz/connman.cpp b/src/test/fuzz/connman.cpp
@@ -10,6 +10,7 @@
 #include <test/fuzz/FuzzedDataProvider.h>
 #include <test/fuzz/fuzz.h>
 #include <test/fuzz/util.h>
+#include <test/util/setup_common.h>
 #include <util/translation.h>
 
 #include <cstdint>
diff --git a/src/test/fuzz/data_stream.cpp b/src/test/fuzz/data_stream.cpp
@@ -7,6 +7,7 @@
 #include <test/fuzz/FuzzedDataProvider.h>
 #include <test/fuzz/fuzz.h>
 #include <test/fuzz/util.h>
+#include <test/util/setup_common.h>
 
 #include <cstdint>
 #include <vector>
diff --git a/src/test/fuzz/policy_estimator.cpp b/src/test/fuzz/policy_estimator.cpp
@@ -7,6 +7,7 @@
 #include <test/fuzz/FuzzedDataProvider.h>
 #include <test/fuzz/fuzz.h>
 #include <test/fuzz/util.h>
+#include <test/util/setup_common.h>
 #include <txmempool.h>
 
 #include <cstdint>
diff --git a/src/test/fuzz/policy_estimator_io.cpp b/src/test/fuzz/policy_estimator_io.cpp
@@ -6,6 +6,7 @@
 #include <test/fuzz/FuzzedDataProvider.h>
 #include <test/fuzz/fuzz.h>
 #include <test/fuzz/util.h>
+#include <test/util/setup_common.h>
 
 #include <cstdint>
 #include <vector>
diff --git a/src/test/fuzz/signet.cpp b/src/test/fuzz/signet.cpp
@@ -10,6 +10,7 @@
 #include <test/fuzz/FuzzedDataProvider.h>
 #include <test/fuzz/fuzz.h>
 #include <test/fuzz/util.h>
+#include <test/util/setup_common.h>
 
 #include <cstdint>
 #include <optional>
diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h
@@ -23,11 +23,9 @@
 #include <test/fuzz/FuzzedDataProvider.h>
 #include <test/fuzz/fuzz.h>
 #include <test/util/net.h>
-#include <test/util/setup_common.h>
 #include <txmempool.h>
 #include <uint256.h>
 #include <util/time.h>
-#include <util/vector.h>
 #include <version.h>
 
 #include <algorithm>
@@ -334,19 +332,6 @@ inline std::unique_ptr<CNode> ConsumeNodeAsUniquePtr(FuzzedDataProvider& fdp, co
 
 void FillNode(FuzzedDataProvider& fuzzed_data_provider, CNode& node, bool init_version) noexcept;
 
-template <class T = const BasicTestingSetup>
-std::unique_ptr<T> MakeNoLogFileContext(const std::string& chain_name = CBaseChainParams::REGTEST, const std::vector<const char*>& extra_args = {})
-{
-    // Prepend default arguments for fuzzing
-    const std::vector<const char*> arguments = Cat(
-        {
-            "-nodebuglogfile",
-        },
-        extra_args);
-
-    return MakeUnique<T>(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
@@ -15,6 +15,7 @@
 #include <txmempool.h>
 #include <util/check.h>
 #include <util/string.h>
+#include <util/vector.h>
 
 #include <type_traits>
 #include <vector>
@@ -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 <class T = const BasicTestingSetup>
+std::unique_ptr<T> MakeNoLogFileContext(const std::string& chain_name = CBaseChainParams::REGTEST, const std::vector<const char*>& extra_args = {})
+{
+    const std::vector<const char*> arguments = Cat(
+        {
+            "-nodebuglogfile",
+            "-nodebug",
+        },
+        extra_args);
+
+    return std::make_unique<T>(chain_name, arguments);
+}
+
 class CTxMemPoolEntry;
 
 struct TestMemPoolEntryHelper

Original file line number	Diff line number	Diff line change
`@@ -79,7 +79,7 @@ static void ComplexMemPool(benchmark::Bench& bench)`
`79`	`79`	`ordered_coins.emplace_back(MakeTransactionRef(tx));`
`80`	`80`	`available_coins.emplace_back(ordered_coins.back(), tx_counter++);`
`81`	`81`	`}`
`82`		`- TestingSetup test_setup;`
	`82`	`+ const auto testing_setup = MakeNoLogFileContext<const TestingSetup>(CBaseChainParams::MAIN);`
`83`	`83`	`CTxMemPool pool;`
`84`	`84`	`LOCK2(cs_main, pool.cs);`
`85`	`85`	`bench.run([&]() NO_THREAD_SAFETY_ANALYSIS {`