each group applies a rule only once

src/graph/optimizer/OptContext.h

@@ -9,6 +9,7 @@
 #include <boost/core/noncopyable.hpp>
 #include <memory>
 #include <unordered_map>
+#include <unordered_set>
 
 #include "common/cpp/helpers.h"
 
@@ -18,11 +19,14 @@ class ObjectPool;
 
 namespace graph {
 class QueryContext;
+class PlanNode;
 }  // namespace graph
 
 namespace opt {
 
 class OptGroupNode;
+class OptGroup;
+class Optimizer;
 
 class OptContext final : private boost::noncopyable, private cpp::NonMovable {
  public:
@@ -48,12 +52,16 @@ class OptContext final : private boost::noncopyable, private cpp::NonMovable {
   const OptGroupNode *findOptGroupNodeByPlanNodeId(int64_t planNodeId) const;
 
  private:
+  friend OptGroup;
+  friend Optimizer;
   // A global flag to record whether this iteration caused a change to the plan
   bool changed_{true};
   graph::QueryContext *qctx_{nullptr};
   // Memo memory management in the Optimizer phase
   std::unique_ptr<ObjectPool> objPool_;
   std::unordered_map<int64_t, const OptGroupNode *> planNodeToOptGroupNodeMap_;
+  std::unordered_set<const OptGroup *> visited_;
+  std::unordered_map<const OptGroup *, const graph::PlanNode *> group2PlanNodeMap_;
 };
 
 }  // namespace opt

src/graph/optimizer/OptGroup.cpp

@@ -28,6 +28,9 @@ OptGroup *OptGroup::create(OptContext *ctx) {
 }
 
 void OptGroup::setUnexplored(const OptRule *rule) {
+  if (!ctx_->visited_.emplace(this).second) {
+    return;
+  }
   auto iter = std::find(exploredRules_.begin(), exploredRules_.end(), rule);
   if (iter != exploredRules_.end()) {
     exploredRules_.erase(iter);
@@ -100,6 +103,9 @@ Status OptGroup::validate(const OptRule *rule) const {
         rule->toString().c_str(),
         groupNodesReferenced_.size());
   }
+  if (!ctx_->visited_.emplace(this).second) {
+    return Status::OK();
+  }
   for (auto *gn : groupNodes_) {
     NG_RETURN_IF_ERROR(gn->validate(rule));
     if (gn->node()->outputVar() != outputVar_) {
@@ -138,6 +144,9 @@ Status OptGroup::explore(const OptRule *rule) {
     return Status::OK();
   }
   setExplored(rule);
+  if (!ctx_->visited_.emplace(this).second) {
+    return Status::OK();
+  }
 
   // TODO(yee): the opt group maybe in the loop body branch
   // DCHECK(isRootGroup_ || !groupNodesReferenced_.empty())
@@ -241,8 +250,15 @@ double OptGroup::getCost() const {
 }
 
 const PlanNode *OptGroup::getPlan() const {
+  auto &group2PlanNodeMap = ctx_->group2PlanNodeMap_;
+  auto iter = group2PlanNodeMap.find(this);
+  if (iter != group2PlanNodeMap.end()) {
+    return iter->second;
+  }
   const OptGroupNode *minGroupNode = findMinCostGroupNode().second;
-  return DCHECK_NOTNULL(minGroupNode)->getPlan();
+  const auto plan = DCHECK_NOTNULL(minGroupNode)->getPlan();
+  group2PlanNodeMap.emplace(this, plan);
+  return plan;
 }
 
 void OptGroup::deleteRefGroupNode(const OptGroupNode *node) {

src/graph/optimizer/Optimizer.cpp

@@ -52,7 +52,8 @@ StatusOr<const PlanNode *> Optimizer::findBestPlan(QueryContext *qctx) {
 
 // Just for Properties Pruning
 Status Optimizer::postprocess(PlanNode *root, graph::QueryContext *qctx, GraphSpaceID spaceID) {
-  NG_RETURN_IF_ERROR(rewriteArgumentInputVar(root));
+  std::unordered_set<const PlanNode *> visitedPlanNode;
+  NG_RETURN_IF_ERROR(rewriteArgumentInputVar(root, visitedPlanNode));
   if (FLAGS_enable_optimizer_property_pruner_rule) {
     graph::PropertyTracker propsUsed;
     graph::PrunePropertiesVisitor visitor(propsUsed, qctx, spaceID);
@@ -81,8 +82,11 @@ Status Optimizer::doExploration(OptContext *octx, OptGroup *rootGroup) {
       for (auto rule : ruleSet->rules()) {
         // Explore until the maximum number of iterations(Rules) is reached
         NG_RETURN_IF_ERROR(rootGroup->exploreUntilMaxRound(rule));
+        octx->visited_.clear();
         NG_RETURN_IF_ERROR(rootGroup->validate(rule));
+        octx->visited_.clear();
         rootGroup->setUnexplored(rule);
+        octx->visited_.clear();
       }
     }
   }
@@ -164,9 +168,14 @@ bool findArgumentRefPlanNodeInPath(const std::vector<const PlanNode *> &path, Pl
 }  // namespace
 
 // static
-Status Optimizer::rewriteArgumentInputVarInternal(PlanNode *root,
-                                                  std::vector<const PlanNode *> &path) {
+Status Optimizer::rewriteArgumentInputVarInternal(
+    PlanNode *root,
+    std::vector<const PlanNode *> &path,
+    std::unordered_set<const PlanNode *> &visitedPlanNode) {
   if (!root) return Status::OK();
+  if (!visitedPlanNode.emplace(root).second) {
+    return Status::OK();
+  }
 
   path.push_back(root);
   switch (root->numDeps()) {
@@ -186,15 +195,15 @@ Status Optimizer::rewriteArgumentInputVarInternal(PlanNode *root,
     }
     case 1: {
       auto *dep = const_cast<PlanNode *>(root->dep());
-      NG_RETURN_IF_ERROR(rewriteArgumentInputVarInternal(dep, path));
+      NG_RETURN_IF_ERROR(rewriteArgumentInputVarInternal(dep, path, visitedPlanNode));
       break;
     }
     case 2: {
       auto *bpn = static_cast<BinaryInputNode *>(root);
       auto *left = const_cast<PlanNode *>(bpn->left());
-      NG_RETURN_IF_ERROR(rewriteArgumentInputVarInternal(left, path));
+      NG_RETURN_IF_ERROR(rewriteArgumentInputVarInternal(left, path, visitedPlanNode));
       auto *right = const_cast<PlanNode *>(bpn->right());
-      NG_RETURN_IF_ERROR(rewriteArgumentInputVarInternal(right, path));
+      NG_RETURN_IF_ERROR(rewriteArgumentInputVarInternal(right, path, visitedPlanNode));
       break;
     }
     default: {
@@ -206,37 +215,47 @@ Status Optimizer::rewriteArgumentInputVarInternal(PlanNode *root,
 
   if (root->kind() == PlanNode::Kind::kLoop) {
     auto loop = static_cast<Loop *>(root);
-    NG_RETURN_IF_ERROR(rewriteArgumentInputVar(const_cast<PlanNode *>(loop->body())));
+    NG_RETURN_IF_ERROR(
+        rewriteArgumentInputVar(const_cast<PlanNode *>(loop->body()), visitedPlanNode));
   }
 
   if (root->kind() == PlanNode::Kind::kSelect) {
     auto sel = static_cast<Select *>(root);
-    NG_RETURN_IF_ERROR(rewriteArgumentInputVar(const_cast<PlanNode *>(sel->then())));
-    NG_RETURN_IF_ERROR(rewriteArgumentInputVar(const_cast<PlanNode *>(sel->otherwise())));
+    NG_RETURN_IF_ERROR(
+        rewriteArgumentInputVar(const_cast<PlanNode *>(sel->then()), visitedPlanNode));
+    NG_RETURN_IF_ERROR(
+        rewriteArgumentInputVar(const_cast<PlanNode *>(sel->otherwise()), visitedPlanNode));
   }
 
   return Status::OK();
 }
 
 // static
-Status Optimizer::rewriteArgumentInputVar(PlanNode *root) {
+Status Optimizer::rewriteArgumentInputVar(PlanNode *root,
+                                          std::unordered_set<const PlanNode *> &visitedPlanNode) {
   std::vector<const PlanNode *> path;
-  return rewriteArgumentInputVarInternal(root, path);
+  return rewriteArgumentInputVarInternal(root, path, visitedPlanNode);
 }
 
 Status Optimizer::checkPlanDepth(const PlanNode *root) const {
   std::queue<const PlanNode *> queue;
+  std::unordered_set<const PlanNode *> visited;
   queue.push(root);
+  visited.emplace(root);
   size_t depth = 0;
   while (!queue.empty()) {
     size_t size = queue.size();
     for (size_t i = 0; i < size; ++i) {
       const PlanNode *node = queue.front();
       queue.pop();
       for (size_t j = 0; j < node->numDeps(); j++) {
-        queue.push(node->dep(j));
+        const auto *dep = node->dep(j);
+        if (visited.emplace(dep).second) {
+          queue.push(dep);
+        }
       }
     }
+
     ++depth;
     if (depth > FLAGS_max_plan_depth) {
       return Status::Error("The depth of plan tree has exceeded the max %lu level",

src/graph/optimizer/Optimizer.h

@@ -47,9 +47,12 @@ class Optimizer final {
                           OptGroupNode *gnode,
                           std::unordered_map<int64_t, OptGroup *> *visited);
 
-  static Status rewriteArgumentInputVar(graph::PlanNode *root);
-  static Status rewriteArgumentInputVarInternal(graph::PlanNode *root,
-                                                std::vector<const graph::PlanNode *> &path);
+  static Status rewriteArgumentInputVar(
+      graph::PlanNode *root, std::unordered_set<const graph::PlanNode *> &visitedPlanNode);
+  static Status rewriteArgumentInputVarInternal(
+      graph::PlanNode *root,
+      std::vector<const graph::PlanNode *> &path,
+      std::unordered_set<const graph::PlanNode *> &visitedPlanNode);
 
   Status checkPlanDepth(const graph::PlanNode *root) const;
 

src/graph/scheduler/Scheduler.cpp

@@ -23,6 +23,7 @@ namespace graph {
 
 /*static*/ void Scheduler::analyzeLifetime(const PlanNode* root, std::size_t loopLayers) {
   std::stack<std::tuple<const PlanNode*, std::size_t>> stack;
+  std::unordered_set<const PlanNode*> visited;
   stack.push(std::make_tuple(root, loopLayers));
   while (!stack.empty()) {
     const auto& current = stack.top();
@@ -36,6 +37,9 @@ namespace graph {
     auto* currentMutNode = const_cast<PlanNode*>(currentNode);
     currentMutNode->setLoopLayers(currentLoopLayers);
     stack.pop();
+    if (!visited.emplace(currentNode).second) {
+      continue;
+    }
 
     for (auto dep : currentNode->dependencies()) {
       stack.push(std::make_tuple(dep, currentLoopLayers));

src/graph/visitor/PrunePropertiesVisitor.cpp

@@ -16,10 +16,16 @@ PrunePropertiesVisitor::PrunePropertiesVisitor(PropertyTracker &propsUsed,
 }
 
 void PrunePropertiesVisitor::visit(PlanNode *node) {
+  if (!visitedPlanNode_.emplace(node).second) {
+    return;
+  }
   status_ = depsPruneProperties(node->dependencies());
 }
 
 void PrunePropertiesVisitor::visit(Filter *node) {
+  if (!visitedPlanNode_.emplace(node).second) {
+    return;
+  }
   visitCurrent(node);  // Filter will use properties in filter expression
   status_ = depsPruneProperties(node->dependencies());
 }
@@ -34,6 +40,9 @@ void PrunePropertiesVisitor::visitCurrent(Filter *node) {
 }
 
 void PrunePropertiesVisitor::visit(Project *node) {
+  if (!visitedPlanNode_.emplace(node).second) {
+    return;
+  }
   visitCurrent(node);  // Project won't use properties in column expression
   status_ = depsPruneProperties(node->dependencies());
 }
@@ -107,6 +116,9 @@ void PrunePropertiesVisitor::visitCurrent(Project *node) {
 }
 
 void PrunePropertiesVisitor::visit(Aggregate *node) {
+  if (!visitedPlanNode_.emplace(node).second) {
+    return;
+  }
   visitCurrent(node);
   status_ = depsPruneProperties(node->dependencies());
 }
@@ -153,6 +165,9 @@ void PrunePropertiesVisitor::visitCurrent(Aggregate *node) {
 }
 
 void PrunePropertiesVisitor::visit(ScanEdges *node) {
+  if (!visitedPlanNode_.emplace(node).second) {
+    return;
+  }
   rootNode_ = false;
   pruneCurrent(node);
   status_ = depsPruneProperties(node->dependencies());
@@ -208,6 +223,9 @@ void PrunePropertiesVisitor::pruneCurrent(ScanEdges *node) {
 }
 
 void PrunePropertiesVisitor::visit(Traverse *node) {
+  if (!visitedPlanNode_.emplace(node).second) {
+    return;
+  }
   rootNode_ = false;
   visitCurrent(node);
   status_ = depsPruneProperties(node->dependencies());
@@ -358,6 +376,9 @@ void PrunePropertiesVisitor::pruneCurrent(Traverse *node) {
 
 // AppendVertices should be deleted when no properties it pulls are used by the parent node.
 void PrunePropertiesVisitor::visit(AppendVertices *node) {
+  if (!visitedPlanNode_.emplace(node).second) {
+    return;
+  }
   visitCurrent(node);
   status_ = depsPruneProperties(node->dependencies());
 }
@@ -478,18 +499,30 @@ void PrunePropertiesVisitor::pruneCurrent(AppendVertices *node) {
 }
 
 void PrunePropertiesVisitor::visit(HashJoin *node) {
+  if (!visitedPlanNode_.emplace(node).second) {
+    return;
+  }
   status_ = depsPruneProperties(node->dependencies());
 }
 
 void PrunePropertiesVisitor::visit(CrossJoin *node) {
+  if (!visitedPlanNode_.emplace(node).second) {
+    return;
+  }
   status_ = pruneBinaryBranch(node->dependencies());
 }
 
 void PrunePropertiesVisitor::visit(Union *node) {
+  if (!visitedPlanNode_.emplace(node).second) {
+    return;
+  }
   status_ = pruneBinaryBranch(node->dependencies());
 }
 
 void PrunePropertiesVisitor::visit(Unwind *node) {
+  if (!visitedPlanNode_.emplace(node).second) {
+    return;
+  }
   visitCurrent(node);
   status_ = depsPruneProperties(node->dependencies());
 }

src/graph/visitor/PrunePropertiesVisitor.h

@@ -85,6 +85,7 @@ class PrunePropertiesVisitor final : public PlanNodeVisitor {
   Status status_;
   bool rootNode_{true};
   const int unknownType_ = 0;
+  std::unordered_set<PlanNode *> visitedPlanNode_;
 };
 
 }  // namespace graph