From 34a77138b7a6f65a31c6273f2770e943cc0b474b Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Wed, 15 Oct 2025 22:52:52 -0400 Subject: [PATCH] txgraph: permit non-topological clusters to defer fixing (optimization) --- src/txgraph.cpp | 88 ++++++++++++++++++++++++++++++------------------- 1 file changed, 55 insertions(+), 33 deletions(-) diff --git a/src/txgraph.cpp b/src/txgraph.cpp index 368b5cf7fb4..d76ee60e032 100644 --- a/src/txgraph.cpp +++ b/src/txgraph.cpp @@ -40,8 +40,12 @@ enum class QualityLevel /** This is a singleton cluster consisting of a transaction that individually exceeds the * cluster size limit. It cannot be merged with anything. */ OVERSIZED_SINGLETON, + /** This cluster may have multiple disconnected components, which are all NEEDS_FIX. */ + NEEDS_SPLIT_FIX, /** This cluster may have multiple disconnected components, which are all NEEDS_RELINEARIZE. */ NEEDS_SPLIT, + /** This cluster may be non-topological. */ + NEEDS_FIX, /** This cluster has undergone changes that warrant re-linearization. */ NEEDS_RELINEARIZE, /** The minimal level of linearization has been performed, but it is not known to be optimal. */ @@ -130,6 +134,11 @@ public: { return m_quality == QualityLevel::ACCEPTABLE || m_quality == QualityLevel::OPTIMAL; } + /** Whether the linearization of this Cluster is topological. */ + bool IsTopological() const noexcept + { + return m_quality != QualityLevel::NEEDS_FIX && m_quality != QualityLevel::NEEDS_SPLIT_FIX; + } /** Whether the linearization of this Cluster is optimal. */ bool IsOptimal() const noexcept { @@ -142,7 +151,7 @@ public: /** Whether this cluster requires splitting. */ bool NeedsSplitting() const noexcept { - return m_quality == QualityLevel::NEEDS_SPLIT; + return m_quality == QualityLevel::NEEDS_SPLIT || m_quality == QualityLevel::NEEDS_SPLIT_FIX; } /** Get the smallest number of transactions this Cluster is intended for. */ @@ -165,8 +174,9 @@ public: virtual DepGraphIndex AppendTransaction(GraphIndex graph_idx, FeePerWeight feerate) noexcept = 0; /** Add dependencies to a given child in this cluster. */ virtual void AddDependencies(SetType parents, DepGraphIndex child) noexcept = 0; - /** Invoke visitor_fn for each transaction in the cluster, in linearization order, then wipe this Cluster. */ - virtual void ExtractTransactions(const std::function& visit_fn) noexcept = 0; + /** Invoke visit1_fn for each transaction in the cluster, in linearization order, then + * visit2_fn in the same order, then wipe this Cluster. */ + virtual void ExtractTransactions(const std::function& visit1_fn, const std::function& visit2_fn) noexcept = 0; /** Figure out what level this Cluster exists at in the graph. In most cases this is known by * the caller already (see all "int level" arguments below), but not always. */ virtual int GetLevel(const TxGraphImpl& graph) const noexcept = 0; @@ -266,7 +276,7 @@ public: GraphIndex GetClusterEntry(DepGraphIndex index) const noexcept final { return m_mapping[index]; } DepGraphIndex AppendTransaction(GraphIndex graph_idx, FeePerWeight feerate) noexcept final; void AddDependencies(SetType parents, DepGraphIndex child) noexcept final; - void ExtractTransactions(const std::function& visit_fn) noexcept final; + void ExtractTransactions(const std::function& visit1_fn, const std::function& visit2_fn) noexcept final; int GetLevel(const TxGraphImpl& graph) const noexcept final; void UpdateMapping(DepGraphIndex cluster_idx, GraphIndex graph_idx) noexcept final { m_mapping[cluster_idx] = graph_idx; } void Updated(TxGraphImpl& graph, int level) noexcept final; @@ -322,7 +332,7 @@ public: GraphIndex GetClusterEntry(DepGraphIndex index) const noexcept final { Assume(index == 0); Assume(GetTxCount()); return m_graph_index; } DepGraphIndex AppendTransaction(GraphIndex graph_idx, FeePerWeight feerate) noexcept final; void AddDependencies(SetType parents, DepGraphIndex child) noexcept final; - void ExtractTransactions(const std::function& visit_fn) noexcept final; + void ExtractTransactions(const std::function& visit1_fn, const std::function& visit2_fn) noexcept final; int GetLevel(const TxGraphImpl& graph) const noexcept final; void UpdateMapping(DepGraphIndex cluster_idx, GraphIndex graph_idx) noexcept final { Assume(cluster_idx == 0); m_graph_index = graph_idx; } void Updated(TxGraphImpl& graph, int level) noexcept final; @@ -913,10 +923,13 @@ void SingletonClusterImpl::AddDependencies(SetType parents, DepGraphIndex child) Assume(parents == SetType{} || parents == SetType::Fill(0)); } -void GenericClusterImpl::ExtractTransactions(const std::function& visit_fn) noexcept +void GenericClusterImpl::ExtractTransactions(const std::function& visit1_fn, const std::function& visit2_fn) noexcept { for (auto pos : m_linearization) { - visit_fn(pos, m_mapping[pos], FeePerWeight::FromFeeFrac(m_depgraph.FeeRate(pos)), m_depgraph.GetReducedParents(pos)); + visit1_fn(pos, m_mapping[pos], FeePerWeight::FromFeeFrac(m_depgraph.FeeRate(pos))); + } + for (auto pos : m_linearization) { + visit2_fn(pos, m_mapping[pos], m_depgraph.GetReducedParents(pos)); } // Purge this Cluster, now that everything has been moved. m_depgraph = DepGraph{}; @@ -924,10 +937,11 @@ void GenericClusterImpl::ExtractTransactions(const std::function& visit_fn) noexcept +void SingletonClusterImpl::ExtractTransactions(const std::function& visit1_fn, const std::function& visit2_fn) noexcept { if (GetTxCount()) { - visit_fn(0, m_graph_index, m_feerate, SetType{}); + visit1_fn(0, m_graph_index, m_feerate); + visit2_fn(0, m_graph_index, SetType{}); m_graph_index = NO_GRAPH_INDEX; } } @@ -1188,7 +1202,8 @@ void GenericClusterImpl::ApplyRemovals(TxGraphImpl& graph, int level, std::span< Compact(); graph.GetClusterSet(level).m_cluster_usage += TotalMemoryUsage(); - graph.SetClusterQuality(level, m_quality, m_setindex, QualityLevel::NEEDS_SPLIT); + auto new_quality = IsTopological() ? QualityLevel::NEEDS_SPLIT : QualityLevel::NEEDS_SPLIT_FIX; + graph.SetClusterQuality(level, m_quality, m_setindex, new_quality); Updated(graph, level); } @@ -1290,6 +1305,7 @@ void SingletonClusterImpl::AppendChunkFeerates(std::vector& ret) const uint64_t GenericClusterImpl::AppendTrimData(std::vector& ret, std::vector>& deps) const noexcept { + Assume(IsAcceptable()); auto linchunking = ChunkLinearizationInfo(m_depgraph, m_linearization); LinearizationIndex pos{0}; uint64_t size{0}; @@ -1333,7 +1349,7 @@ bool GenericClusterImpl::Split(TxGraphImpl& graph, int level) noexcept // This function can only be called when the Cluster needs splitting. Assume(NeedsSplitting()); // Determine the new quality the split-off Clusters will have. - QualityLevel new_quality = QualityLevel::NEEDS_RELINEARIZE; + QualityLevel new_quality = IsTopological() ? QualityLevel::NEEDS_RELINEARIZE : QualityLevel::NEEDS_FIX; /** Which positions are still left in this Cluster. */ auto todo = m_depgraph.Positions(); /** Mapping from transaction positions in this Cluster to the Cluster where it ends up, and @@ -1345,7 +1361,7 @@ bool GenericClusterImpl::Split(TxGraphImpl& graph, int level) noexcept while (todo.Any()) { auto component = m_depgraph.FindConnectedComponent(todo); auto component_size = component.Count(); - auto split_quality = component_size <= 2 ? QualityLevel::OPTIMAL : new_quality; + auto split_quality = component_size <= 1 ? QualityLevel::OPTIMAL : new_quality; if (first && component == todo && SetType::Fill(component_size) == component && component_size >= MIN_INTENDED_TX_COUNT) { // The existing Cluster is an entire component, without holes. Leave it be, but update // its quality. If there are holes, we continue, so that the Cluster is reconstructed @@ -1415,7 +1431,7 @@ void GenericClusterImpl::Merge(TxGraphImpl& graph, int level, Cluster& other) no /** Vector to store the positions in this Cluster for each position in other. */ std::vector remap(other.GetDepGraphIndexRange()); // Iterate over all transactions in the other Cluster (the one being absorbed). - other.ExtractTransactions([&](DepGraphIndex pos, GraphIndex idx, FeePerWeight feerate, SetType other_parents) noexcept { + other.ExtractTransactions([&](DepGraphIndex pos, GraphIndex idx, FeePerWeight feerate) noexcept { // Copy the transaction into this Cluster, and remember its position. auto new_pos = m_depgraph.AddTransaction(feerate); // Since this cluster must have been made hole-free before being merged into, all added @@ -1424,9 +1440,8 @@ void GenericClusterImpl::Merge(TxGraphImpl& graph, int level, Cluster& other) no remap[pos] = new_pos; m_mapping.push_back(idx); m_linearization.push_back(new_pos); - // Copy the transaction's dependencies, translating them using remap. Note that since - // pos iterates in linearization order, which is topological, all parents of pos should - // already be in remap. + }, [&](DepGraphIndex pos, GraphIndex idx, SetType other_parents) noexcept { + // Copy the transaction's dependencies, translating them using remap. SetType parents; for (auto par : other_parents) { parents.Set(remap[par]); @@ -1439,7 +1454,7 @@ void GenericClusterImpl::Merge(TxGraphImpl& graph, int level, Cluster& other) no // Discard any potential ChunkData prior to modifying the Cluster (as that could // invalidate its ordering). if (level == 0) graph.ClearChunkData(entry); - entry.m_locator[level].SetPresent(this, new_pos); + entry.m_locator[level].SetPresent(this, remap[pos]); }); } @@ -1475,14 +1490,11 @@ void GenericClusterImpl::ApplyDependencies(TxGraphImpl& graph, int level, std::s m_depgraph.AddDependencies(parents, child_idx); } - // Finally fix the linearization, as the new dependencies may have invalidated the - // linearization. - FixLinearization(m_depgraph, m_linearization); + // Finally set the cluster to NEEDS_FIX, so its linearization is fixed the next time it is + // attempted to be made ACCEPTABLE. Assume(!NeedsSplitting()); Assume(!IsOversized()); - if (IsAcceptable()) { - graph.SetClusterQuality(level, m_quality, m_setindex, QualityLevel::NEEDS_RELINEARIZE); - } + graph.SetClusterQuality(level, m_quality, m_setindex, QualityLevel::NEEDS_FIX); // Finally push the changes to graph.m_entries. Updated(graph, level); @@ -1731,9 +1743,11 @@ void TxGraphImpl::SplitAll(int up_to_level) noexcept // Before splitting all Cluster, first make sure all removals are applied. ApplyRemovals(up_to_level); for (int level = 0; level <= up_to_level; ++level) { - auto& queue = GetClusterSet(level).m_clusters[int(QualityLevel::NEEDS_SPLIT)]; - while (!queue.empty()) { - Split(*queue.back().get(), level); + for (auto quality : {QualityLevel::NEEDS_SPLIT_FIX, QualityLevel::NEEDS_SPLIT}) { + auto& queue = GetClusterSet(level).m_clusters[int(quality)]; + while (!queue.empty()) { + Split(*queue.back().get(), level); + } } } } @@ -2047,7 +2061,7 @@ std::pair GenericClusterImpl::Relinearize(TxGraphImpl& graph, in if (IsOptimal()) return {0, false}; // Invoke the actual linearization algorithm (passing in the existing one). uint64_t rng_seed = graph.m_rng.rand64(); - auto [linearization, optimal, cost] = Linearize(m_depgraph, max_iters, rng_seed, m_linearization); + auto [linearization, optimal, cost] = Linearize(m_depgraph, max_iters, rng_seed, m_linearization, /*is_topological=*/IsTopological()); // Postlinearize to undo some of the non-determinism caused by randomizing the linearization. // This also guarantees that all chunks are connected (which is not guaranteed by SFL // currently, even when optimal). @@ -2062,6 +2076,9 @@ std::pair GenericClusterImpl::Relinearize(TxGraphImpl& graph, in } else if (max_iters >= graph.m_acceptable_iters && !IsAcceptable()) { graph.SetClusterQuality(level, m_quality, m_setindex, QualityLevel::ACCEPTABLE); improved = true; + } else if (!IsTopological()) { + graph.SetClusterQuality(level, m_quality, m_setindex, QualityLevel::NEEDS_RELINEARIZE); + improved = true; } // Update the Entry objects. Updated(graph, level); @@ -2089,9 +2106,11 @@ void TxGraphImpl::MakeAllAcceptable(int level) noexcept ApplyDependencies(level); auto& clusterset = GetClusterSet(level); if (clusterset.m_oversized == true) return; - auto& queue = clusterset.m_clusters[int(QualityLevel::NEEDS_RELINEARIZE)]; - while (!queue.empty()) { - MakeAcceptable(*queue.back().get(), level); + for (auto quality : {QualityLevel::NEEDS_FIX, QualityLevel::NEEDS_RELINEARIZE}) { + auto& queue = clusterset.m_clusters[int(quality)]; + while (!queue.empty()) { + MakeAcceptable(*queue.back().get(), level); + } } } @@ -2727,7 +2746,9 @@ void GenericClusterImpl::SanityCheck(const TxGraphImpl& graph, int level) const const auto& entry = graph.m_entries[m_mapping[lin_pos]]; // Check that the linearization is topological. m_done.Set(lin_pos); - assert(m_done.IsSupersetOf(m_depgraph.Ancestors(lin_pos))); + if (IsTopological()) { + assert(m_done.IsSupersetOf(m_depgraph.Ancestors(lin_pos))); + } // Check that the Entry has a locator pointing back to this Cluster & position within it. assert(entry.m_locator[level].cluster == this); assert(entry.m_locator[level].index == lin_pos); @@ -2966,7 +2987,7 @@ bool TxGraphImpl::DoWork(uint64_t iters) noexcept uint64_t iters_done{0}; // First linearize everything in NEEDS_RELINEARIZE to an acceptable level. If more budget // remains after that, try to make everything optimal. - for (QualityLevel quality : {QualityLevel::NEEDS_RELINEARIZE, QualityLevel::ACCEPTABLE}) { + for (QualityLevel quality : {QualityLevel::NEEDS_FIX, QualityLevel::NEEDS_RELINEARIZE, QualityLevel::ACCEPTABLE}) { // First linearize staging, if it exists, then main. for (int level = GetTopLevel(); level >= 0; --level) { // Do not modify main if it has any observers. @@ -2983,7 +3004,7 @@ bool TxGraphImpl::DoWork(uint64_t iters) noexcept // one. auto pos = m_rng.randrange(queue.size()); auto iters_now = iters - iters_done; - if (quality == QualityLevel::NEEDS_RELINEARIZE) { + if (quality == QualityLevel::NEEDS_FIX || quality == QualityLevel::NEEDS_RELINEARIZE) { // If we're working with clusters that need relinearization still, only perform // up to m_acceptable_iters iterations. If they become ACCEPTABLE, and we still // have budget after all other clusters are ACCEPTABLE too, we'll spend the @@ -3245,6 +3266,7 @@ std::vector TxGraphImpl::Trim() noexcept .subspan(group_data.m_cluster_offset, group_data.m_cluster_count); uint64_t size{0}; for (Cluster* cluster : cluster_span) { + MakeAcceptable(*cluster, cluster->GetLevel(*this)); size += cluster->AppendTrimData(trim_data, deps_by_child); } // If this group of Clusters does not violate any limits, continue to the next group.