Merge 6a9c6ea973 into a46065e36c

src/Makefile.am

@@ -289,6 +289,7 @@ BITCOIN_CORE_H = \
   undo.h \
   util/any.h \
   util/asmap.h \
+  util/bag.h \
   util/batchpriority.h \
   util/bip32.h \
   util/bitdeque.h \

src/Makefile.test.include

@@ -68,6 +68,7 @@ BITCOIN_TESTS =\
   test/amount_tests.cpp \
   test/argsman_tests.cpp \
   test/arith_uint256_tests.cpp \
+  test/bag_test.cpp \
   test/banman_tests.cpp \
   test/base32_tests.cpp \
   test/base58_tests.cpp \

src/checkqueue.h

@@ -7,6 +7,7 @@
 
 #include <sync.h>
 #include <tinyformat.h>
+#include <util/bag.h>
 #include <util/threadnames.h>
 
 #include <algorithm>
@@ -36,9 +37,8 @@ private:
     //! Master thread blocks on this when out of work
     std::condition_variable m_master_cv;
 
-    //! The queue of elements to be processed.
-    //! As the order of booleans doesn't matter, it is used as a LIFO (stack)
-    std::vector<T> queue GUARDED_BY(m_mutex);
+    //! A bag of elements to be processed. The order doesn't matter.
+    Bag<T> m_bag GUARDED_BY(m_mutex);
 
     //! The number of workers (including the master) that are idle.
     int nIdle GUARDED_BY(m_mutex){0};
@@ -51,7 +51,7 @@ private:
 
     /**
      * Number of verifications that haven't completed yet.
-     * This includes elements that are no longer queued, but still in the
+     * This includes elements that are no longer in the bag, but still in the
      * worker's own batches.
      */
     unsigned int nTodo GUARDED_BY(m_mutex){0};
@@ -85,7 +85,7 @@ private:
                     nTotal++;
                 }
                 // logically, the do loop starts here
-                while (queue.empty() && !m_request_stop) {
+                while (m_bag.empty() && !m_request_stop) {
                     if (fMaster && nTodo == 0) {
                         nTotal--;
                         bool fRet = fAllOk;
@@ -107,10 +107,8 @@ private:
                 //   all workers finish approximately simultaneously.
                 // * Try to account for idle jobs which will instantly start helping.
                 // * Don't do batches smaller than 1 (duh), or larger than nBatchSize.
-                nNow = std::max(1U, std::min(nBatchSize, (unsigned int)queue.size() / (nTotal + nIdle + 1)));
-                auto start_it = queue.end() - nNow;
-                vChecks.assign(std::make_move_iterator(start_it), std::make_move_iterator(queue.end()));
-                queue.erase(start_it, queue.end());
+                nNow = std::max(1U, std::min(nBatchSize, (unsigned int)m_bag.size() / (nTotal + nIdle + 1)));
+                m_bag.pop(nNow, vChecks);
                 // Check whether we need to do work at all
                 fOk = fAllOk;
             }
@@ -158,14 +156,14 @@ public:
         if (vChecks.empty()) {
             return;
         }
-
+        auto num_checks = vChecks.size();
         {
             LOCK(m_mutex);
-            queue.insert(queue.end(), std::make_move_iterator(vChecks.begin()), std::make_move_iterator(vChecks.end()));
-            nTodo += vChecks.size();
+            m_bag.push(std::move(vChecks));
+            nTodo += num_checks;
         }
 
-        if (vChecks.size() == 1) {
+        if (num_checks == 1) {
             m_worker_cv.notify_one();
         } else {
             m_worker_cv.notify_all();

src/test/bag_test.cpp

@@ -0,0 +1,141 @@
+// Copyright (c) 2023 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <util/bag.h>
+
+#include <boost/test/unit_test.hpp>
+
+#include <set>
+#include <vector>
+
+BOOST_AUTO_TEST_SUITE(bag_tests)
+
+// Simple struct that keeps track of the number of copies, moves, etc.
+// It has no default constructor.
+struct CountCopyAndMoves {
+    static int n_ctor;
+    static int n_copy_ctor;
+    static int n_copy_assignment;
+    static int n_move_ctor;
+    static int n_move_assignment;
+    static int n_dtor;
+
+    int m_x;
+
+    static void ClearCounters()
+    {
+        n_ctor = 0;
+        n_copy_ctor = 0;
+        n_copy_assignment = 0;
+        n_move_ctor = 0;
+        n_move_assignment = 0;
+        n_dtor = 0;
+    }
+
+    CountCopyAndMoves(int x) : m_x{x}
+    {
+        ++n_ctor;
+    }
+
+    CountCopyAndMoves(const CountCopyAndMoves& other)
+        : m_x(other.m_x)
+    {
+        ++n_copy_ctor;
+    }
+
+    CountCopyAndMoves& operator=(const CountCopyAndMoves& other)
+    {
+        m_x = other.m_x;
+        ++n_copy_assignment;
+        return *this;
+    }
+
+    CountCopyAndMoves(CountCopyAndMoves&& other) noexcept
+        : m_x(other.m_x)
+    {
+        other.m_x = -1;
+        ++n_move_ctor;
+    }
+
+    CountCopyAndMoves& operator=(CountCopyAndMoves&& other) noexcept
+    {
+        m_x = other.m_x;
+        other.m_x = -1;
+        ++n_move_assignment;
+        return *this;
+    }
+
+    ~CountCopyAndMoves()
+    {
+        ++n_dtor;
+    }
+};
+
+int CountCopyAndMoves::n_ctor = 0;
+int CountCopyAndMoves::n_copy_ctor = 0;
+int CountCopyAndMoves::n_copy_assignment = 0;
+int CountCopyAndMoves::n_move_ctor = 0;
+int CountCopyAndMoves::n_move_assignment = 0;
+int CountCopyAndMoves::n_dtor = 0;
+
+BOOST_AUTO_TEST_CASE(bag_simple)
+{
+    CountCopyAndMoves::ClearCounters();
+
+    Bag<CountCopyAndMoves> bag{};
+    BOOST_TEST(bag.empty());
+
+    {
+        std::vector<CountCopyAndMoves> entries;
+        entries.emplace_back(1);
+        entries.emplace_back(2);
+        bag.push(std::move(entries));
+    }
+    BOOST_TEST(bag.size() == 2);
+    BOOST_TEST(!bag.empty());
+
+    {
+        std::vector<CountCopyAndMoves> data;
+        data.emplace_back(123);
+        bag.pop(1, data);
+        BOOST_TEST(bag.size() == 1);
+        BOOST_TEST(data.front().m_x != 123);
+        std::set<int> taken_out{};
+        taken_out.insert(data.front().m_x);
+
+        bag.pop(100, data);
+        BOOST_TEST(bag.empty());
+        BOOST_TEST(data.size() == 1);
+        taken_out.insert(data.front().m_x);
+
+        BOOST_TEST((taken_out == std::set<int>{1, 2}));
+    }
+
+    BOOST_TEST(CountCopyAndMoves::n_ctor + CountCopyAndMoves::n_move_ctor == CountCopyAndMoves::n_dtor);
+    BOOST_TEST(CountCopyAndMoves::n_copy_assignment == 0);
+    BOOST_TEST(CountCopyAndMoves::n_copy_ctor == 0);
+}
+
+
+BOOST_AUTO_TEST_CASE(bag_larger)
+{
+    CountCopyAndMoves::ClearCounters();
+    {
+        Bag<CountCopyAndMoves> bag{};
+        {
+            std::vector<CountCopyAndMoves> data;
+            data.reserve(100);
+            for (int i = 0; i < 100; ++i) {
+                data.emplace_back(i);
+            }
+            bag.push(std::move(data));
+        }
+        BOOST_TEST(CountCopyAndMoves::n_ctor + CountCopyAndMoves::n_move_ctor - CountCopyAndMoves::n_dtor == 100);
+    }
+    BOOST_TEST(CountCopyAndMoves::n_ctor + CountCopyAndMoves::n_move_ctor == CountCopyAndMoves::n_dtor);
+    BOOST_TEST(CountCopyAndMoves::n_copy_assignment == 0);
+    BOOST_TEST(CountCopyAndMoves::n_copy_ctor == 0);
+}
+
+BOOST_AUTO_TEST_SUITE_END()

src/util/bag.h

@@ -0,0 +1,55 @@
+// Copyright (c) 2023 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_UTIL_BAG_H
+#define BITCOIN_UTIL_BAG_H
+
+#include <algorithm>
+#include <cstddef>
+#include <iterator>
+#include <vector>
+
+/**
+ * A container that holds a number of elements. Elements can be added and
+ * removed, and the order how the elements are extracted is unspecified
+ * and subject to optimization.
+ */
+template <typename T>
+class Bag final
+{
+    std::vector<T> m_data{};
+
+public:
+    /**
+     * Adds all elements from data to the bag.
+     */
+    void push(std::vector<T>&& data)
+    {
+        m_data.insert(m_data.end(), std::make_move_iterator(data.begin()), std::make_move_iterator(data.end()));
+    }
+
+    /**
+     * Removes n elements from the bag and puts them into out, replacing elements in out if it holds any.
+     * If the bag holds less than n elements only the available elements will be put into out.
+     */
+    void pop(std::size_t n, std::vector<T>& out)
+    {
+        n = std::min(n, m_data.size());
+        auto it = m_data.end() - n;
+        out.assign(std::make_move_iterator(it), std::make_move_iterator(m_data.end()));
+        m_data.erase(it, m_data.end());
+    }
+
+    [[nodiscard]] bool empty() const noexcept
+    {
+        return m_data.empty();
+    }
+
+    [[nodiscard]] std::size_t size() const noexcept
+    {
+        return m_data.size();
+    }
+};
+
+#endif // BITCOIN_UTIL_BAG_H