clusterlin: make DepGraph::AddDependency support multiple dependencies at once

sipa · instagibbs · sipa · commit 75b5d42419ed · 2024-10-07T13:47:52.000-04:00
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 &lt;gsanders87@gmail.com&gt;
diff --git a/src/bench/cluster_linearize.cpp b/src/bench/cluster_linearize.cpp
@@ -28,7 +28,7 @@ DepGraph<SetType> MakeLinearGraph(ClusterIndex ntx)
     DepGraph<SetType> 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<SetType> MakeWideGraph(ClusterIndex ntx)
     DepGraph<SetType> 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<SetType> 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<SetType> 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<SetType> 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<ClusterIndex> lin1;
     std::vector<ClusterIndex> lin2;
diff --git a/src/cluster_linearize.h b/src/cluster_linearize.h
@@ -86,57 +86,30 @@ class DepGraph
      *
      * Complexity: O(N^2) where N=cluster.size().
      */
-    explicit DepGraph(const Cluster<SetType>& cluster) noexcept : entries(cluster.size())
+    explicit DepGraph(const Cluster<SetType>& 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);
         }
     }
 
     /** Construct a DepGraph object given another DepGraph and a mapping from old to new.
      *
      * Complexity: O(N^2) where N=depgraph.TxCount().
      */
-    DepGraph(const DepGraph<SetType>& depgraph, Span<const ClusterIndex> mapping) noexcept : entries(depgraph.TxCount())
+    DepGraph(const DepGraph<SetType>& depgraph, Span<const ClusterIndex> 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 @@ class DepGraph
         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
@@ -3,6 +3,7 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <cluster_linearize.h>
+#include <random.h>
 #include <serialize.h>
 #include <streams.h>
 #include <test/fuzz/fuzz.h>
@@ -176,7 +177,7 @@ void MakeConnected(DepGraph<BS>& 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<ClusterIndex> ReadLinearization(const DepGraph<BS>& 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.
 
-    // Construct a cluster of a certain length, with no dependencies.
     FuzzedDataProvider provider(buffer.data(), buffer.size());
-    auto num_tx = provider.ConsumeIntegralInRange<ClusterIndex>(2, 32);
+    auto rng_seed = provider.ConsumeIntegral<uint64_t>();
+    InsecureRandomContext rng(rng_seed);
+
+    // Construct a cluster of a certain length, with no dependencies.
+    auto num_tx = provider.ConsumeIntegralInRange<ClusterIndex>(2, TestBitSet::Size());
     Cluster<TestBitSet> 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<ClusterIndex>(0, num_tx - 1);
-        auto child = provider.ConsumeIntegralInRange<ClusterIndex>(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<std::pair<ClusterIndex, ClusterIndex>> deps_list;
+    LIMITED_WHILE(provider.remaining_bytes() > 0, TestBitSet::Size()) {
+        const auto parents_mask = provider.ConsumeIntegralInRange<uint64_t>(0, (uint64_t{1} << num_tx) - 1);
+        auto child = provider.ConsumeIntegralInRange<ClusterIndex>(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<TestBitSet> 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
@@ -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) {
