From 75b5d42419ed1286fc9557bc97ec5bac3f9be837 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Wed, 4 Sep 2024 15:14:54 -0400 Subject: [PATCH] clusterlin: make DepGraph::AddDependency support multiple dependencies at once This changes DepGraph::AddDependency into DepGraph::AddDependencies, which takes in a single child, but a set of parent transactions, making them all dependencies at once. This is important for performance. N transactions can have O(N^2) parents combined, so constructing a full DepGraph using just AddDependency (which is O(N) on its own) could take O(N^3) time, while doing the same with AddDependencies (also O(N) on its own) only takes O(N^2). Notably, this matters for DepGraphFormatter::Unser, which goes from O(N^3) to O(N^2). Co-Authored-By: Greg Sanders --- src/bench/cluster_linearize.cpp | 24 ++++----- src/cluster_linearize.h | 70 +++++++++---------------- src/test/fuzz/cluster_linearize.cpp | 80 +++++++++++++++++++++-------- src/test/util/cluster_linearize.h | 2 +- 4 files changed, 96 insertions(+), 80 deletions(-) diff --git a/src/bench/cluster_linearize.cpp b/src/bench/cluster_linearize.cpp index cf071dda2d1..7d011975ddb 100644 --- a/src/bench/cluster_linearize.cpp +++ b/src/bench/cluster_linearize.cpp @@ -28,7 +28,7 @@ DepGraph MakeLinearGraph(ClusterIndex ntx) DepGraph depgraph; for (ClusterIndex i = 0; i < ntx; ++i) { depgraph.AddTransaction({-int32_t(i), 1}); - if (i > 0) depgraph.AddDependency(i - 1, i); + if (i > 0) depgraph.AddDependencies(SetType::Singleton(i - 1), i); } return depgraph; } @@ -43,7 +43,7 @@ DepGraph MakeWideGraph(ClusterIndex ntx) DepGraph depgraph; for (ClusterIndex i = 0; i < ntx; ++i) { depgraph.AddTransaction({int32_t(i) + 1, 1}); - if (i > 0) depgraph.AddDependency(0, i); + if (i > 0) depgraph.AddDependencies(SetType::Singleton(0), i); } return depgraph; } @@ -70,19 +70,19 @@ DepGraph MakeHardGraph(ClusterIndex ntx) depgraph.AddTransaction({1, 2}); } else if (i == 1) { depgraph.AddTransaction({14, 2}); - depgraph.AddDependency(0, 1); + depgraph.AddDependencies(SetType::Singleton(0), 1); } else if (i == 2) { depgraph.AddTransaction({6, 1}); - depgraph.AddDependency(2, 1); + depgraph.AddDependencies(SetType::Singleton(2), 1); } else if (i == 3) { depgraph.AddTransaction({5, 1}); - depgraph.AddDependency(2, 3); + depgraph.AddDependencies(SetType::Singleton(2), 3); } else if ((i & 1) == 0) { depgraph.AddTransaction({7, 1}); - depgraph.AddDependency(i - 1, i); + depgraph.AddDependencies(SetType::Singleton(i - 1), i); } else { depgraph.AddTransaction({5, 1}); - depgraph.AddDependency(i, 4); + depgraph.AddDependencies(SetType::Singleton(i), 4); } } else { // Even cluster size. @@ -98,16 +98,16 @@ DepGraph MakeHardGraph(ClusterIndex ntx) depgraph.AddTransaction({1, 1}); } else if (i == 1) { depgraph.AddTransaction({3, 1}); - depgraph.AddDependency(0, 1); + depgraph.AddDependencies(SetType::Singleton(0), 1); } else if (i == 2) { depgraph.AddTransaction({1, 1}); - depgraph.AddDependency(0, 2); + depgraph.AddDependencies(SetType::Singleton(0), 2); } else if (i & 1) { depgraph.AddTransaction({4, 1}); - depgraph.AddDependency(i - 1, i); + depgraph.AddDependencies(SetType::Singleton(i - 1), i); } else { depgraph.AddTransaction({0, 1}); - depgraph.AddDependency(i, 3); + depgraph.AddDependencies(SetType::Singleton(i), 3); } } } @@ -195,7 +195,7 @@ void BenchMergeLinearizationsWorstCase(ClusterIndex ntx, benchmark::Bench& bench DepGraph depgraph; for (ClusterIndex i = 0; i < ntx; ++i) { depgraph.AddTransaction({i, 1}); - if (i) depgraph.AddDependency(0, i); + if (i) depgraph.AddDependencies(SetType::Singleton(0), i); } std::vector lin1; std::vector lin2; diff --git a/src/cluster_linearize.h b/src/cluster_linearize.h index 8467f3f08be..bcc455874d7 100644 --- a/src/cluster_linearize.h +++ b/src/cluster_linearize.h @@ -86,35 +86,13 @@ public: * * Complexity: O(N^2) where N=cluster.size(). */ - explicit DepGraph(const Cluster& cluster) noexcept : entries(cluster.size()) + explicit DepGraph(const Cluster& cluster) noexcept : DepGraph(cluster.size()) { for (ClusterIndex i = 0; i < cluster.size(); ++i) { // Fill in fee and size. entries[i].feerate = cluster[i].first; - // Fill in direct parents as ancestors. - entries[i].ancestors = cluster[i].second; - // Make sure transactions are ancestors of themselves. - entries[i].ancestors.Set(i); - } - - // Propagate ancestor information. - for (ClusterIndex i = 0; i < entries.size(); ++i) { - // At this point, entries[a].ancestors[b] is true iff b is an ancestor of a and there - // is a path from a to b through the subgraph consisting of {a, b} union - // {0, 1, ..., (i-1)}. - SetType to_merge = entries[i].ancestors; - for (ClusterIndex j = 0; j < entries.size(); ++j) { - if (entries[j].ancestors[i]) { - entries[j].ancestors |= to_merge; - } - } - } - - // Fill in descendant information by transposing the ancestor information. - for (ClusterIndex i = 0; i < entries.size(); ++i) { - for (auto j : entries[i].ancestors) { - entries[j].descendants.Set(i); - } + // Fill in dependencies. + AddDependencies(cluster[i].second, i); } } @@ -122,21 +100,16 @@ public: * * Complexity: O(N^2) where N=depgraph.TxCount(). */ - DepGraph(const DepGraph& depgraph, Span mapping) noexcept : entries(depgraph.TxCount()) + DepGraph(const DepGraph& depgraph, Span mapping) noexcept : DepGraph(depgraph.TxCount()) { Assert(mapping.size() == depgraph.TxCount()); - // Fill in fee, size, ancestors. for (ClusterIndex i = 0; i < depgraph.TxCount(); ++i) { - const auto& input = depgraph.entries[i]; - auto& output = entries[mapping[i]]; - output.feerate = input.feerate; - for (auto j : input.ancestors) output.ancestors.Set(mapping[j]); - } - // Fill in descendant information. - for (ClusterIndex i = 0; i < entries.size(); ++i) { - for (auto j : entries[i].ancestors) { - entries[j].descendants.Set(i); - } + // Fill in fee and size. + entries[mapping[i]].feerate = depgraph.entries[i].feerate; + // Fill in dependencies by mapping direct parents. + SetType parents; + for (auto j : depgraph.GetReducedParents(i)) parents.Set(mapping[j]); + AddDependencies(parents, mapping[i]); } } @@ -164,21 +137,26 @@ public: return new_idx; } - /** Modify this transaction graph, adding a dependency between a specified parent and child. + /** Modify this transaction graph, adding multiple parents to a specified child. * * Complexity: O(N) where N=TxCount(). - **/ - void AddDependency(ClusterIndex parent, ClusterIndex child) noexcept + */ + void AddDependencies(const SetType& parents, ClusterIndex child) noexcept { - // Bail out if dependency is already implied. - if (entries[child].ancestors[parent]) return; - // To each ancestor of the parent, add as descendants the descendants of the child. + // Compute the ancestors of parents that are not already ancestors of child. + SetType par_anc; + for (auto par : parents - Ancestors(child)) { + par_anc |= Ancestors(par); + } + par_anc -= Ancestors(child); + // Bail out if there are no such ancestors. + if (par_anc.None()) return; + // To each such ancestor, add as descendants the descendants of the child. const auto& chl_des = entries[child].descendants; - for (auto anc_of_par : Ancestors(parent)) { + for (auto anc_of_par : par_anc) { entries[anc_of_par].descendants |= chl_des; } - // To each descendant of the child, add as ancestors the ancestors of the parent. - const auto& par_anc = entries[parent].ancestors; + // To each descendant of the child, add those ancestors. for (auto dec_of_chl : Descendants(child)) { entries[dec_of_chl].ancestors |= par_anc; } diff --git a/src/test/fuzz/cluster_linearize.cpp b/src/test/fuzz/cluster_linearize.cpp index 4976afbaad8..bf7db4482c6 100644 --- a/src/test/fuzz/cluster_linearize.cpp +++ b/src/test/fuzz/cluster_linearize.cpp @@ -3,6 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include #include #include #include @@ -176,7 +177,7 @@ void MakeConnected(DepGraph& depgraph) while (todo.Any()) { auto nextcomp = depgraph.FindConnectedComponent(todo); Assume(depgraph.IsConnected(nextcomp)); - depgraph.AddDependency(comp.Last(), nextcomp.First()); + depgraph.AddDependencies(BS::Singleton(comp.Last()), nextcomp.First()); todo -= nextcomp; comp = nextcomp; } @@ -240,32 +241,65 @@ std::vector ReadLinearization(const DepGraph& depgraph, SpanRe } // namespace -FUZZ_TARGET(clusterlin_add_dependency) +FUZZ_TARGET(clusterlin_add_dependencies) { - // Verify that computing a DepGraph from a cluster, or building it step by step using AddDependency - // have the same effect. + // Verify that computing a DepGraph from a cluster, or building it step by step using + // AddDependencies has the same effect. + + FuzzedDataProvider provider(buffer.data(), buffer.size()); + auto rng_seed = provider.ConsumeIntegral(); + InsecureRandomContext rng(rng_seed); // Construct a cluster of a certain length, with no dependencies. - FuzzedDataProvider provider(buffer.data(), buffer.size()); - auto num_tx = provider.ConsumeIntegralInRange(2, 32); + auto num_tx = provider.ConsumeIntegralInRange(2, TestBitSet::Size()); Cluster cluster(num_tx, std::pair{FeeFrac{0, 1}, TestBitSet{}}); // Construct the corresponding DepGraph object (also no dependencies). - DepGraph depgraph(cluster); - SanityCheck(depgraph); - // Read (parent, child) pairs, and add them to the cluster and depgraph. - LIMITED_WHILE(provider.remaining_bytes() > 0, TestBitSet::Size() * TestBitSet::Size()) { - auto parent = provider.ConsumeIntegralInRange(0, num_tx - 1); - auto child = provider.ConsumeIntegralInRange(0, num_tx - 2); - child += (child >= parent); - cluster[child].second.Set(parent); - depgraph.AddDependency(parent, child); - assert(depgraph.Ancestors(child)[parent]); - assert(depgraph.Descendants(parent)[child]); + DepGraph depgraph_batch(cluster); + SanityCheck(depgraph_batch); + // Read (parents, child) pairs, and add the dependencies to the cluster and depgraph. + std::vector> deps_list; + LIMITED_WHILE(provider.remaining_bytes() > 0, TestBitSet::Size()) { + const auto parents_mask = provider.ConsumeIntegralInRange(0, (uint64_t{1} << num_tx) - 1); + auto child = provider.ConsumeIntegralInRange(0, num_tx - 1); + + auto parents_mask_shifted = parents_mask; + TestBitSet deps; + for (ClusterIndex i = 0; i < num_tx; ++i) { + if (parents_mask_shifted & 1) { + deps.Set(i); + cluster[child].second.Set(i); + } + parents_mask_shifted >>= 1; + } + assert(parents_mask_shifted == 0); + depgraph_batch.AddDependencies(deps, child); + for (auto i : deps) { + deps_list.emplace_back(i, child); + assert(depgraph_batch.Ancestors(child)[i]); + assert(depgraph_batch.Descendants(i)[child]); + } } // Sanity check the result. - SanityCheck(depgraph); + SanityCheck(depgraph_batch); // Verify that the resulting DepGraph matches one recomputed from the cluster. - assert(DepGraph(cluster) == depgraph); + assert(DepGraph(cluster) == depgraph_batch); + + DepGraph depgraph_individual; + // Add all transactions to depgraph_individual. + for (const auto& [feerate, parents] : cluster) { + depgraph_individual.AddTransaction(feerate); + } + SanityCheck(depgraph_individual); + // Add all individual dependencies to depgraph_individual in randomized order. + std::shuffle(deps_list.begin(), deps_list.end(), rng); + for (auto [parent, child] : deps_list) { + depgraph_individual.AddDependencies(TestBitSet::Singleton(parent), child); + assert(depgraph_individual.Ancestors(child)[parent]); + assert(depgraph_individual.Descendants(parent)[child]); + } + // Sanity check and compare again the batched version. + SanityCheck(depgraph_individual); + assert(depgraph_individual == depgraph_batch); } FUZZ_TARGET(clusterlin_cluster_serialization) @@ -897,12 +931,16 @@ FUZZ_TARGET(clusterlin_postlinearize_tree) if (direction & 1) { for (ClusterIndex i = 0; i < depgraph_gen.TxCount(); ++i) { auto children = depgraph_gen.GetReducedChildren(i); - if (children.Any()) depgraph_tree.AddDependency(i, children.First()); + if (children.Any()) { + depgraph_tree.AddDependencies(TestBitSet::Singleton(i), children.First()); + } } } else { for (ClusterIndex i = 0; i < depgraph_gen.TxCount(); ++i) { auto parents = depgraph_gen.GetReducedParents(i); - if (parents.Any()) depgraph_tree.AddDependency(parents.First(), i); + if (parents.Any()) { + depgraph_tree.AddDependencies(TestBitSet::Singleton(parents.First()), i); + } } } diff --git a/src/test/util/cluster_linearize.h b/src/test/util/cluster_linearize.h index a9ae4ff12a8..bec4d41d385 100644 --- a/src/test/util/cluster_linearize.h +++ b/src/test/util/cluster_linearize.h @@ -234,7 +234,7 @@ struct DepGraphFormatter if (new_feerate.IsEmpty()) break; assert(reordering.size() < SetType::Size()); auto topo_idx = topo_depgraph.AddTransaction(new_feerate); - for (auto parent : new_ancestors) topo_depgraph.AddDependency(parent, topo_idx); + topo_depgraph.AddDependencies(new_ancestors, topo_idx); diff %= total_size + 1; // Insert the new transaction at distance diff back from the end. for (auto& pos : reordering) {