clusterlin: abstract out functions from MergeStep (refactor)
What changed, and why it matters
This is a pure code cleanup: a single function was split into three smaller, clearer helper functions with no change to what the code actually does. There is no security issue visible in the commit.
No security action needed; treat as ordinary code-quality review.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit refactors MergeStep in src/cluster_linearize.h by extracting the candidate-selection logic into PickMergeCandidate and the directed merge call into MergeChunksDirected. The control flow, conditions, and observable behavior remain identical. An Assume(m_chunk_idxs[chunk_idx]) assertion is added inside PickMergeCandidate, which is a defensive sanity check rather than a functional change. No security-relevant modifications are present.
Changed components
src/cluster_linearize.hInspect captured patch +26 / −11
diff --git a/src/cluster_linearize.h b/src/cluster_linearize.h
index 2b935da6..3864de7e 100644
--- a/src/cluster_linearize.h
+++ b/src/cluster_linearize.h
@@ -864,12 +864,24 @@ private:
return INVALID_SET_IDX;
}
- /** Perform an upward or downward merge step, on the specified chunk. Returns the merged chunk,
- * or INVALID_SET_IDX if no merge took place. */
+ /** Activate a dependency from chunk_idx to merge_chunk_idx (if !DownWard), or a dependency
+ * from merge_chunk_idx to chunk_idx (if DownWard). Return the index of the merged chunk. */
template<bool DownWard>
- SetIdx MergeStep(SetIdx chunk_idx) noexcept
+ SetIdx MergeChunksDirected(SetIdx chunk_idx, SetIdx merge_chunk_idx) noexcept
+ {
+ if constexpr (DownWard) {
+ return MergeChunks(chunk_idx, merge_chunk_idx);
+ } else {
+ return MergeChunks(merge_chunk_idx, chunk_idx);
+ }
+ }
+
+ /** Determine which chunk to merge chunk_idx with, or INVALID_SET_IDX if none. */
+ template<bool DownWard>
+ SetIdx PickMergeCandidate(SetIdx chunk_idx) noexcept
{
/** 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
@@ -913,18 +925,21 @@ private:
}
}
}
- // Stop if there are no candidate chunks to merge with.
- if (best_other_chunk_idx == INVALID_SET_IDX) return INVALID_SET_IDX;
- if constexpr (DownWard) {
- chunk_idx = MergeChunks(chunk_idx, best_other_chunk_idx);
- } else {
- chunk_idx = MergeChunks(best_other_chunk_idx, chunk_idx);
- }
+ return best_other_chunk_idx;
+ }
+
+ /** Perform an upward or downward merge step, on the specified chunk. Returns the merged chunk,
+ * or INVALID_SET_IDX if no merge took place. */
+ template<bool DownWard>
+ SetIdx MergeStep(SetIdx chunk_idx) noexcept
+ {
+ auto merge_chunk_idx = PickMergeCandidate<DownWard>(chunk_idx);
+ if (merge_chunk_idx == INVALID_SET_IDX) return INVALID_SET_IDX;
+ chunk_idx = MergeChunksDirected<DownWard>(chunk_idx, merge_chunk_idx);
Assume(chunk_idx != INVALID_SET_IDX);
return chunk_idx;
}
-
/** Perform an upward or downward merge sequence on the specified transaction. */
template<bool DownWard>
void MergeSequence(TxIdx tx_idx) noexcept
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.