clusterlin: simplify PickMergeCandidate (optimization)
What changed, and why it matters
This is a routine code cleanup and performance tweak inside Bitcoin Core's transaction-cluster linearization logic. It rewrites how the algorithm picks which chunk of transactions to merge next, making the code simpler and slightly faster. There is no indication of a security bug being fixed.
No security action required. Treat as normal code-quality/performance improvement and review through standard peer review.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit refactors PickMergeCandidate in src/cluster_linearize.h. Previously the function iterated per-transaction and tracked newly-reached parents/children incrementally. The patch introduces a helper GetReachable
Changed components
src/cluster_linearize.h - cluster linearization merge-candidate selectionsrc/test/util/cluster_linearize.h - test iteration bounds tableInspect captured patch +45 / −34
diff --git a/src/cluster_linearize.h b/src/cluster_linearize.h
index 894a3dcf..de5c6b20 100644
--- a/src/cluster_linearize.h
+++ b/src/cluster_linearize.h
@@ -742,6 +742,22 @@ private:
}
}
+ /** Find the set of out-of-chunk transactions reachable from tx_idxs. */
+ template<bool DownWard>
+ SetType GetReachable(const SetType& tx_idxs) const noexcept
+ {
+ SetType ret;
+ for (auto tx_idx : tx_idxs) {
+ const auto& tx_data = m_tx_data[tx_idx];
+ if constexpr (DownWard) {
+ ret |= tx_data.children;
+ } else {
+ ret |= tx_data.parents;
+ }
+ }
+ return ret - tx_idxs;
+ }
+
/** Make the inactive dependency from child to parent, which must not be in the same chunk
* already, active. Returns the merged chunk idx. */
SetIdx Activate(TxIdx parent_idx, TxIdx child_idx) noexcept
@@ -883,16 +899,11 @@ private:
/** Information about the chunk. */
Assume(m_chunk_idxs[chunk_idx]);
auto& chunk_info = m_set_info[chunk_idx];
- SetType chunk_txn = chunk_info.transactions;
- // Iterate over all transactions in the chunk, figuring out which other chunk each
- // depends on, but only testing each other chunk once. For those depended-on chunks,
+ // Iterate over all chunks reachable from this one. For those depended-on chunks,
// remember the highest-feerate (if DownWard) or lowest-feerate (if !DownWard) one.
// If multiple equal-feerate candidate chunks to merge with exist, pick a random one
// among them.
- /** Which transactions have been reached from this chunk already. Initialize with the
- * chunk itself, so internal dependencies within the chunk are ignored. */
- SetType explored = chunk_txn;
/** The minimum feerate (if downward) or maximum feerate (if upward) to consider when
* looking for candidate chunks to merge with. Initially, this is the original chunk's
* feerate, but is updated to be the current best candidate whenever one is found. */
@@ -902,29 +913,29 @@ private:
/** We generate random tiebreak values to pick between equal-feerate candidate chunks.
* This variable stores the tiebreak of the current best candidate. */
uint64_t best_other_chunk_tiebreak{0};
- for (auto tx_idx : chunk_txn) {
- auto& tx_data = m_tx_data[tx_idx];
- /** The transactions reached by following dependencies from tx that have not been
- * explored before. */
- auto newly_reached = (DownWard ? tx_data.children : tx_data.parents) - explored;
- explored |= newly_reached;
- while (newly_reached.Any()) {
- // Find a chunk inside newly_reached, and remove it from newly_reached.
- auto reached_chunk_idx = m_tx_data[newly_reached.First()].chunk_idx;
- auto& reached_chunk_info = m_set_info[reached_chunk_idx];
- newly_reached -= reached_chunk_info.transactions;
- // See if it has an acceptable feerate.
- auto cmp = DownWard ? FeeRateCompare(best_other_chunk_feerate, reached_chunk_info.feerate)
- : FeeRateCompare(reached_chunk_info.feerate, best_other_chunk_feerate);
- if (cmp > 0) continue;
- uint64_t tiebreak = m_rng.rand64();
- if (cmp < 0 || tiebreak >= best_other_chunk_tiebreak) {
- best_other_chunk_feerate = reached_chunk_info.feerate;
- best_other_chunk_idx = reached_chunk_idx;
- best_other_chunk_tiebreak = tiebreak;
- }
+
+ /** Which parent/child transactions we still need to process the chunks for. */
+ auto todo = GetReachable<DownWard>(chunk_info.transactions);
+ unsigned steps = 0;
+ while (todo.Any()) {
+ ++steps;
+ // Find a chunk for a transaction in todo, and remove all its transactions from todo.
+ auto reached_chunk_idx = m_tx_data[todo.First()].chunk_idx;
+ auto& reached_chunk_info = m_set_info[reached_chunk_idx];
+ todo -= reached_chunk_info.transactions;
+ // See if it has an acceptable feerate.
+ auto cmp = DownWard ? FeeRateCompare(best_other_chunk_feerate, reached_chunk_info.feerate)
+ : FeeRateCompare(reached_chunk_info.feerate, best_other_chunk_feerate);
+ if (cmp > 0) continue;
+ uint64_t tiebreak = m_rng.rand64();
+ if (cmp < 0 || tiebreak >= best_other_chunk_tiebreak) {
+ best_other_chunk_feerate = reached_chunk_info.feerate;
+ best_other_chunk_idx = reached_chunk_idx;
+ best_other_chunk_tiebreak = tiebreak;
}
}
+ Assume(steps <= m_set_info.size());
+
return best_other_chunk_idx;
}
diff --git a/src/test/util/cluster_linearize.h b/src/test/util/cluster_linearize.h
index 7e98312c..f14b00d5 100644
--- a/src/test/util/cluster_linearize.h
+++ b/src/test/util/cluster_linearize.h
@@ -403,13 +403,13 @@ inline uint64_t MaxOptimalLinearizationIters(DepGraphIndex cluster_count)
static constexpr uint64_t ITERS[65] = {
0,
0, 4, 10, 34, 76, 156, 229, 380,
- 432, 607, 738, 896, 1037, 1366, 1464, 1711,
- 2060, 2542, 3068, 3116, 4029, 3467, 5324, 5402,
- 6481, 7161, 7441, 8329, 8843, 9353, 11104, 11269,
- 11791, 11981, 12413, 14259, 15331, 12397, 13581, 18569,
- 18737, 16581, 23217, 23271, 27350, 28591, 33636, 34486,
- 34414, 26227, 35570, 38045, 40814, 29622, 37793, 32122,
- 35915, 49823, 39722, 43765, 42365, 53620, 59417, 67035
+ 432, 517, 678, 896, 1037, 1366, 1479, 1711,
+ 2060, 2542, 3068, 3116, 4029, 3467, 5324, 5512,
+ 6481, 7161, 7441, 8183, 8843, 9353, 11104, 11269,
+ 12354, 11871, 13367, 14259, 14229, 12397, 13581, 17774,
+ 18737, 16581, 23217, 24044, 29597, 28879, 34069, 34162,
+ 36028, 26227, 34471, 37212, 40814, 29554, 40305, 34019,
+ 36582, 55659, 39994, 41277, 42365, 52822, 60151, 67035
};
assert(cluster_count < std::size(ITERS));
// Multiply the table number by two, to account for the fact that they are not absolutes.
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.