Merge pull request #408 from samolisov/dominator-tree-vector-of-structs

Aggregate predecessor state for Lengauer-Tarjan

Improves performance on several benchmarks although regresses on one.

Author: jeremy-murphy

.clang-format

@@ -5,6 +5,7 @@ AlignEscapedNewlinesLeft: true
 AlwaysBreakAfterDefinitionReturnType: None
 BreakBeforeBraces: Allman
 BreakConstructorInitializersBeforeComma: false
+BreakTemplateDeclarations: Yes
 ColumnLimit: 80
 ConstructorInitializerAllOnOneLineOrOnePerLine: true
 ConstructorInitializerIndentWidth: 0

include/boost/graph/dominator_tree.hpp

@@ -62,6 +62,21 @@ namespace detail
             Tag >(timeMap, v, t);
     }
 
+    // Auxiliary structure of different kinds of predecessors are used to
+    // calculate the semidominators: ancestor, semidominator, and the ancestor
+    // with the lowest semidominator (`best`). Placing these predecessors in a
+    // structure let us organize a "vector of structs" what improves cache
+    // efficiency.
+    template < class Graph >
+    struct vertex_triple
+    {
+        using Vertex = typename graph_traits< Graph >::vertex_descriptor;
+
+        Vertex semi { graph_traits< Graph >::null_vertex() };
+        Vertex ancestor { graph_traits< Graph >::null_vertex() };
+        Vertex best { graph_traits< Graph >::null_vertex() };
+    };
+
     template < class Graph, class IndexMap, class TimeMap, class PredMap,
         class DomTreePredMap >
     class dominator_visitor
@@ -80,13 +95,9 @@ namespace detail
          */
         dominator_visitor(const Graph& g, const Vertex& entry,
             const IndexMap& indexMap, DomTreePredMap domTreePredMap)
-        : semi_(num_vertices(g))
-        , ancestor_(num_vertices(g), graph_traits< Graph >::null_vertex())
-        , samedom_(ancestor_)
-        , best_(semi_)
-        , semiMap_(make_iterator_property_map(semi_.begin(), indexMap))
-        , ancestorMap_(make_iterator_property_map(ancestor_.begin(), indexMap))
-        , bestMap_(make_iterator_property_map(best_.begin(), indexMap))
+        : pred_(num_vertices(g))
+        , predMap_(make_iterator_property_map(pred_.begin(), indexMap))
+        , samedom_(num_vertices(g), graph_traits< Graph >::null_vertex())
         , buckets_(num_vertices(g))
         , bucketMap_(make_iterator_property_map(buckets_.begin(), indexMap))
         , entry_(entry)
@@ -132,18 +143,18 @@ namespace detail
                 if (get(dfnumMap, v) <= get(dfnumMap, n))
                     s2 = v;
                 else
-                    s2 = get(semiMap_, ancestor_with_lowest_semi_(v, dfnumMap));
+                    s2 = get(predMap_, ancestor_with_lowest_semi_(v, dfnumMap))
+                             .semi;
 
                 if (get(dfnumMap, s2) < get(dfnumMap, s))
                     s = s2;
             }
-            put(semiMap_, n, s);
+            auto& pred_of_n = get(predMap_, n);
+            pred_of_n = {s, p, n};
 
             // 2. Calculation of n's dominator is deferred until
             // the path from s to n has been linked into the forest
             get(bucketMap_, s).push_back(n);
-            get(ancestorMap_, n) = p;
-            get(bestMap_, n) = n;
 
             // 3. Now that the path from p to v has been linked into
             // the spanning forest, these lines calculate the dominator of v,
@@ -161,7 +172,7 @@ namespace detail
             {
                 const Vertex v(*buckItr);
                 const Vertex y(ancestor_with_lowest_semi_(v, dfnumMap));
-                if (get(semiMap_, y) == get(semiMap_, v))
+                if (get(predMap_, y).semi == get(predMap_, v).semi)
                     put(domTreePredMap_, v, p);
                 else
                     put(samedomMap, v, y);
@@ -177,24 +188,32 @@ namespace detail
         const Vertex ancestor_with_lowest_semi_(
             const Vertex& v, const TimeMap& dfnumMap)
         {
-            const Vertex a(get(ancestorMap_, v));
+            const Vertex a(get(predMap_, v).ancestor);
+            const auto& pred_of_a = get(predMap_, a);
+
+            auto& pred_of_v = get(predMap_, v);
 
-            if (get(ancestorMap_, a) != graph_traits< Graph >::null_vertex())
+            if (pred_of_a.ancestor != graph_traits< Graph >::null_vertex())
             {
                 const Vertex b(ancestor_with_lowest_semi_(a, dfnumMap));
+                const auto& pred_of_b = get(predMap_, b);
 
-                put(ancestorMap_, v, get(ancestorMap_, a));
+                pred_of_v.ancestor = pred_of_a.ancestor;
 
-                if (get(dfnumMap, get(semiMap_, b))
-                    < get(dfnumMap, get(semiMap_, get(bestMap_, v))))
-                    put(bestMap_, v, b);
+                if (get(dfnumMap, pred_of_b.semi)
+                    < get(dfnumMap, get(predMap_, pred_of_v.best).semi))
+                    pred_of_v.best = b;
             }
 
-            return get(bestMap_, v);
+            return pred_of_v.best;
         }
 
-        std::vector< Vertex > semi_, ancestor_, samedom_, best_;
-        PredMap semiMap_, ancestorMap_, bestMap_;
+        std::vector< vertex_triple< Graph > > pred_;
+        iterator_property_map< typename std::vector< vertex_triple< Graph > >::iterator,
+            IndexMap >
+            predMap_;
+
+        std::vector< Vertex > samedom_;
         std::vector< std::vector< Vertex > > buckets_;
 
         iterator_property_map<

test/dominator_tree_test.cpp

@@ -9,6 +9,7 @@
 #include <iostream>
 #include <algorithm>
 #include <boost/graph/adjacency_list.hpp>
+#include <boost/graph/adjacency_matrix.hpp>
 #include <boost/graph/dominator_tree.hpp>
 
 using namespace std;
@@ -24,13 +25,51 @@ struct DominatorCorrectnessTestSet
 
 using namespace boost;
 
-typedef adjacency_list< listS, listS, bidirectionalS,
-    property< vertex_index_t, std::size_t >, no_property >
-    G;
+// a workaround for the C++ standard before C++17, after switching to C++17,
+// the method may be just inlined into the run_test() with constexpr if.
+namespace detail
+{
 
-int main(int, char*[])
+template < typename Graph >
+void index_graph(Graph&, std::true_type /*IsRandomAccessAdjacentList*/)
+{
+    // nothing to do for already indexed adjacent list
+}
+
+template < typename Graph >
+void index_graph(Graph& g, std::false_type /*IsRandomAccessAdjacentList*/)
 {
-    typedef DominatorCorrectnessTestSet::edge edge;
+    using IndexMap = typename property_map< Graph, vertex_index_t >::type;
+    IndexMap indexMap(get(vertex_index, g));
+    typename graph_traits< Graph >::vertex_iterator uItr, uEnd;
+    int j = 0;
+    for (boost::tie(uItr, uEnd) = vertices(g); uItr != uEnd; ++uItr, ++j)
+    {
+        put(indexMap, *uItr, j);
+    }
+}
+
+} // namespace detail
+
+template < typename OEL, typename VL, typename D, typename VP, typename EP,
+    typename GP, typename EL >
+void index_graph(adjacency_list< OEL, VL, D, VP, EP, GP, EL >& g)
+{
+    using Traits = adjacency_list_traits< OEL, VL, D, EL >;
+    ::detail::index_graph(
+        g, std::integral_constant< bool, Traits::is_rand_access::value > {});
+}
+
+template < typename D, typename VP, typename EP, typename GP, typename A >
+void index_graph(adjacency_matrix<D, VP, EP, GP, A>&)
+{
+    // nothing to do for already indexed adjacent matrix
+}
+
+template < typename Graph >
+void run_test()
+{
+    using edge = DominatorCorrectnessTestSet::edge;
 
     DominatorCorrectnessTestSet testSet[7];
 
@@ -217,34 +256,32 @@ int main(int, char*[])
     {
         const int numOfVertices = testSet[i].numOfVertices;
 
-        G g(testSet[i].edges.begin(), testSet[i].edges.end(), numOfVertices);
+        Graph g(testSet[i].edges.begin(), testSet[i].edges.end(), numOfVertices);
+
+        using Vertex = typename graph_traits< Graph >::vertex_descriptor;
+        using IndexMap = typename property_map< Graph, vertex_index_t >::type;
+        IndexMap indexMap(get(vertex_index, g));
+        using PredMap
+            = iterator_property_map< typename vector< Vertex >::iterator, IndexMap >;
 
-        typedef graph_traits< G >::vertex_descriptor Vertex;
-        typedef property_map< G, vertex_index_t >::type IndexMap;
-        typedef iterator_property_map< vector< Vertex >::iterator, IndexMap >
-            PredMap;
+        index_graph(g);
 
         vector< Vertex > domTreePredVector, domTreePredVector2;
-        IndexMap indexMap(get(vertex_index, g));
-        graph_traits< G >::vertex_iterator uItr, uEnd;
-        int j = 0;
-        for (boost::tie(uItr, uEnd) = vertices(g); uItr != uEnd; ++uItr, ++j)
-        {
-            put(indexMap, *uItr, j);
-        }
 
         // Lengauer-Tarjan dominator tree algorithm
         domTreePredVector = vector< Vertex >(
-            num_vertices(g), graph_traits< G >::null_vertex());
+            num_vertices(g), graph_traits< Graph >::null_vertex());
         PredMap domTreePredMap
             = make_iterator_property_map(domTreePredVector.begin(), indexMap);
 
         lengauer_tarjan_dominator_tree(g, vertex(0, g), domTreePredMap);
 
         vector< int > idom(num_vertices(g));
+        typename graph_traits< Graph >::vertex_iterator uItr, uEnd;
         for (boost::tie(uItr, uEnd) = vertices(g); uItr != uEnd; ++uItr)
         {
-            if (get(domTreePredMap, *uItr) != graph_traits< G >::null_vertex())
+            if (get(domTreePredMap, *uItr)
+                != graph_traits< Graph >::null_vertex())
                 idom[get(indexMap, *uItr)]
                     = get(indexMap, get(domTreePredMap, *uItr));
             else
@@ -260,7 +297,7 @@ int main(int, char*[])
 
         // compare results of fast version and slow version of dominator tree
         domTreePredVector2 = vector< Vertex >(
-            num_vertices(g), graph_traits< G >::null_vertex());
+            num_vertices(g), graph_traits< Graph >::null_vertex());
         domTreePredMap
             = make_iterator_property_map(domTreePredVector2.begin(), indexMap);
 
@@ -269,7 +306,8 @@ int main(int, char*[])
         vector< int > idom2(num_vertices(g));
         for (boost::tie(uItr, uEnd) = vertices(g); uItr != uEnd; ++uItr)
         {
-            if (get(domTreePredMap, *uItr) != graph_traits< G >::null_vertex())
+            if (get(domTreePredMap, *uItr)
+                != graph_traits< Graph >::null_vertex())
                 idom2[get(indexMap, *uItr)]
                     = get(indexMap, get(domTreePredMap, *uItr));
             else
@@ -283,6 +321,21 @@ int main(int, char*[])
         for (k = 0; k < num_vertices(g); ++k)
             BOOST_TEST(domTreePredVector[k] == domTreePredVector2[k]);
     }
+    cout << endl;
+}
+
+int main(int, char*[])
+{
+    using AdjacencyListList = adjacency_list< listS, listS, bidirectionalS,
+        property< vertex_index_t, std::size_t >, no_property >;
+
+    using AdjacencyListVec = adjacency_list< listS, vecS, bidirectionalS >;
+
+    using AdjacencyMatrix = adjacency_matrix< directedS >;
+
+    run_test< AdjacencyListList >();
+    run_test< AdjacencyListVec >();
+    run_test< AdjacencyMatrix >();
 
     return boost::report_errors();
 }