ng_calc_components: rework to move graphs

Rather than cloning graphs for output, rework calc components so that it
moves them.

diff --git a/src/compiler/compiler.cpp b/src/compiler/compiler.cpp
index 49ed88f4a37f9380bea107708423989711b41448..47bf514c3350af5b5ae336b32716283ff0613439 100644 (file)
--- a/src/compiler/compiler.cpp
+++ b/src/compiler/compiler.cpp
@@ -276,13 +276,12 @@ void addExpression(NG &ng, unsigned index, const char *expression,
         throw CompileError("Internal error.");
     }
 
-    auto &g = *built_expr.g;
-    if (!pe.expr.allow_vacuous && matches_everywhere(g)) {
+    if (!pe.expr.allow_vacuous && matches_everywhere(*built_expr.g)) {
         throw CompileError("Pattern matches empty buffer; use "
                            "HS_FLAG_ALLOWEMPTY to enable support.");
     }
 
-    if (!ng.addGraph(built_expr.expr, g)) {
+    if (!ng.addGraph(built_expr.expr, std::move(built_expr.g))) {
         DEBUG_PRINTF("NFA addGraph failed on ID %u.\n", pe.expr.report);
         throw CompileError("Error compiling expression.");
     }

diff --git a/src/nfagraph/ng.cpp b/src/nfagraph/ng.cpp
index eded7af2e17c7c70892f1449d7ec5e7191ffe286..bdd767e989c5f4e07844806e0135121e6e9d7873 100644 (file)
--- a/src/nfagraph/ng.cpp
+++ b/src/nfagraph/ng.cpp
@@ -315,7 +315,10 @@ bool processComponents(NG &ng, ExpressionInfo &expr,
     return false;
 }
 
-bool NG::addGraph(ExpressionInfo &expr, NGHolder &g) {
+bool NG::addGraph(ExpressionInfo &expr, unique_ptr<NGHolder> g_ptr) {
+    assert(g_ptr);
+    NGHolder &g = *g_ptr;
+
     // remove reports that aren't on vertices connected to accept.
     clearReports(g);
 
@@ -431,15 +434,16 @@ bool NG::addGraph(ExpressionInfo &expr, NGHolder &g) {
         return true;
     }
 
-    // Split the graph into a set of connected components.
+    // Split the graph into a set of connected components and process those.
+    // Note: this invalidates g_ptr.
 
-    deque<unique_ptr<NGHolder>> g_comp = calcComponents(g);
+    auto g_comp = calcComponents(std::move(g_ptr));
     assert(!g_comp.empty());
 
     if (!som) {
-        for (u32 i = 0; i < g_comp.size(); i++) {
-            assert(g_comp[i]);
-            reformLeadingDots(*g_comp[i]);
+        for (auto &gc : g_comp) {
+            assert(gc);
+            reformLeadingDots(*gc);
         }
 
         recalcComponents(g_comp);
@@ -453,12 +457,11 @@ bool NG::addGraph(ExpressionInfo &expr, NGHolder &g) {
     // have another shot at accepting the graph.
 
     if (cc.grey.prefilterReductions && expr.prefilter) {
-        for (u32 i = 0; i < g_comp.size(); i++) {
-            if (!g_comp[i]) {
+        for (auto &gc : g_comp) {
+            if (!gc) {
                 continue;
             }
-
-            prefilterReductions(*g_comp[i], cc);
+            prefilterReductions(*gc, cc);
         }
 
         if (processComponents(*this, expr, g_comp, som)) {

diff --git a/src/nfagraph/ng.h b/src/nfagraph/ng.h
index 6693773e173ee4f2ed0cad968e3fa86fb38cddfd..dc797acf7af33a0a7e7bbf8bcfdb73d1bbc70e7d 100644 (file)
--- a/src/nfagraph/ng.h
+++ b/src/nfagraph/ng.h
@@ -70,7 +70,7 @@ public:
 
     /** \brief Consumes a pattern, returns false or throws a CompileError
      * exception if the graph cannot be consumed. */
-    bool addGraph(ExpressionInfo &expr, NGHolder &h);
+    bool addGraph(ExpressionInfo &expr, std::unique_ptr<NGHolder> g_ptr);
 
     /** \brief Consumes a graph, cut-down version of addGraph for use by SOM
      * processing. */

diff --git a/src/nfagraph/ng_calc_components.cpp b/src/nfagraph/ng_calc_components.cpp
index da6775e44729ca4760186f4ba806f73bf105810d..ff0d0fe1e3647922c109c71323510a0a5da360bd 100644 (file)
--- a/src/nfagraph/ng_calc_components.cpp
+++ b/src/nfagraph/ng_calc_components.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015-2016, Intel Corporation
+ * Copyright (c) 2015-2017, Intel Corporation
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
@@ -245,31 +245,33 @@ void renumberVertices(NFAUndirectedGraph &ug) {
  * one or more connected components, adding them to the comps deque.
  */
 static
-void splitIntoComponents(const NGHolder &g, deque<unique_ptr<NGHolder>> &comps,
+void splitIntoComponents(unique_ptr<NGHolder> g,
+                         deque<unique_ptr<NGHolder>> &comps,
                          const depth &max_head_depth,
                          const depth &max_tail_depth, bool *shell_comp) {
-    DEBUG_PRINTF("graph has %zu vertices\n", num_vertices(g));
+    DEBUG_PRINTF("graph has %zu vertices\n", num_vertices(*g));
 
     assert(shell_comp);
     *shell_comp = false;
 
     // Compute "shell" head and tail subgraphs.
     vector<NFAVertexBidiDepth> depths;
-    calcDepths(g, depths);
-    auto head_shell = findHeadShell(g, depths, max_head_depth);
-    auto tail_shell = findTailShell(g, depths, max_tail_depth);
+    calcDepths(*g, depths);
+    auto head_shell = findHeadShell(*g, depths, max_head_depth);
+    auto tail_shell = findTailShell(*g, depths, max_tail_depth);
     for (auto v : head_shell) {
         tail_shell.erase(v);
     }
 
-    if (head_shell.size() + tail_shell.size() + N_SPECIALS >= num_vertices(g)) {
+    if (head_shell.size() + tail_shell.size() + N_SPECIALS >=
+        num_vertices(*g)) {
         DEBUG_PRINTF("all in shell component\n");
-        comps.push_back(cloneHolder(g));
+        comps.push_back(std::move(g));
         *shell_comp = true;
         return;
     }
 
-    vector<NFAEdge> shell_edges = findShellEdges(g, head_shell, tail_shell);
+    vector<NFAEdge> shell_edges = findShellEdges(*g, head_shell, tail_shell);
 
     DEBUG_PRINTF("%zu vertices in head, %zu in tail, %zu shell edges\n",
                  head_shell.size(), tail_shell.size(), shell_edges.size());
@@ -277,7 +279,7 @@ void splitIntoComponents(const NGHolder &g, deque<unique_ptr<NGHolder>> &comps,
     NFAUndirectedGraph ug;
     ue2::unordered_map<NFAVertex, NFAUndirectedVertex> old2new;
 
-    createUnGraph(g, true, true, ug, old2new);
+    createUnGraph(*g, true, true, ug, old2new);
 
     // Construct reverse mapping.
     ue2::unordered_map<NFAUndirectedVertex, NFAVertex> new2old;
@@ -298,7 +300,7 @@ void splitIntoComponents(const NGHolder &g, deque<unique_ptr<NGHolder>> &comps,
     assert(num > 0);
     if (num == 1 && shell_edges.empty()) {
         DEBUG_PRINTF("single component\n");
-        comps.push_back(cloneHolder(g));
+        comps.push_back(std::move(g));
         return;
     }
 
@@ -313,7 +315,7 @@ void splitIntoComponents(const NGHolder &g, deque<unique_ptr<NGHolder>> &comps,
         assert(contains(new2old, uv));
         NFAVertex v = new2old.at(uv);
         verts[c].push_back(v);
-        DEBUG_PRINTF("vertex %zu is in comp %u\n", g[v].index, c);
+        DEBUG_PRINTF("vertex %zu is in comp %u\n", (*g)[v].index, c);
     }
 
     ue2::unordered_map<NFAVertex, NFAVertex> v_map; // temp map for fillHolder
@@ -328,12 +330,12 @@ void splitIntoComponents(const NGHolder &g, deque<unique_ptr<NGHolder>> &comps,
 
         auto gc = ue2::make_unique<NGHolder>();
         v_map.clear();
-        fillHolder(gc.get(), g, vv, &v_map);
+        fillHolder(gc.get(), *g, vv, &v_map);
 
         // Remove shell edges, which will get their own component.
         for (const auto &e : shell_edges) {
-            auto cu = v_map.at(source(e, g));
-            auto cv = v_map.at(target(e, g));
+            auto cu = v_map.at(source(e, *g));
+            auto cv = v_map.at(target(e, *g));
             assert(edge(cu, cv, *gc).second);
             remove_edge(cu, cv, *gc);
         }
@@ -352,7 +354,7 @@ void splitIntoComponents(const NGHolder &g, deque<unique_ptr<NGHolder>> &comps,
 
         auto gc = ue2::make_unique<NGHolder>();
         v_map.clear();
-        fillHolder(gc.get(), g, vv, &v_map);
+        fillHolder(gc.get(), *g, vv, &v_map);
 
         pruneUseless(*gc);
         DEBUG_PRINTF("shell edge component %zu has %zu vertices\n",
@@ -374,26 +376,26 @@ void splitIntoComponents(const NGHolder &g, deque<unique_ptr<NGHolder>> &comps,
                   }));
 }
 
-deque<unique_ptr<NGHolder>> calcComponents(const NGHolder &g) {
+deque<unique_ptr<NGHolder>> calcComponents(unique_ptr<NGHolder> g) {
     deque<unique_ptr<NGHolder>> comps;
 
     // For trivial cases, we needn't bother running the full
     // connected_components algorithm.
-    if (isAlternationOfClasses(g)) {
-        comps.push_back(cloneHolder(g));
+    if (isAlternationOfClasses(*g)) {
+        comps.push_back(std::move(g));
         return comps;
     }
 
     bool shell_comp = false;
-    splitIntoComponents(g, comps, MAX_HEAD_SHELL_DEPTH, MAX_TAIL_SHELL_DEPTH,
-                        &shell_comp);
+    splitIntoComponents(std::move(g), comps, MAX_HEAD_SHELL_DEPTH,
+                        MAX_TAIL_SHELL_DEPTH, &shell_comp);
 
     if (shell_comp) {
         DEBUG_PRINTF("re-running on shell comp\n");
         assert(!comps.empty());
-        auto sc = move(comps.back());
+        auto sc = std::move(comps.back());
         comps.pop_back();
-        splitIntoComponents(*sc, comps, 0, 0, &shell_comp);
+        splitIntoComponents(std::move(sc), comps, 0, 0, &shell_comp);
     }
 
     DEBUG_PRINTF("finished; split into %zu components\n", comps.size());
@@ -409,14 +411,13 @@ void recalcComponents(deque<unique_ptr<NGHolder>> &comps) {
         }
 
         if (isAlternationOfClasses(*gc)) {
-            out.push_back(move(gc));
+            out.push_back(std::move(gc));
             continue;
         }
 
-        auto gc_comps = calcComponents(*gc);
-        for (auto &elem : gc_comps) {
-            out.push_back(move(elem));
-        }
+        auto gc_comps = calcComponents(std::move(gc));
+        out.insert(end(out), std::make_move_iterator(begin(gc_comps)),
+                   std::make_move_iterator(end(gc_comps)));
     }
 
     // Replace comps with our recalculated list.

diff --git a/src/nfagraph/ng_calc_components.h b/src/nfagraph/ng_calc_components.h
index e68c81fccea1bf5bf8fa0fe0d70cc575811635eb..0359f4a0b6f0b22852af3a60b9c163d78d380a6f 100644 (file)
--- a/src/nfagraph/ng_calc_components.h
+++ b/src/nfagraph/ng_calc_components.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, Intel Corporation
+ * Copyright (c) 2015-2017, Intel Corporation
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
@@ -42,7 +42,8 @@ class NGHolder;
 
 bool isAlternationOfClasses(const NGHolder &g);
 
-std::deque<std::unique_ptr<NGHolder>> calcComponents(const NGHolder &g);
+std::deque<std::unique_ptr<NGHolder>>
+calcComponents(std::unique_ptr<NGHolder> g);
 
 void recalcComponents(std::deque<std::unique_ptr<NGHolder>> &comps);
 

diff --git a/unit/internal/nfagraph_comp.cpp b/unit/internal/nfagraph_comp.cpp
index 41af3f0cae5d10ce67c625743314c4108cde4d16..8aae951934c274ca893397bd7e7d2cfd3f116aae 100644 (file)
--- a/unit/internal/nfagraph_comp.cpp
+++ b/unit/internal/nfagraph_comp.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, Intel Corporation
+ * Copyright (c) 2015-2017, Intel Corporation
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
@@ -33,13 +33,8 @@
 #include "config.h"
 #include "gtest/gtest.h"
 #include "nfagraph_common.h"
-#include "grey.h"
-#include "hs.h"
-#include "compiler/compiler.h"
 #include "nfagraph/ng.h"
-#include "nfagraph/ng_builder.h"
 #include "nfagraph/ng_calc_components.h"
-#include "util/target_info.h"
 
 using namespace std;
 using namespace ue2;
@@ -48,7 +43,7 @@ TEST(NFAGraph, CalcComp1) {
     auto graph = constructGraph("abc|def|ghi", 0);
     ASSERT_TRUE(graph != nullptr);
 
-    deque<unique_ptr<NGHolder>> comps = calcComponents(*graph);
+    auto comps = calcComponents(std::move(graph));
     ASSERT_EQ(3, comps.size());
 }
 
@@ -56,7 +51,7 @@ TEST(NFAGraph, CalcComp2) {
     auto graph = constructGraph("a|b|c|d|e|f|g|h|i", 0);
     ASSERT_TRUE(graph != nullptr);
 
-    deque<unique_ptr<NGHolder>> comps = calcComponents(*graph);
+    auto comps = calcComponents(std::move(graph));
 
     // We should be identifying this as a trivial case and not splitting it.
     ASSERT_EQ(1, comps.size());