diff --git a/src/cluster_linearize.h b/src/cluster_linearize.h index 64610ee751f..4d0d7c55d44 100644 --- a/src/cluster_linearize.h +++ b/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 - * is picked (this will be changed in a later commit). + * - A round-robin queue of chunks to improve is maintained. * * - 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 m_tx_data; /** Information about each dependency. Indexed by DepIdx. */ std::vector m_dep_data; + /** A FIFO of chunk representatives of chunks that may be improved still. */ + VecDeque 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) { - bool done = true; - // Iterate over all transactions (only processing those which are chunk representatives). - for (auto chunk : m_transaction_idxs) { - auto& chunk_data = m_tx_data[chunk]; - // If this is not a chunk representative, skip. - if (chunk_data.chunk_rep != chunk) continue; - // Attempt to merge the chunk upwards. - auto result_up = MergeStep(chunk); - if (result_up != TxIdx(-1)) { - done = false; - continue; - } - // Attempt to merge the chunk downwards. - auto result_down = MergeStep(chunk); - if (result_down != TxIdx(-1)) { - done = false; - continue; - } + for (auto tx : m_transaction_idxs) { + auto& tx_data = m_tx_data[tx]; + if (tx_data.chunk_rep == tx) { + m_suboptimal_chunks.emplace_back(tx); + } + } + while (!m_suboptimal_chunks.empty()) { + // 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 what was popped is not currently a chunk representative, continue. This may + // happen when it was merged with something else since being added. + if (chunk_data.chunk_rep != chunk) continue; + // Attempt to merge the chunk upwards. + auto result_up = MergeStep(chunk); + if (result_up != TxIdx(-1)) { + m_suboptimal_chunks.push_back(result_up); + continue; + } + // Attempt to merge the chunk downwards. + auto result_down = MergeStep(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). - for (auto chunk : m_transaction_idxs) { + while (!m_suboptimal_chunks.empty()) { + // 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, 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.OptimizeStep()) { - optimal = true; - break; - } + if (forest.GetCost() < max_iterations) { + forest.StartOptimizing(); + do { + if (!forest.OptimizeStep()) { + optimal = true; + break; + } + } while (forest.GetCost() < max_iterations); } return {forest.GetLinearization(), optimal, forest.GetCost()}; } diff --git a/src/test/fuzz/cluster_linearize.cpp b/src/test/fuzz/cluster_linearize.cpp index b4d5234421b..fe18407dbb4 100644 --- a/src/test/fuzz/cluster_linearize.cpp +++ b/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; diff --git a/src/test/util/cluster_linearize.h b/src/test/util/cluster_linearize.h index d4126063960..2e73032e2c3 100644 --- a/src/test/util/cluster_linearize.h +++ b/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, - 3816, 5717, 4096, 5933, 5225, 5684, 6205, 6407, - 7671, 12044, 11799, 9577, 9631, 10819, 12277, 15250, - 18609, 14439, 22283, 16461, 22887, 20641, 22009, 22053, - 27068, 22173, 31066, 30848, 31841, 37174, 39701, 35666, - 42728, 43679, 45719, 40217, 51395, 57796, 72739, 60079 + 1414, 1473, 1770, 2045, 2285, 2417, 3669, 3953, + 3816, 5720, 4103, 5934, 5443, 5323, 6338, 6407, + 7671, 11625, 11799, 10104, 9631, 11203, 12487, 15262, + 17800, 14132, 21915, 16495, 23350, 21304, 22221, 22230, + 26119, 22182, 31118, 30848, 32166, 37174, 39708, 36189, + 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.