checkqueue: Use buckets for batch validation to prevent lock contention

Author: fjahr

src/checkqueue.h

@@ -48,6 +48,16 @@ class CCheckQueue
 
     BatchSchnorrVerifier m_batch;
 
+    static constexpr size_t SIGNATURE_BUCKETS = 15; // Same as MAX_SCRIPTCHECK_THREADS
+    std::array<std::vector<SchnorrSignatureToVerify>, SIGNATURE_BUCKETS + 1 /* Add one for master thread */> m_signature_buckets;
+
+    void AddSignaturesToBucket(const std::vector<SchnorrSignatureToVerify>& signatures, size_t thread_idx) {
+        assert(thread_idx >= 0 && thread_idx < m_signature_buckets.size());
+
+        auto& bucket = m_signature_buckets[thread_idx];
+        bucket.insert(bucket.end(), signatures.begin(), signatures.end());
+    }
+
     //! Mutex to protect the inner state
     Mutex m_mutex;
 
@@ -84,7 +94,7 @@ class CCheckQueue
     bool m_request_stop GUARDED_BY(m_mutex){false};
 
     /** Internal function that does bulk of the verification work. If fMaster, return the final result. */
-    std::optional<R> Loop(bool fMaster) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
+    std::optional<R> Loop(bool fMaster, size_t thread_idx) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
     {
         std::condition_variable& cond = fMaster ? m_master_cv : m_worker_cv;
         std::vector<T> vChecks;
@@ -114,6 +124,12 @@ class CCheckQueue
                     if (fMaster && nTodo == 0) {
                         // All checks are done; master performs batch verification
                         if constexpr (std::is_same_v<R, ScriptFailureResult>) {
+                            for (auto& bucket : m_signature_buckets) {
+                                for (const auto& sig : bucket) {
+                                    m_batch.Add(sig.sig, sig.pubkey, sig.sighash);
+                                }
+                                bucket.clear();
+                            }
                             if (!m_batch.Verify()) {
                                 m_result = ScriptFailureResult(SCRIPT_ERR_BATCH_VALIDATION_FAILED, "Schnorr batch validation failed");
                             }
@@ -159,9 +175,7 @@ class CCheckQueue
                         }
                         // Check succeeded; add signatures to shared batch
                         const auto& signatures = std::get<std::vector<SchnorrSignatureToVerify>>(check_result);
-                        for (const auto& sig : signatures) {
-                            m_batch.Add(sig.sig, sig.pubkey, sig.sighash);
-                        }
+                        AddSignaturesToBucket(signatures, thread_idx);
                     } else {
                         local_result = check_result;
                         if (local_result.has_value()) break;
@@ -185,7 +199,7 @@ class CCheckQueue
         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 */);
+                Loop(false /* worker thread */, n /* index */);
             });
         }
     }
@@ -201,7 +215,7 @@ class CCheckQueue
     //! its error.
     std::optional<R> Complete() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
     {
-        return Loop(true /* master thread */);
+        return Loop(true /* master thread */, SIGNATURE_BUCKETS /* take the last bucket */);
     }
 
     //! Add a batch of checks to the queue

src/test/transaction_tests.cpp

@@ -567,7 +567,7 @@ BOOST_AUTO_TEST_CASE(test_big_witness_transaction)
 
     // check all inputs concurrently, with the cache
     PrecomputedTransactionData txdata(tx);
-    CCheckQueue<CScriptCheck, ScriptFailureResult> scriptcheckqueue(/*batch_size=*/128, /*worker_threads_num=*/20);
+    CCheckQueue<CScriptCheck, ScriptFailureResult> scriptcheckqueue(/*batch_size=*/128, /*worker_threads_num=*/15);
     CCheckQueueControl<CScriptCheck, ScriptFailureResult> control(&scriptcheckqueue);
 
     std::vector<Coin> coins;