clusterlin: keep FIFO queue of improvable chunks (preparation)

This introduces a queue of chunks that still need processing, in both
MakeTopological() and OptimizationStep(). This is simultaneously:
* A preparation for introducing randomization, by allowing permuting the
  queue.
* An improvement to the fairness of suboptimal solutions, by distributing
  the work more fairly over chunks.
* An optimization, by avoiding retrying chunks over and over again which
  are already known to be optimal.

src/cluster_linearize.h

@@ -690,8 +690,7 @@ public:
  *     transaction in the other chunk is activated (this will be changed in a later commit).
  *
  * - How to decide which chunk to find a dependency to split in:
- *   - The chunk with the lowest-index representative (an implementation detail) that can be split
+ *   - A round-robin queue of chunks to improve is maintained.
- *     is picked (this will be changed in a later commit).
  *
  * - How to decide what dependency to deactivate (when splitting chunks):
  *   - Inside the selected chunk (see above), among the dependencies whose top feerate is strictly
@@ -742,6 +741,8 @@ private:
     std::vector<TxData> m_tx_data;
     /** Information about each dependency. Indexed by DepIdx. */
     std::vector<DepData> m_dep_data;
+    /** A FIFO of chunk representatives of chunks that may be improved still. */
+    VecDeque<TxIdx> m_suboptimal_chunks;
 
     /** The number of updated transactions in activations/deactivations. */
     uint64_t m_cost{0};
@@ -959,6 +960,8 @@ private:
             if (merged_rep == TxIdx(-1)) break;
             chunk_rep = merged_rep;
         }
+        // Add the chunk to the queue of improvable chunks.
+        m_suboptimal_chunks.push_back(chunk_rep);
     }
 
     /** Split a chunk, and then merge the resulting two chunks to make the graph topological
@@ -1043,38 +1046,58 @@ public:
     /** Make state topological. Can be called after constructing, or after LoadLinearization. */
     void MakeTopological() noexcept
     {
-        while (true) {
+        for (auto tx : m_transaction_idxs) {
-            bool done = true;
+            auto& tx_data = m_tx_data[tx];
-            // Iterate over all transactions (only processing those which are chunk representatives).
+            if (tx_data.chunk_rep == tx) {
-            for (auto chunk : m_transaction_idxs) {
+                m_suboptimal_chunks.emplace_back(tx);
-                auto& chunk_data = m_tx_data[chunk];
+            }
-                // If this is not a chunk representative, skip.
+        }
-                if (chunk_data.chunk_rep != chunk) continue;
+        while (!m_suboptimal_chunks.empty()) {
-                // Attempt to merge the chunk upwards.
+            // Pop an entry from the potentially-suboptimal chunk queue.
-                auto result_up = MergeStep<false>(chunk);
+            TxIdx chunk = m_suboptimal_chunks.front();
-                if (result_up != TxIdx(-1)) {
+            m_suboptimal_chunks.pop_front();
-                    done = false;
+            auto& chunk_data = m_tx_data[chunk];
-                    continue;
+            // If what was popped is not currently a chunk representative, continue. This may
-                }
+            // happen when it was merged with something else since being added.
-                // Attempt to merge the chunk downwards.
+            if (chunk_data.chunk_rep != chunk) continue;
-                auto result_down = MergeStep<true>(chunk);
+            // Attempt to merge the chunk upwards.
-                if (result_down != TxIdx(-1)) {
+            auto result_up = MergeStep<false>(chunk);
-                    done = false;
+            if (result_up != TxIdx(-1)) {
-                    continue;
+                m_suboptimal_chunks.push_back(result_up);
-                }
+                continue;
+            }
+            // Attempt to merge the chunk downwards.
+            auto result_down = MergeStep<true>(chunk);
+            if (result_down != TxIdx(-1)) {
+                m_suboptimal_chunks.push_back(result_down);
+                continue;
+            }
+        }
+    }
+
+    /** Initialize the data structure for optimization. It must be topological already. */
+    void StartOptimizing() noexcept
+    {
+        // Mark chunks suboptimal.
+        for (auto tx : m_transaction_idxs) {
+            auto& tx_data = m_tx_data[tx];
+            if (tx_data.chunk_rep == tx) {
+                m_suboptimal_chunks.push_back(tx);
             }
-            // Stop if no changes were made anymore.
-            if (done) break;
         }
     }
 
     /** Try to improve the forest. Returns false if it is optimal, true otherwise. */
     bool OptimizeStep() noexcept
     {
-        // Iterate over all transactions (only processing those which are chunk representatives).
+        while (!m_suboptimal_chunks.empty()) {
-        for (auto chunk : m_transaction_idxs) {
+            // Pop an entry from the potentially-suboptimal chunk queue.
+            TxIdx chunk = m_suboptimal_chunks.front();
+            m_suboptimal_chunks.pop_front();
             auto& chunk_data = m_tx_data[chunk];
-            // If this is not a chunk representative, skip.
+            // If what was popped is not currently a chunk representative, continue. This may
+            // happen when a split chunk merges in Improve() with one or more existing chunks that
+            // are themselves on the suboptimal queue already.
             if (chunk_data.chunk_rep != chunk) continue;
             // Iterate over all transactions of the chunk.
             for (auto tx : chunk_data.chunk_setinfo.transactions) {
@@ -1344,6 +1367,14 @@ public:
             assert(dep_data.top_setinfo.feerate ==
                    depgraph.FeeRate(dep_data.top_setinfo.transactions));
         }
+
+        //
+        // Verify m_suboptimal_chunks.
+        //
+        for (size_t i = 0; i < m_suboptimal_chunks.size(); ++i) {
+            auto tx_idx = m_suboptimal_chunks[i];
+            assert(m_transaction_idxs[tx_idx]);
+        }
     }
 };
 
@@ -1377,11 +1408,14 @@ std::tuple<std::vector<DepGraphIndex>, bool, uint64_t> Linearize(const DepGraph<
     // Make improvement steps to it until we hit the max_iterations limit, or an optimal result
     // is found.
     bool optimal = false;
-    while (forest.GetCost() < max_iterations) {
+    if (forest.GetCost() < max_iterations) {
-        if (!forest.OptimizeStep()) {
+        forest.StartOptimizing();
-            optimal = true;
+        do {
-            break;
+            if (!forest.OptimizeStep()) {
-        }
+                optimal = true;
+                break;
+            }
+        } while (forest.GetCost() < max_iterations);
     }
     return {forest.GetLinearization(), optimal, forest.GetCost()};
 }

src/test/fuzz/cluster_linearize.cpp

@@ -1068,6 +1068,8 @@ FUZZ_TARGET(clusterlin_sfl)
     }
 
     // Loop until optimal.
+    test_fn();
+    sfl.StartOptimizing();
     while (true) {
         test_fn();
         if (!sfl.OptimizeStep()) break;

src/test/util/cluster_linearize.h

@@ -404,12 +404,12 @@ inline uint64_t MaxOptimalLinearizationIters(DepGraphIndex cluster_count)
         0,
         0, 2, 8, 21, 51, 99, 162, 208,
         300, 349, 489, 627, 776, 867, 982, 1204,
-        1414, 1473, 1770, 2045, 2391, 2417, 3669, 3953,
+        1414, 1473, 1770, 2045, 2285, 2417, 3669, 3953,
-        3816, 5717, 4096, 5933, 5225, 5684, 6205, 6407,
+        3816, 5720, 4103, 5934, 5443, 5323, 6338, 6407,
-        7671, 12044, 11799, 9577, 9631, 10819, 12277, 15250,
+        7671, 11625, 11799, 10104, 9631, 11203, 12487, 15262,
-        18609, 14439, 22283, 16461, 22887, 20641, 22009, 22053,
+        17800, 14132, 21915, 16495, 23350, 21304, 22221, 22230,
-        27068, 22173, 31066, 30848, 31841, 37174, 39701, 35666,
+        26119, 22182, 31118, 30848, 32166, 37174, 39708, 36189,
-        42728, 43679, 45719, 40217, 51395, 57796, 72739, 60079
+        42747, 43689, 46555, 39818, 51077, 58489, 72633, 59756
     };
     assert(cluster_count < std::size(ITERS));
     // Multiply the table number by two, to account for the fact that they are not absolutes.