From be4ff3060b7b43b496dfb5a2c02b114b2b717106 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Fri, 7 Jul 2023 10:33:11 +0100 Subject: [PATCH 1/6] Move global `scriptcheckqueue` into `ChainstateManager` class --- src/bitcoin-chainstate.cpp | 2 +- src/init.cpp | 20 +------------------- src/kernel/chainstatemanager_opts.h | 2 ++ src/node/chainstatemanager_args.cpp | 13 +++++++++++++ src/test/util/setup_common.cpp | 6 ++---- src/validation.cpp | 25 ++++++++++++------------- src/validation.h | 12 +++++++----- 7 files changed, 38 insertions(+), 42 deletions(-) diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index fc83a4ad3a3..1b4a3130293 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -290,7 +290,7 @@ epilogue: // dereferencing and UB. scheduler.stop(); if (chainman.m_thread_load.joinable()) chainman.m_thread_load.join(); - StopScriptCheckWorkerThreads(); + chainman.StopScriptCheckWorkerThreads(); GetMainSignals().FlushBackgroundCallbacks(); { diff --git a/src/init.cpp b/src/init.cpp index a0b44258981..b37552d407c 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -271,7 +271,7 @@ void Shutdown(NodeContext& node) // CScheduler/checkqueue, scheduler and load block thread. if (node.scheduler) node.scheduler->stop(); if (node.chainman && node.chainman->m_thread_load.joinable()) node.chainman->m_thread_load.join(); - StopScriptCheckWorkerThreads(); + if (node.chainman) node.chainman->StopScriptCheckWorkerThreads(); // After the threads that potentially access these pointers have been stopped, // destruct and reset all to nullptr. @@ -1109,24 +1109,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) return InitError(strprintf(_("Unable to allocate memory for -maxsigcachesize: '%s' MiB"), args.GetIntArg("-maxsigcachesize", DEFAULT_MAX_SIG_CACHE_BYTES >> 20))); } - int script_threads = args.GetIntArg("-par", DEFAULT_SCRIPTCHECK_THREADS); - if (script_threads <= 0) { - // -par=0 means autodetect (number of cores - 1 script threads) - // -par=-n means "leave n cores free" (number of cores - n - 1 script threads) - script_threads += GetNumCores(); - } - - // Subtract 1 because the main thread counts towards the par threads - script_threads = std::max(script_threads - 1, 0); - - // Number of script-checking threads <= MAX_SCRIPTCHECK_THREADS - script_threads = std::min(script_threads, MAX_SCRIPTCHECK_THREADS); - - LogPrintf("Script verification uses %d additional threads\n", script_threads); - if (script_threads >= 1) { - StartScriptCheckWorkerThreads(script_threads); - } - assert(!node.scheduler); node.scheduler = std::make_unique(); diff --git a/src/kernel/chainstatemanager_opts.h b/src/kernel/chainstatemanager_opts.h index 917f7d226c5..ee20eabd795 100644 --- a/src/kernel/chainstatemanager_opts.h +++ b/src/kernel/chainstatemanager_opts.h @@ -45,6 +45,8 @@ struct ChainstateManagerOpts { DBOptions coins_db{}; CoinsViewOptions coins_view{}; Notifications& notifications; + //! Number of script check worker threads. Zero means no parallel verification. + int worker_threads_num{0}; }; } // namespace kernel diff --git a/src/node/chainstatemanager_args.cpp b/src/node/chainstatemanager_args.cpp index 87d9238c180..e61deca3ec3 100644 --- a/src/node/chainstatemanager_args.cpp +++ b/src/node/chainstatemanager_args.cpp @@ -6,7 +6,9 @@ #include #include +#include #include +#include #include #include #include @@ -16,6 +18,7 @@ #include #include +#include #include #include @@ -41,6 +44,16 @@ util::Result ApplyArgsManOptions(const ArgsManager& args, ChainstateManage ReadDatabaseArgs(args, opts.coins_db); ReadCoinsViewArgs(args, opts.coins_view); + int script_threads = args.GetIntArg("-par", DEFAULT_SCRIPTCHECK_THREADS); + if (script_threads <= 0) { + // -par=0 means autodetect (number of cores - 1 script threads) + // -par=-n means "leave n cores free" (number of cores - n - 1 script threads) + script_threads += GetNumCores(); + } + // Subtract 1 because the main thread counts towards the par threads. + opts.worker_threads_num = std::clamp(script_threads - 1, 0, MAX_SCRIPTCHECK_THREADS); + LogPrintf("Script verification uses %d additional threads\n", opts.worker_threads_num); + return {}; } } // namespace node diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 2947bc3fcb2..f14f13fdda9 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -176,6 +176,7 @@ ChainTestingSetup::ChainTestingSetup(const ChainType chainType, const std::vecto .adjusted_time_callback = GetAdjustedTime, .check_block_index = true, .notifications = *m_node.notifications, + .worker_threads_num = 2, }; const BlockManager::Options blockman_opts{ .chainparams = chainman_opts.chainparams, @@ -187,15 +188,12 @@ ChainTestingSetup::ChainTestingSetup(const ChainType chainType, const std::vecto .path = m_args.GetDataDirNet() / "blocks" / "index", .cache_bytes = static_cast(m_cache_sizes.block_tree_db), .memory_only = true}); - - constexpr int script_check_threads = 2; - StartScriptCheckWorkerThreads(script_check_threads); } ChainTestingSetup::~ChainTestingSetup() { if (m_node.scheduler) m_node.scheduler->stop(); - StopScriptCheckWorkerThreads(); + m_node.chainman->StopScriptCheckWorkerThreads(); GetMainSignals().FlushBackgroundCallbacks(); GetMainSignals().UnregisterBackgroundSignalScheduler(); m_node.connman.reset(); diff --git a/src/validation.cpp b/src/validation.cpp index 30b3dde74f0..f744126c9f3 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2047,16 +2047,9 @@ DisconnectResult Chainstate::DisconnectBlock(const CBlock& block, const CBlockIn return fClean ? DISCONNECT_OK : DISCONNECT_UNCLEAN; } -static CCheckQueue scriptcheckqueue(128); - -void StartScriptCheckWorkerThreads(int threads_num) +void ChainstateManager::StopScriptCheckWorkerThreads() { - scriptcheckqueue.StartWorkerThreads(threads_num); -} - -void StopScriptCheckWorkerThreads() -{ - scriptcheckqueue.StopWorkerThreads(); + m_script_check_queue.StopWorkerThreads(); } /** @@ -2147,7 +2140,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, uint256 block_hash{block.GetHash()}; assert(*pindex->phashBlock == block_hash); - const bool parallel_script_checks{scriptcheckqueue.HasThreads()}; + const bool parallel_script_checks{m_chainman.GetCheckQueue().HasThreads()}; const auto time_start{SteadyClock::now()}; const CChainParams& params{m_chainman.GetParams()}; @@ -2336,7 +2329,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, // in multiple threads). Preallocate the vector size so a new allocation // doesn't invalidate pointers into the vector, and keep txsdata in scope // for as long as `control`. - CCheckQueueControl control(fScriptChecks && parallel_script_checks ? &scriptcheckqueue : nullptr); + CCheckQueueControl control(fScriptChecks && parallel_script_checks ? &m_chainman.GetCheckQueue() : nullptr); std::vector txsdata(block.vtx.size()); std::vector prevheights; @@ -5751,12 +5744,18 @@ static ChainstateManager::Options&& Flatten(ChainstateManager::Options&& opts) } ChainstateManager::ChainstateManager(const util::SignalInterrupt& interrupt, Options options, node::BlockManager::Options blockman_options) - : m_interrupt{interrupt}, + : m_script_check_queue{/*nBatchSizeIn=*/128}, + m_interrupt{interrupt}, m_options{Flatten(std::move(options))}, - m_blockman{interrupt, std::move(blockman_options)} {} + m_blockman{interrupt, std::move(blockman_options)} +{ + m_script_check_queue.StartWorkerThreads(m_options.worker_threads_num); +} ChainstateManager::~ChainstateManager() { + StopScriptCheckWorkerThreads(); + LOCK(::cs_main); m_versionbitscache.Clear(); diff --git a/src/validation.h b/src/validation.h index 94a00e44a4e..5fc4efae141 100644 --- a/src/validation.h +++ b/src/validation.h @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -98,11 +99,6 @@ extern uint256 g_best_block; /** Documentation for argument 'checklevel'. */ extern const std::vector CHECKLEVEL_DOC; -/** Run instances of script checking worker threads */ -void StartScriptCheckWorkerThreads(int threads_num); -/** Stop all of the script checking worker threads */ -void StopScriptCheckWorkerThreads(); - CAmount GetBlockSubsidy(int nHeight, const Consensus::Params& consensusParams); bool FatalError(kernel::Notifications& notifications, BlockValidationState& state, const std::string& strMessage, const bilingual_str& userMessage = {}); @@ -896,6 +892,9 @@ private: return cs && !cs->m_disabled; } + //! A queue for script verifications that have to be performed by worker threads. + CCheckQueue m_script_check_queue; + public: using Options = kernel::ChainstateManagerOpts; @@ -1246,6 +1245,9 @@ public: //! nullopt. std::optional GetSnapshotBaseHeight() const EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + CCheckQueue& GetCheckQueue() { return m_script_check_queue; } + void StopScriptCheckWorkerThreads(); + ~ChainstateManager(); }; From d03eaacbcfb276fb638db1b423113ff43bd7ec41 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Fri, 7 Jul 2023 10:37:36 +0100 Subject: [PATCH 2/6] Make `CCheckQueue` destructor stop worker threads --- src/bench/checkqueue.cpp | 1 - src/bitcoin-chainstate.cpp | 1 - src/checkqueue.h | 10 +--------- src/init.cpp | 3 +-- src/test/checkqueue_tests.cpp | 6 ------ src/test/transaction_tests.cpp | 1 - src/test/util/setup_common.cpp | 1 - src/validation.cpp | 7 ------- src/validation.h | 1 - 9 files changed, 2 insertions(+), 29 deletions(-) diff --git a/src/bench/checkqueue.cpp b/src/bench/checkqueue.cpp index 70e0b86ebaa..c7e1ad4f838 100644 --- a/src/bench/checkqueue.cpp +++ b/src/bench/checkqueue.cpp @@ -61,7 +61,6 @@ static void CCheckQueueSpeedPrevectorJob(benchmark::Bench& bench) // it is done explicitly here for clarity control.Wait(); }); - queue.StopWorkerThreads(); ECC_Stop(); } BENCHMARK(CCheckQueueSpeedPrevectorJob, benchmark::PriorityLevel::HIGH); diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index 1b4a3130293..a8f6fec90ab 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -290,7 +290,6 @@ epilogue: // dereferencing and UB. scheduler.stop(); if (chainman.m_thread_load.joinable()) chainman.m_thread_load.join(); - chainman.StopScriptCheckWorkerThreads(); GetMainSignals().FlushBackgroundCallbacks(); { diff --git a/src/checkqueue.h b/src/checkqueue.h index a3299fb3fe9..a89be2cfd53 100644 --- a/src/checkqueue.h +++ b/src/checkqueue.h @@ -179,24 +179,16 @@ public: } } - //! Stop all of the worker threads. - void StopWorkerThreads() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) + ~CCheckQueue() { WITH_LOCK(m_mutex, m_request_stop = true); m_worker_cv.notify_all(); for (std::thread& t : m_worker_threads) { t.join(); } - m_worker_threads.clear(); - WITH_LOCK(m_mutex, m_request_stop = false); } bool HasThreads() const { return !m_worker_threads.empty(); } - - ~CCheckQueue() - { - assert(m_worker_threads.empty()); - } }; /** diff --git a/src/init.cpp b/src/init.cpp index b37552d407c..a60e08a9803 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -268,10 +268,9 @@ void Shutdown(NodeContext& node) StopTorControl(); // After everything has been shut down, but before things get flushed, stop the - // CScheduler/checkqueue, scheduler and load block thread. + // scheduler and load block thread. if (node.scheduler) node.scheduler->stop(); if (node.chainman && node.chainman->m_thread_load.joinable()) node.chainman->m_thread_load.join(); - if (node.chainman) node.chainman->StopScriptCheckWorkerThreads(); // After the threads that potentially access these pointers have been stopped, // destruct and reset all to nullptr. diff --git a/src/test/checkqueue_tests.cpp b/src/test/checkqueue_tests.cpp index cb3831071a0..9c2a5d1ae66 100644 --- a/src/test/checkqueue_tests.cpp +++ b/src/test/checkqueue_tests.cpp @@ -176,7 +176,6 @@ static void Correct_Queue_range(std::vector range) BOOST_REQUIRE(control.Wait()); BOOST_REQUIRE_EQUAL(FakeCheckCheckCompletion::n_calls, i); } - small_queue->StopWorkerThreads(); } /** Test that 0 checks is correct @@ -240,7 +239,6 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_Catches_Failure) BOOST_REQUIRE(success); } } - fail_queue->StopWorkerThreads(); } // Test that a block validation which fails does not interfere with // future blocks, ie, the bad state is cleared. @@ -262,7 +260,6 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_Recovers_From_Failure) BOOST_REQUIRE(r != end_fails); } } - fail_queue->StopWorkerThreads(); } // Test that unique checks are actually all called individually, rather than @@ -294,7 +291,6 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_UniqueCheck) } BOOST_REQUIRE(r); } - queue->StopWorkerThreads(); } @@ -325,7 +321,6 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_Memory) } BOOST_REQUIRE_EQUAL(MemoryCheck::fake_allocated_memory, 0U); } - queue->StopWorkerThreads(); } // Test that a new verification cannot occur until all checks @@ -361,7 +356,6 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_FrozenCleanup) // Wait for control to finish t0.join(); BOOST_REQUIRE(!fails); - queue->StopWorkerThreads(); } diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index a4c0db8aea5..830cfeb25a9 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -553,7 +553,6 @@ BOOST_AUTO_TEST_CASE(test_big_witness_transaction) bool controlCheck = control.Wait(); assert(controlCheck); - scriptcheckqueue.StopWorkerThreads(); } SignatureData CombineSignatures(const CMutableTransaction& input1, const CMutableTransaction& input2, const CTransactionRef tx) diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index f14f13fdda9..0124d85580d 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -193,7 +193,6 @@ ChainTestingSetup::ChainTestingSetup(const ChainType chainType, const std::vecto ChainTestingSetup::~ChainTestingSetup() { if (m_node.scheduler) m_node.scheduler->stop(); - m_node.chainman->StopScriptCheckWorkerThreads(); GetMainSignals().FlushBackgroundCallbacks(); GetMainSignals().UnregisterBackgroundSignalScheduler(); m_node.connman.reset(); diff --git a/src/validation.cpp b/src/validation.cpp index f744126c9f3..91027872b02 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2047,11 +2047,6 @@ DisconnectResult Chainstate::DisconnectBlock(const CBlock& block, const CBlockIn return fClean ? DISCONNECT_OK : DISCONNECT_UNCLEAN; } -void ChainstateManager::StopScriptCheckWorkerThreads() -{ - m_script_check_queue.StopWorkerThreads(); -} - /** * Threshold condition checker that triggers when unknown versionbits are seen on the network. */ @@ -5754,8 +5749,6 @@ ChainstateManager::ChainstateManager(const util::SignalInterrupt& interrupt, Opt ChainstateManager::~ChainstateManager() { - StopScriptCheckWorkerThreads(); - LOCK(::cs_main); m_versionbitscache.Clear(); diff --git a/src/validation.h b/src/validation.h index 5fc4efae141..aacc9b828f8 100644 --- a/src/validation.h +++ b/src/validation.h @@ -1246,7 +1246,6 @@ public: std::optional GetSnapshotBaseHeight() const EXCLUSIVE_LOCKS_REQUIRED(::cs_main); CCheckQueue& GetCheckQueue() { return m_script_check_queue; } - void StopScriptCheckWorkerThreads(); ~ChainstateManager(); }; From 9cf89f7a5b81197e38f58b24be0793b28fe41477 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Fri, 7 Jul 2023 10:40:13 +0100 Subject: [PATCH 3/6] refactor: Make `CCheckQueue` constructor start worker threads --- src/bench/checkqueue.cpp | 5 +++-- src/checkqueue.h | 19 ++++--------------- src/test/checkqueue_tests.cpp | 23 +++++++---------------- src/test/fuzz/checkqueue.cpp | 4 ++-- src/test/transaction_tests.cpp | 4 +--- src/validation.cpp | 3 +-- 6 files changed, 18 insertions(+), 40 deletions(-) diff --git a/src/bench/checkqueue.cpp b/src/bench/checkqueue.cpp index c7e1ad4f838..114dd9d39c8 100644 --- a/src/bench/checkqueue.cpp +++ b/src/bench/checkqueue.cpp @@ -37,10 +37,11 @@ static void CCheckQueueSpeedPrevectorJob(benchmark::Bench& bench) return true; } }; - CCheckQueue queue {QUEUE_BATCH_SIZE}; + // The main thread should be counted to prevent thread oversubscription, and // to decrease the variance of benchmark results. - queue.StartWorkerThreads(GetNumCores() - 1); + int worker_threads_num{GetNumCores() - 1}; + CCheckQueue queue{QUEUE_BATCH_SIZE, worker_threads_num}; // create all the data once, then submit copies in the benchmark. FastRandomContext insecure_rand(true); diff --git a/src/checkqueue.h b/src/checkqueue.h index a89be2cfd53..f8b288c10a6 100644 --- a/src/checkqueue.h +++ b/src/checkqueue.h @@ -130,22 +130,11 @@ public: Mutex m_control_mutex; //! Create a new check queue - explicit CCheckQueue(unsigned int nBatchSizeIn) - : nBatchSize(nBatchSizeIn) + explicit CCheckQueue(unsigned int batch_size, int worker_threads_num) + : nBatchSize(batch_size) { - } - - //! Create a pool of new worker threads. - void StartWorkerThreads(const int threads_num) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) - { - { - LOCK(m_mutex); - nIdle = 0; - nTotal = 0; - fAllOk = true; - } - assert(m_worker_threads.empty()); - for (int n = 0; n < threads_num; ++n) { + m_worker_threads.reserve(worker_threads_num); + for (int n = 0; n < worker_threads_num; ++n) { m_worker_threads.emplace_back([this, n]() { util::ThreadRename(strprintf("scriptch.%i", n)); Loop(false /* worker thread */); diff --git a/src/test/checkqueue_tests.cpp b/src/test/checkqueue_tests.cpp index 9c2a5d1ae66..023a5e8e70d 100644 --- a/src/test/checkqueue_tests.cpp +++ b/src/test/checkqueue_tests.cpp @@ -158,8 +158,7 @@ typedef CCheckQueue FrozenCleanup_Queue; */ static void Correct_Queue_range(std::vector range) { - auto small_queue = std::make_unique(QUEUE_BATCH_SIZE); - small_queue->StartWorkerThreads(SCRIPT_CHECK_THREADS); + auto small_queue = std::make_unique(QUEUE_BATCH_SIZE, SCRIPT_CHECK_THREADS); // Make vChecks here to save on malloc (this test can be slow...) std::vector vChecks; vChecks.reserve(9); @@ -217,9 +216,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_Correct_Random) /** Test that failing checks are caught */ BOOST_AUTO_TEST_CASE(test_CheckQueue_Catches_Failure) { - auto fail_queue = std::make_unique(QUEUE_BATCH_SIZE); - fail_queue->StartWorkerThreads(SCRIPT_CHECK_THREADS); - + auto fail_queue = std::make_unique(QUEUE_BATCH_SIZE, SCRIPT_CHECK_THREADS); for (size_t i = 0; i < 1001; ++i) { CCheckQueueControl control(fail_queue.get()); size_t remaining = i; @@ -244,9 +241,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_Catches_Failure) // future blocks, ie, the bad state is cleared. BOOST_AUTO_TEST_CASE(test_CheckQueue_Recovers_From_Failure) { - auto fail_queue = std::make_unique(QUEUE_BATCH_SIZE); - fail_queue->StartWorkerThreads(SCRIPT_CHECK_THREADS); - + auto fail_queue = std::make_unique(QUEUE_BATCH_SIZE, SCRIPT_CHECK_THREADS); for (auto times = 0; times < 10; ++times) { for (const bool end_fails : {true, false}) { CCheckQueueControl control(fail_queue.get()); @@ -267,9 +262,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_Recovers_From_Failure) // more than once as well BOOST_AUTO_TEST_CASE(test_CheckQueue_UniqueCheck) { - auto queue = std::make_unique(QUEUE_BATCH_SIZE); - queue->StartWorkerThreads(SCRIPT_CHECK_THREADS); - + auto queue = std::make_unique(QUEUE_BATCH_SIZE, SCRIPT_CHECK_THREADS); size_t COUNT = 100000; size_t total = COUNT; { @@ -301,8 +294,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_UniqueCheck) // time could leave the data hanging across a sequence of blocks. BOOST_AUTO_TEST_CASE(test_CheckQueue_Memory) { - auto queue = std::make_unique(QUEUE_BATCH_SIZE); - queue->StartWorkerThreads(SCRIPT_CHECK_THREADS); + auto queue = std::make_unique(QUEUE_BATCH_SIZE, SCRIPT_CHECK_THREADS); for (size_t i = 0; i < 1000; ++i) { size_t total = i; { @@ -327,9 +319,8 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_Memory) // have been destructed BOOST_AUTO_TEST_CASE(test_CheckQueue_FrozenCleanup) { - auto queue = std::make_unique(QUEUE_BATCH_SIZE); + auto queue = std::make_unique(QUEUE_BATCH_SIZE, SCRIPT_CHECK_THREADS); bool fails = false; - queue->StartWorkerThreads(SCRIPT_CHECK_THREADS); std::thread t0([&]() { CCheckQueueControl control(queue.get()); std::vector vChecks(1); @@ -362,7 +353,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_FrozenCleanup) /** Test that CCheckQueueControl is threadsafe */ BOOST_AUTO_TEST_CASE(test_CheckQueueControl_Locks) { - auto queue = std::make_unique(QUEUE_BATCH_SIZE); + auto queue = std::make_unique(QUEUE_BATCH_SIZE, SCRIPT_CHECK_THREADS); { std::vector tg; std::atomic nThreads {0}; diff --git a/src/test/fuzz/checkqueue.cpp b/src/test/fuzz/checkqueue.cpp index 429570526f1..6320b500b6f 100644 --- a/src/test/fuzz/checkqueue.cpp +++ b/src/test/fuzz/checkqueue.cpp @@ -31,8 +31,8 @@ FUZZ_TARGET(checkqueue) FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); const unsigned int batch_size = fuzzed_data_provider.ConsumeIntegralInRange(0, 1024); - CCheckQueue check_queue_1{batch_size}; - CCheckQueue check_queue_2{batch_size}; + CCheckQueue check_queue_1{batch_size, /*worker_threads_num=*/0}; + CCheckQueue check_queue_2{batch_size, /*worker_threads_num=*/0}; std::vector checks_1; std::vector checks_2; const int size = fuzzed_data_provider.ConsumeIntegralInRange(0, 1024); diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 830cfeb25a9..932c40cdec1 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -530,11 +530,9 @@ BOOST_AUTO_TEST_CASE(test_big_witness_transaction) // check all inputs concurrently, with the cache PrecomputedTransactionData txdata(tx); - CCheckQueue scriptcheckqueue(128); + CCheckQueue scriptcheckqueue(/*batch_size=*/128, /*worker_threads_num=*/20); CCheckQueueControl control(&scriptcheckqueue); - scriptcheckqueue.StartWorkerThreads(20); - std::vector coins; for(uint32_t i = 0; i < mtx.vin.size(); i++) { Coin coin; diff --git a/src/validation.cpp b/src/validation.cpp index 91027872b02..ba88ef2ec5d 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -5739,12 +5739,11 @@ static ChainstateManager::Options&& Flatten(ChainstateManager::Options&& opts) } ChainstateManager::ChainstateManager(const util::SignalInterrupt& interrupt, Options options, node::BlockManager::Options blockman_options) - : m_script_check_queue{/*nBatchSizeIn=*/128}, + : m_script_check_queue{/*batch_size=*/128, options.worker_threads_num}, m_interrupt{interrupt}, m_options{Flatten(std::move(options))}, m_blockman{interrupt, std::move(blockman_options)} { - m_script_check_queue.StartWorkerThreads(m_options.worker_threads_num); } ChainstateManager::~ChainstateManager() From 8111e74653dc5c93cb510672d99048c3f741d8dc Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Fri, 7 Jul 2023 10:42:49 +0100 Subject: [PATCH 4/6] refactor: Drop unneeded declaration --- src/checkqueue.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/checkqueue.h b/src/checkqueue.h index f8b288c10a6..f3c126e2624 100644 --- a/src/checkqueue.h +++ b/src/checkqueue.h @@ -13,9 +13,6 @@ #include #include -template -class CCheckQueueControl; - /** * Queue for verifications that have to be performed. * The verifications are represented by a type T, which must provide an From 6e17b3168072ab77ed7170ab81327c017877133a Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Fri, 7 Jul 2023 10:43:09 +0100 Subject: [PATCH 5/6] refactor: Make `CCheckQueue` non-copyable and non-movable explicitly --- src/checkqueue.h | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/checkqueue.h b/src/checkqueue.h index f3c126e2624..a1de000714d 100644 --- a/src/checkqueue.h +++ b/src/checkqueue.h @@ -139,6 +139,13 @@ public: } } + // Since this class manages its own resources, which is a thread + // pool `m_worker_threads`, copy and move operations are not appropriate. + CCheckQueue(const CCheckQueue&) = delete; + CCheckQueue& operator=(const CCheckQueue&) = delete; + CCheckQueue(CCheckQueue&&) = delete; + CCheckQueue& operator=(CCheckQueue&&) = delete; + //! Wait until execution finishes, and return whether all evaluations were successful. bool Wait() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) { From 5b3ea5fa2e7f6dc1c9161ed8b74c9be4bd1e92dd Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Fri, 7 Jul 2023 10:43:24 +0100 Subject: [PATCH 6/6] refactor: Move `{MAX,DEFAULT}_SCRIPTCHECK_THREADS` constants --- src/node/chainstatemanager_args.cpp | 1 - src/node/chainstatemanager_args.h | 5 +++++ src/qt/optionsdialog.cpp | 2 +- src/qt/optionsmodel.cpp | 1 + src/validation.h | 4 ---- 5 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/node/chainstatemanager_args.cpp b/src/node/chainstatemanager_args.cpp index e61deca3ec3..1cc126cb051 100644 --- a/src/node/chainstatemanager_args.cpp +++ b/src/node/chainstatemanager_args.cpp @@ -7,7 +7,6 @@ #include #include #include -#include #include #include #include diff --git a/src/node/chainstatemanager_args.h b/src/node/chainstatemanager_args.h index 701515953e8..b2cdba68b8f 100644 --- a/src/node/chainstatemanager_args.h +++ b/src/node/chainstatemanager_args.h @@ -10,6 +10,11 @@ class ArgsManager; +/** Maximum number of dedicated script-checking threads allowed */ +static constexpr int MAX_SCRIPTCHECK_THREADS{15}; +/** -par default (number of script-checking threads, 0 = auto) */ +static constexpr int DEFAULT_SCRIPTCHECK_THREADS{0}; + namespace node { [[nodiscard]] util::Result ApplyArgsManOptions(const ArgsManager& args, ChainstateManager::Options& opts); } // namespace node diff --git a/src/qt/optionsdialog.cpp b/src/qt/optionsdialog.cpp index 512fce473d2..6e1d36effbe 100644 --- a/src/qt/optionsdialog.cpp +++ b/src/qt/optionsdialog.cpp @@ -17,9 +17,9 @@ #include #include +#include #include #include -#include #include diff --git a/src/qt/optionsmodel.cpp b/src/qt/optionsmodel.cpp index c1563fe1e27..43564dad167 100644 --- a/src/qt/optionsmodel.cpp +++ b/src/qt/optionsmodel.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include // for -dbcache defaults #include #include // For DEFAULT_SCRIPTCHECK_THREADS diff --git a/src/validation.h b/src/validation.h index aacc9b828f8..1165511ead5 100644 --- a/src/validation.h +++ b/src/validation.h @@ -66,10 +66,6 @@ namespace util { class SignalInterrupt; } // namespace util -/** Maximum number of dedicated script-checking threads allowed */ -static const int MAX_SCRIPTCHECK_THREADS = 15; -/** -par default (number of script-checking threads, 0 = auto) */ -static const int DEFAULT_SCRIPTCHECK_THREADS = 0; /** Block files containing a block-height within MIN_BLOCKS_TO_KEEP of ActiveChain().Tip() will not be pruned. */ static const unsigned int MIN_BLOCKS_TO_KEEP = 288; static const signed int DEFAULT_CHECKBLOCKS = 6;