From 9d4d06bc0eb71f7c9cb4e2cb3ec507c4f907d5c1 Mon Sep 17 00:00:00 2001 From: jimingquan Date: Tue, 25 Apr 2023 15:22:32 +0800 Subject: [PATCH 1/5] add some print info --- src/graph/executor/algo/AllPathsExecutor.cpp | 47 ++++++++++++++++---- src/graph/executor/algo/AllPathsExecutor.h | 11 +++++ 2 files changed, 50 insertions(+), 8 deletions(-) diff --git a/src/graph/executor/algo/AllPathsExecutor.cpp b/src/graph/executor/algo/AllPathsExecutor.cpp index 70d2ba0b999..7cc347e23e9 100644 --- a/src/graph/executor/algo/AllPathsExecutor.cpp +++ b/src/graph/executor/algo/AllPathsExecutor.cpp @@ -292,15 +292,21 @@ folly::Future AllPathsExecutor::buildResult() { } } } + time::Duration buildPathTime; auto future = buildPathMultiJobs(); - return future.via(runner()).thenValue([this](auto&& resp) { - UNUSED(resp); - if (!withProp_ || emptyPropVids_.empty()) { - finish(ResultBuilder().value(Value(std::move(result_))).build()); - return folly::makeFuture(Status::OK()); - } - return getPathProps(); - }); + return future.via(runner()) + .ensure([this, buildPathTime]() { + otherStats_.emplace("build_path_time", + folly::sformat("{}(us)", buildPathTime.elapsedInUSec())); + }) + .thenValue([this](auto&& resp) { + UNUSED(resp); + if (!withProp_ || emptyPropVids_.empty()) { + finish(ResultBuilder().value(Value(std::move(result_))).build()); + return folly::makeFuture(Status::OK()); + } + return getPathProps(); + }); } folly::Future AllPathsExecutor::buildPathMultiJobs() { @@ -331,6 +337,27 @@ folly::Future AllPathsExecutor::buildPathMultiJobs() { }); } +size_t AllPathsExecutor::getPathsSize(size_t start, + size_t end, + std::shared_ptr>> pathsPtr, + const VertexMap& adjList) { + size_t size = 0; + for (auto i = start; i < end; ++i) { + auto& path = (*pathsPtr)[i]; + auto& edgeValue = path.back(); + DCHECK(edgeValue.isEdge()); + auto& dst = edgeValue.getEdge().dst; + + auto adjIter = adjList.find(dst); + if (adjIter == adjList.end()) { + continue; + } + auto& adjEdges = adjIter->second; + size += adjEdges.size(); + } + return size; +} + folly::Future> AllPathsExecutor::doBuildPath( size_t step, size_t start, @@ -344,6 +371,10 @@ folly::Future> AllPathsExecutor::doBuildPath( auto currentPathPtr = std::make_unique>(); auto newPathsPtr = std::make_shared>>(); + if (step <= maxStep_) { + newPathsPtr->reserve(getPathsSize(start, end, pathsPtr, adjList)); + } + for (auto i = start; i < end; ++i) { auto& path = (*pathsPtr)[i]; auto& edgeValue = path.back(); diff --git a/src/graph/executor/algo/AllPathsExecutor.h b/src/graph/executor/algo/AllPathsExecutor.h index 6c06ed1b231..c70b58d04d0 100644 --- a/src/graph/executor/algo/AllPathsExecutor.h +++ b/src/graph/executor/algo/AllPathsExecutor.h @@ -46,6 +46,12 @@ class AllPathsExecutor final : public StorageAccessExecutor { template using VertexMap = std::unordered_map, VertexHash, VertexEqual>; + struct NPath { + NPath* p{nullptr}; + Value v; + Value e; + }; + private: void buildRequestVids(bool reverse); @@ -73,6 +79,11 @@ class AllPathsExecutor final : public StorageAccessExecutor { bool hasSameVertices(const std::vector& edgeList, const Edge& edge); + size_t getPathsSize(size_t start, + size_t end, + std::shared_ptr>> pathsPtr, + const VertexMap& adjList); + private: const AllPaths* pathNode_{nullptr}; bool withProp_{false}; From f74880cb16b765932839eb624768d5b1ef352d61 Mon Sep 17 00:00:00 2001 From: jimingquan Date: Tue, 25 Apr 2023 16:12:50 +0800 Subject: [PATCH 2/5] use NPath --- src/graph/executor/algo/AllPathsExecutor.cpp | 103 ++++++++++++------- src/graph/executor/algo/AllPathsExecutor.h | 41 ++++++-- tests/tck/features/path/AllPath.feature | 1 + 3 files changed, 95 insertions(+), 50 deletions(-) diff --git a/src/graph/executor/algo/AllPathsExecutor.cpp b/src/graph/executor/algo/AllPathsExecutor.cpp index 7cc347e23e9..7ceb3d93e12 100644 --- a/src/graph/executor/algo/AllPathsExecutor.cpp +++ b/src/graph/executor/algo/AllPathsExecutor.cpp @@ -310,7 +310,9 @@ folly::Future AllPathsExecutor::buildResult() { } folly::Future AllPathsExecutor::buildPathMultiJobs() { - auto pathsPtr = std::make_shared>>(); + auto pathsPtr = std::make_shared>(); + threadLocalPtr_.reset(new std::vector()); + threadLocalPtr_->reserve(64); for (auto& vid : leftInitVids_) { auto vidIter = leftAdjList_.find(vid); if (vidIter == leftAdjList_.end()) { @@ -323,7 +325,9 @@ folly::Future AllPathsExecutor::buildPathMultiJobs() { } pathsPtr->reserve(adjEdges.size() + pathsPtr->size()); for (auto& edge : adjEdges) { - pathsPtr->emplace_back(std::vector({src, edge})); + NPath* newPath = new NPath(src, edge); + threadLocalPtr_->push_back(newPath); + pathsPtr->emplace_back(newPath); } } size_t step = 2; @@ -337,55 +341,49 @@ folly::Future AllPathsExecutor::buildPathMultiJobs() { }); } -size_t AllPathsExecutor::getPathsSize(size_t start, - size_t end, - std::shared_ptr>> pathsPtr, - const VertexMap& adjList) { - size_t size = 0; - for (auto i = start; i < end; ++i) { - auto& path = (*pathsPtr)[i]; - auto& edgeValue = path.back(); - DCHECK(edgeValue.isEdge()); - auto& dst = edgeValue.getEdge().dst; - - auto adjIter = adjList.find(dst); - if (adjIter == adjList.end()) { - continue; - } - auto& adjEdges = adjIter->second; - size += adjEdges.size(); +// construct ROW[src1, [e1, v2, e2]] +Row AllPathsExecutor::convertNPath2Row(NPath* path) { + std::vector list; + NPath* head = path; + while (head != nullptr) { + list.emplace_back(head->edge); + list.emplace_back(head->vertex); + head = head->p; } - return size; + Row row; + // add src; + row.values.emplace_back(list.back()); + list.pop_back(); + std::reverse(list.begin(), list.end()); + List edgeList(std::move(list)); + row.values.emplace_back(std::move(edgeList)); + return row; } folly::Future> AllPathsExecutor::doBuildPath( size_t step, size_t start, size_t end, - std::shared_ptr>> pathsPtr) { + std::shared_ptr> pathsPtr) { if (cnt_.load(std::memory_order_relaxed) >= limit_) { return folly::makeFuture>(std::vector()); } - + threadLocalPtr_.reset(new std::vector()); + threadLocalPtr_->reserve(64); auto& adjList = leftAdjList_; auto currentPathPtr = std::make_unique>(); - auto newPathsPtr = std::make_shared>>(); + auto newPathsPtr = std::make_shared>(); - if (step <= maxStep_) { - newPathsPtr->reserve(getPathsSize(start, end, pathsPtr, adjList)); - } for (auto i = start; i < end; ++i) { - auto& path = (*pathsPtr)[i]; - auto& edgeValue = path.back(); + auto path = (*pathsPtr)[i]; + auto& edgeValue = path->edge; DCHECK(edgeValue.isEdge()); auto& dst = edgeValue.getEdge().dst; auto dstIter = rightInitVids_.find(dst); if (dstIter != rightInitVids_.end()) { - Row row; - row.values.emplace_back(path.front()); - List edgeList(std::vector(path.begin() + 1, path.end())); - row.values.emplace_back(std::move(edgeList)); + auto row = convertNPath2Row(path); + // add dst row.values.emplace_back(*dstIter); currentPathPtr->emplace_back(std::move(row)); ++cnt_; @@ -402,19 +400,17 @@ folly::Future> AllPathsExecutor::doBuildPath( auto& adjedges = adjIter->second; for (auto& edge : adjedges) { if (noLoop_) { - if (hasSameVertices(path, edge.getEdge())) { + if (hasSameV(path, edge.getEdge())) { continue; } } else { - if (hasSameEdge(path, edge.getEdge())) { + if (hasSameE(path, edge.getEdge())) { continue; } } - // copy - auto newPath = path; - newPath.emplace_back(adjIter->first); - newPath.emplace_back(edge); - newPathsPtr->emplace_back(std::move(newPath)); + NPath* newPath = new NPath(path, adjIter->first, edge); + threadLocalPtr_->push_back(newPath); + newPathsPtr->emplace_back(newPath); } } } @@ -472,6 +468,25 @@ folly::Future AllPathsExecutor::getPathProps() { }); } +bool AllPathsExecutor::hasSameE(NPath* path, const Edge& edge) { + NPath* head = path; + while (head != nullptr) { + if (edge == head->edge) { + return true; + } + head = head->p; + } + return false; +} + +bool AllPathsExecutor::hasSameV(NPath* path, const Edge& edge) { + if (edge.src == edge.dst) { + return true; + } + UNUSED(path); + return false; +} + bool AllPathsExecutor::hasSameVertices(const std::vector& edgeList, const Edge& edge) { if (edge.src == edge.dst) { return true; @@ -489,5 +504,15 @@ bool AllPathsExecutor::hasSameVertices(const std::vector& edgeList, const return false; } +Status AllPathsExecutor::close() { + auto accessor = threadLocalPtr_.accessAllThreads(); + for (auto iter = accessor.begin(); iter != accessor.end(); ++iter) { + for (auto& ptr : *iter) { + ptr->~NPath(); + } + } + return Executor::close(); +} + } // namespace graph } // namespace nebula diff --git a/src/graph/executor/algo/AllPathsExecutor.h b/src/graph/executor/algo/AllPathsExecutor.h index c70b58d04d0..2640b6ad7ab 100644 --- a/src/graph/executor/algo/AllPathsExecutor.h +++ b/src/graph/executor/algo/AllPathsExecutor.h @@ -6,6 +6,7 @@ #define GRAPH_EXECUTOR_ALGO_ALLPATHSEXECUTOR_H_ #include "graph/executor/StorageAccessExecutor.h" +#include "folly/ThreadLocal.h" // Using the two-way BFS algorithm, a heuristic algorithm is used in the expansion process // when the number of vid to be expanded on the left and right @@ -48,8 +49,18 @@ class AllPathsExecutor final : public StorageAccessExecutor { struct NPath { NPath* p{nullptr}; - Value v; - Value e; + Value vertex; + Value edge; + NPath(const Value& v, const Value& e) : vertex(v), edge(e) {} + NPath(NPath* path, const Value& v, const Value& e) : p(path), vertex(v), edge(e) {} + NPath(NPath&& v) noexcept : p(v.p), vertex(std::move(v.vertex)), edge(std::move(v.edge)) {} + NPath(const NPath& v) : p(v.p), vertex(v.vertex), edge(v.edge) {} + ~NPath() { + if (p != nullptr) { + delete p; + p = nullptr; + } + } }; private: @@ -65,11 +76,18 @@ class AllPathsExecutor final : public StorageAccessExecutor { void expandFromRight(GetNeighborsIter* iter); - folly::Future> doBuildPath( - size_t step, - size_t start, - size_t end, - std::shared_ptr>> edgeLists); + Row convertNPath2Row(NPath* path); + + // folly::Future> doBuildPath( + // size_t step, + // size_t start, + // size_t end, + // std::shared_ptr>> edgeLists); + + folly::Future> doBuildPath(size_t step, + size_t start, + size_t end, + std::shared_ptr> paths); folly::Future getPathProps(); @@ -78,11 +96,10 @@ class AllPathsExecutor final : public StorageAccessExecutor { folly::Future buildResult(); bool hasSameVertices(const std::vector& edgeList, const Edge& edge); + bool hasSameV(NPath* path, const Edge& edge); + bool hasSameE(NPath* Path, const Edge& edge); - size_t getPathsSize(size_t start, - size_t end, - std::shared_ptr>> pathsPtr, - const VertexMap& adjList); + Status close() override; private: const AllPaths* pathNode_{nullptr}; @@ -105,6 +122,8 @@ class AllPathsExecutor final : public StorageAccessExecutor { DataSet result_; std::vector emptyPropVids_; + class NewTag {}; + folly::ThreadLocalPtr, NewTag> threadLocalPtr_; }; } // namespace graph } // namespace nebula diff --git a/tests/tck/features/path/AllPath.feature b/tests/tck/features/path/AllPath.feature index 91afd8c9342..55c296b3575 100644 --- a/tests/tck/features/path/AllPath.feature +++ b/tests/tck/features/path/AllPath.feature @@ -1,6 +1,7 @@ # Copyright (c) 2020 vesoft inc. All rights reserved. # # This source code is licensed under Apache 2.0 License. +@jmq Feature: All Path Background: From cc81a73dd752f2d8979461b169ae3fc689119fb3 Mon Sep 17 00:00:00 2001 From: jimingquan Date: Wed, 26 Apr 2023 16:49:07 +0800 Subject: [PATCH 3/5] address comment --- src/graph/executor/algo/AllPathsExecutor.cpp | 14 +++++++++----- src/graph/executor/algo/AllPathsExecutor.h | 13 +------------ 2 files changed, 10 insertions(+), 17 deletions(-) diff --git a/src/graph/executor/algo/AllPathsExecutor.cpp b/src/graph/executor/algo/AllPathsExecutor.cpp index 7ceb3d93e12..4930b77caa0 100644 --- a/src/graph/executor/algo/AllPathsExecutor.cpp +++ b/src/graph/executor/algo/AllPathsExecutor.cpp @@ -311,8 +311,10 @@ folly::Future AllPathsExecutor::buildResult() { folly::Future AllPathsExecutor::buildPathMultiJobs() { auto pathsPtr = std::make_shared>(); - threadLocalPtr_.reset(new std::vector()); - threadLocalPtr_->reserve(64); + if (threadLocalPtr_.get() == nullptr) { + threadLocalPtr_.reset(new std::vector()); + threadLocalPtr_->reserve(64); + } for (auto& vid : leftInitVids_) { auto vidIter = leftAdjList_.find(vid); if (vidIter == leftAdjList_.end()) { @@ -368,8 +370,10 @@ folly::Future> AllPathsExecutor::doBuildPath( if (cnt_.load(std::memory_order_relaxed) >= limit_) { return folly::makeFuture>(std::vector()); } - threadLocalPtr_.reset(new std::vector()); - threadLocalPtr_->reserve(64); + if (threadLocalPtr_.get() == nullptr) { + threadLocalPtr_.reset(new std::vector()); + threadLocalPtr_->reserve(64); + } auto& adjList = leftAdjList_; auto currentPathPtr = std::make_unique>(); auto newPathsPtr = std::make_shared>(); @@ -508,7 +512,7 @@ Status AllPathsExecutor::close() { auto accessor = threadLocalPtr_.accessAllThreads(); for (auto iter = accessor.begin(); iter != accessor.end(); ++iter) { for (auto& ptr : *iter) { - ptr->~NPath(); + delete ptr; } } return Executor::close(); diff --git a/src/graph/executor/algo/AllPathsExecutor.h b/src/graph/executor/algo/AllPathsExecutor.h index 2640b6ad7ab..25038c18e44 100644 --- a/src/graph/executor/algo/AllPathsExecutor.h +++ b/src/graph/executor/algo/AllPathsExecutor.h @@ -55,12 +55,7 @@ class AllPathsExecutor final : public StorageAccessExecutor { NPath(NPath* path, const Value& v, const Value& e) : p(path), vertex(v), edge(e) {} NPath(NPath&& v) noexcept : p(v.p), vertex(std::move(v.vertex)), edge(std::move(v.edge)) {} NPath(const NPath& v) : p(v.p), vertex(v.vertex), edge(v.edge) {} - ~NPath() { - if (p != nullptr) { - delete p; - p = nullptr; - } - } + ~NPath() {} }; private: @@ -78,12 +73,6 @@ class AllPathsExecutor final : public StorageAccessExecutor { Row convertNPath2Row(NPath* path); - // folly::Future> doBuildPath( - // size_t step, - // size_t start, - // size_t end, - // std::shared_ptr>> edgeLists); - folly::Future> doBuildPath(size_t step, size_t start, size_t end, From cd46df5829c2a89541e2ca7836c328ed5e75e9f7 Mon Sep 17 00:00:00 2001 From: jimingquan Date: Thu, 27 Apr 2023 13:49:34 +0800 Subject: [PATCH 4/5] use localThreadPtr to manage memory --- src/graph/executor/algo/AllPathsExecutor.cpp | 109 ++++++++----------- src/graph/executor/algo/AllPathsExecutor.h | 22 ++-- tests/tck/features/path/AllPath.feature | 1 - 3 files changed, 57 insertions(+), 75 deletions(-) diff --git a/src/graph/executor/algo/AllPathsExecutor.cpp b/src/graph/executor/algo/AllPathsExecutor.cpp index 4930b77caa0..51d794dd5e5 100644 --- a/src/graph/executor/algo/AllPathsExecutor.cpp +++ b/src/graph/executor/algo/AllPathsExecutor.cpp @@ -12,7 +12,7 @@ DEFINE_uint32( 100, "the number of vids to expand, when this threshold is exceeded, use heuristic expansion"); DEFINE_uint32(path_threshold_ratio, 2, "threshold for heuristics expansion"); -DEFINE_uint32(path_batch_size, 5000, "number of paths constructed by each thread"); +DEFINE_uint32(path_batch_size, 50000, "number of paths constructed by each thread"); namespace nebula { namespace graph { @@ -156,6 +156,8 @@ folly::Future AllPathsExecutor::getNeighbors(bool reverse) { } auto listVal = std::make_shared(std::move(list)); auto iter = std::make_unique(listVal); + time::Duration buildAdjTime; + auto key = folly::sformat("buildAdjTime {}step[{}]", reverse ? "reverse " : "", step); if (reverse) { rightNextStepVids_.clear(); expandFromRight(iter.get()); @@ -163,6 +165,7 @@ folly::Future AllPathsExecutor::getNeighbors(bool reverse) { leftNextStepVids_.clear(); expandFromLeft(iter.get()); } + otherStats_.emplace(key, folly::sformat("{}(us)", buildAdjTime.elapsedInUSec())); return Status::OK(); }); } @@ -261,6 +264,7 @@ folly::Future AllPathsExecutor::buildResult() { // if key exists, discard the right adjacency's key & values // because the right adjacency list may have fewer edges // a->c->o, a->b, c->f, f->o + time::Duration mergeAdjTime; for (auto& rAdj : rightAdjList_) { auto& src = rAdj.first; auto iter = leftAdjList_.find(src); @@ -292,6 +296,7 @@ folly::Future AllPathsExecutor::buildResult() { } } } + otherStats_.emplace("merge_adj_time", folly::sformat("{}(us)", mergeAdjTime.elapsedInUSec())); time::Duration buildPathTime; auto future = buildPathMultiJobs(); return future.via(runner()) @@ -312,39 +317,43 @@ folly::Future AllPathsExecutor::buildResult() { folly::Future AllPathsExecutor::buildPathMultiJobs() { auto pathsPtr = std::make_shared>(); if (threadLocalPtr_.get() == nullptr) { - threadLocalPtr_.reset(new std::vector()); - threadLocalPtr_->reserve(64); + threadLocalPtr_.reset(new std::deque()); } for (auto& vid : leftInitVids_) { auto vidIter = leftAdjList_.find(vid); if (vidIter == leftAdjList_.end()) { continue; } - auto src = vidIter->first; + auto& src = vidIter->first; auto& adjEdges = vidIter->second; if (adjEdges.empty()) { continue; } pathsPtr->reserve(adjEdges.size() + pathsPtr->size()); for (auto& edge : adjEdges) { - NPath* newPath = new NPath(src, edge); - threadLocalPtr_->push_back(newPath); - pathsPtr->emplace_back(newPath); + threadLocalPtr_->emplace_back(NPath(src, edge)); + pathsPtr->emplace_back(&threadLocalPtr_->back()); } } size_t step = 2; auto future = doBuildPath(step, 0, pathsPtr->size(), pathsPtr); - return future.via(runner()).thenValue([this](std::vector&& paths) { + return future.via(runner()).thenValue([this](std::vector>&& paths) { memory::MemoryCheckGuard guard; + if (!paths.empty()) { - result_.rows.swap(paths); + time::Duration convertPathTime; + for (auto& path : paths) { + result_.rows.emplace_back(convertNPath2Row(path.first, path.second)); + } + otherStats_.emplace("convert_path_time", + folly::sformat("{}(us)", convertPathTime.elapsedInUSec())); } return Status::OK(); }); } -// construct ROW[src1, [e1, v2, e2]] -Row AllPathsExecutor::convertNPath2Row(NPath* path) { +// construct ROW[src1, [e1, v2, e2], v3] +Row AllPathsExecutor::convertNPath2Row(NPath* path, Value dst) { std::vector list; NPath* head = path; while (head != nullptr) { @@ -359,26 +368,26 @@ Row AllPathsExecutor::convertNPath2Row(NPath* path) { std::reverse(list.begin(), list.end()); List edgeList(std::move(list)); row.values.emplace_back(std::move(edgeList)); + row.values.emplace_back(std::move(dst)); return row; } -folly::Future> AllPathsExecutor::doBuildPath( - size_t step, - size_t start, - size_t end, - std::shared_ptr> pathsPtr) { +folly::Future>> +AllPathsExecutor::doBuildPath(size_t step, + size_t start, + size_t end, + std::shared_ptr> pathsPtr) { if (cnt_.load(std::memory_order_relaxed) >= limit_) { - return folly::makeFuture>(std::vector()); + return folly::makeFuture>>( + std::vector>()); } if (threadLocalPtr_.get() == nullptr) { - threadLocalPtr_.reset(new std::vector()); - threadLocalPtr_->reserve(64); + threadLocalPtr_.reset(new std::deque()); } auto& adjList = leftAdjList_; - auto currentPathPtr = std::make_unique>(); + auto currentStepResult = std::make_unique>>(); auto newPathsPtr = std::make_shared>(); - for (auto i = start; i < end; ++i) { auto path = (*pathsPtr)[i]; auto& edgeValue = path->edge; @@ -386,10 +395,7 @@ folly::Future> AllPathsExecutor::doBuildPath( auto& dst = edgeValue.getEdge().dst; auto dstIter = rightInitVids_.find(dst); if (dstIter != rightInitVids_.end()) { - auto row = convertNPath2Row(path); - // add dst - row.values.emplace_back(*dstIter); - currentPathPtr->emplace_back(std::move(row)); + currentStepResult->emplace_back(std::make_pair(path, *dstIter)); ++cnt_; if (cnt_.load(std::memory_order_relaxed) >= limit_) { break; @@ -404,26 +410,25 @@ folly::Future> AllPathsExecutor::doBuildPath( auto& adjedges = adjIter->second; for (auto& edge : adjedges) { if (noLoop_) { - if (hasSameV(path, edge.getEdge())) { + if (hasSameVertices(path, edge.getEdge())) { continue; } } else { - if (hasSameE(path, edge.getEdge())) { + if (hasSameEdge(path, edge.getEdge())) { continue; } } - NPath* newPath = new NPath(path, adjIter->first, edge); - threadLocalPtr_->push_back(newPath); - newPathsPtr->emplace_back(newPath); + threadLocalPtr_->emplace_back(NPath(path, adjIter->first, edge)); + newPathsPtr->emplace_back(&threadLocalPtr_->back()); } } } auto newPathsSize = newPathsPtr->size(); if (step > maxStep_ || newPathsSize == 0) { - return folly::makeFuture>(std::move(*currentPathPtr)); + return folly::makeFuture>>(std::move(*currentStepResult)); } - std::vector>> futures; + std::vector>>> futures; if (newPathsSize < FLAGS_path_batch_size) { futures.emplace_back(folly::via(runner(), [this, step, newPathsSize, newPathsPtr]() { return doBuildPath(step + 1, 0, newPathsSize, newPathsPtr); @@ -438,9 +443,10 @@ folly::Future> AllPathsExecutor::doBuildPath( } } return folly::collect(futures).via(runner()).thenValue( - [pathPtr = std::move(currentPathPtr)](std::vector>&& paths) { + [pathPtr = std::move(currentStepResult)]( + std::vector>>&& paths) { memory::MemoryCheckGuard guard; - std::vector result = std::move(*pathPtr); + std::vector> result = std::move(*pathPtr); for (auto& path : paths) { if (path.empty()) { continue; @@ -472,7 +478,7 @@ folly::Future AllPathsExecutor::getPathProps() { }); } -bool AllPathsExecutor::hasSameE(NPath* path, const Edge& edge) { +bool AllPathsExecutor::hasSameEdge(NPath* path, const Edge& edge) { NPath* head = path; while (head != nullptr) { if (edge == head->edge) { @@ -483,40 +489,21 @@ bool AllPathsExecutor::hasSameE(NPath* path, const Edge& edge) { return false; } -bool AllPathsExecutor::hasSameV(NPath* path, const Edge& edge) { - if (edge.src == edge.dst) { - return true; - } - UNUSED(path); - return false; -} - -bool AllPathsExecutor::hasSameVertices(const std::vector& edgeList, const Edge& edge) { +bool AllPathsExecutor::hasSameVertices(NPath* path, const Edge& edge) { if (edge.src == edge.dst) { return true; } auto& vid = edge.dst; - auto iter = edgeList.begin() + 1; - for (; iter != edgeList.end(); iter++) { - if (iter->isEdge()) { - auto& edgeVal = iter->getEdge(); - if (edgeVal.src == vid) { - return true; - } + NPath* head = path; + while (head != nullptr) { + auto& vertex = head->vertex; + if (vertex.getVertex().vid == vid) { + return true; } + head = head->p; } return false; } -Status AllPathsExecutor::close() { - auto accessor = threadLocalPtr_.accessAllThreads(); - for (auto iter = accessor.begin(); iter != accessor.end(); ++iter) { - for (auto& ptr : *iter) { - delete ptr; - } - } - return Executor::close(); -} - } // namespace graph } // namespace nebula diff --git a/src/graph/executor/algo/AllPathsExecutor.h b/src/graph/executor/algo/AllPathsExecutor.h index 25038c18e44..9282d77408e 100644 --- a/src/graph/executor/algo/AllPathsExecutor.h +++ b/src/graph/executor/algo/AllPathsExecutor.h @@ -27,10 +27,10 @@ // when expanding, if the vid has already been visited, do not visit again // leftAdjList_ save result of forward expansion // rightAdjList_ save result of backward expansion - namespace nebula { namespace graph { class AllPaths; +struct NPath; class AllPathsExecutor final : public StorageAccessExecutor { public: AllPathsExecutor(const PlanNode* node, QueryContext* qctx) @@ -49,8 +49,8 @@ class AllPathsExecutor final : public StorageAccessExecutor { struct NPath { NPath* p{nullptr}; - Value vertex; - Value edge; + const Value& vertex; + const Value& edge; NPath(const Value& v, const Value& e) : vertex(v), edge(e) {} NPath(NPath* path, const Value& v, const Value& e) : p(path), vertex(v), edge(e) {} NPath(NPath&& v) noexcept : p(v.p), vertex(std::move(v.vertex)), edge(std::move(v.edge)) {} @@ -71,12 +71,10 @@ class AllPathsExecutor final : public StorageAccessExecutor { void expandFromRight(GetNeighborsIter* iter); - Row convertNPath2Row(NPath* path); + Row convertNPath2Row(NPath* path, Value dst); - folly::Future> doBuildPath(size_t step, - size_t start, - size_t end, - std::shared_ptr> paths); + folly::Future>> doBuildPath( + size_t step, size_t start, size_t end, std::shared_ptr> paths); folly::Future getPathProps(); @@ -84,11 +82,9 @@ class AllPathsExecutor final : public StorageAccessExecutor { folly::Future buildResult(); - bool hasSameVertices(const std::vector& edgeList, const Edge& edge); - bool hasSameV(NPath* path, const Edge& edge); - bool hasSameE(NPath* Path, const Edge& edge); + bool hasSameEdge(NPath* path, const Edge& edge); - Status close() override; + bool hasSameVertices(NPath* path, const Edge& edge); private: const AllPaths* pathNode_{nullptr}; @@ -112,7 +108,7 @@ class AllPathsExecutor final : public StorageAccessExecutor { DataSet result_; std::vector emptyPropVids_; class NewTag {}; - folly::ThreadLocalPtr, NewTag> threadLocalPtr_; + folly::ThreadLocalPtr, NewTag> threadLocalPtr_; }; } // namespace graph } // namespace nebula diff --git a/tests/tck/features/path/AllPath.feature b/tests/tck/features/path/AllPath.feature index 55c296b3575..91afd8c9342 100644 --- a/tests/tck/features/path/AllPath.feature +++ b/tests/tck/features/path/AllPath.feature @@ -1,7 +1,6 @@ # Copyright (c) 2020 vesoft inc. All rights reserved. # # This source code is licensed under Apache 2.0 License. -@jmq Feature: All Path Background: From b49be074eebce19cf2c2bef839cf91bacbf267f3 Mon Sep 17 00:00:00 2001 From: jimingquan Date: Thu, 27 Apr 2023 15:23:42 +0800 Subject: [PATCH 5/5] add go simple plan --- src/graph/context/ast/QueryAstContext.h | 1 + src/graph/executor/algo/AllPathsExecutor.h | 2 +- src/graph/executor/query/ExpandExecutor.cpp | 3 +- src/graph/planner/ngql/GoPlanner.cpp | 30 +++++ src/graph/planner/ngql/GoPlanner.h | 2 + src/graph/validator/GoValidator.cpp | 38 +++++++ src/graph/validator/GoValidator.h | 2 + tests/tck/features/go/SimpleCase.feature | 104 +++++++++--------- .../PushFilterDownHashLeftJoinRule.feature | 29 +++-- 9 files changed, 139 insertions(+), 72 deletions(-) diff --git a/src/graph/context/ast/QueryAstContext.h b/src/graph/context/ast/QueryAstContext.h index 5e8d60fdcda..db981658b82 100644 --- a/src/graph/context/ast/QueryAstContext.h +++ b/src/graph/context/ast/QueryAstContext.h @@ -93,6 +93,7 @@ struct GoContext final : AstContext { bool joinInput{false}; // true when $$.tag.prop exist bool joinDst{false}; + bool isSimple{false}; ExpressionProps exprProps; diff --git a/src/graph/executor/algo/AllPathsExecutor.h b/src/graph/executor/algo/AllPathsExecutor.h index 9282d77408e..695a8efaa06 100644 --- a/src/graph/executor/algo/AllPathsExecutor.h +++ b/src/graph/executor/algo/AllPathsExecutor.h @@ -5,8 +5,8 @@ #ifndef GRAPH_EXECUTOR_ALGO_ALLPATHSEXECUTOR_H_ #define GRAPH_EXECUTOR_ALGO_ALLPATHSEXECUTOR_H_ -#include "graph/executor/StorageAccessExecutor.h" #include "folly/ThreadLocal.h" +#include "graph/executor/StorageAccessExecutor.h" // Using the two-way BFS algorithm, a heuristic algorithm is used in the expansion process // when the number of vid to be expanded on the left and right diff --git a/src/graph/executor/query/ExpandExecutor.cpp b/src/graph/executor/query/ExpandExecutor.cpp index 9cc1be79dc4..e5a1d9f295d 100644 --- a/src/graph/executor/query/ExpandExecutor.cpp +++ b/src/graph/executor/query/ExpandExecutor.cpp @@ -94,7 +94,8 @@ folly::Future ExpandExecutor::GetDstBySrc() { size = (*result.dsts_ref()).size(); } auto info = util::collectRespProfileData(result.result, hostLatency[i], size); - otherStats_.emplace(folly::sformat("resp[{}]", i), folly::toPrettyJson(info)); + otherStats_.emplace(folly::sformat("step{} resp [{}]", currentStep_, i), + folly::toPrettyJson(info)); } auto result = handleCompleteness(resps, FLAGS_accept_partial_success); if (!result.ok()) { diff --git a/src/graph/planner/ngql/GoPlanner.cpp b/src/graph/planner/ngql/GoPlanner.cpp index 4721c1e8d3a..d99dbdb1c06 100644 --- a/src/graph/planner/ngql/GoPlanner.cpp +++ b/src/graph/planner/ngql/GoPlanner.cpp @@ -142,6 +142,33 @@ PlanNode* GoPlanner::buildJoinDstPlan(PlanNode* dep) { return join; } +SubPlan GoPlanner::doSimplePlan() { + auto qctx = goCtx_->qctx; + size_t step = goCtx_->steps.mSteps(); + auto* expand = Expand::make(qctx, + startNode_, + goCtx_->space.id, + false, // random + step, + buildEdgeProps(true)); + expand->setEdgeTypes(buildEdgeTypes()); + expand->setColNames({"_expand_vid"}); + expand->setInputVar(goCtx_->vidsVar); + + auto* dedup = Dedup::make(qctx, expand); + + auto pool = qctx->objPool(); + auto* newYieldExpr = pool->makeAndAdd(); + newYieldExpr->addColumn(new YieldColumn(ColumnExpression::make(pool, 0))); + auto* project = Project::make(qctx, dedup, newYieldExpr); + project->setColNames(std::move(goCtx_->colNames)); + + SubPlan subPlan; + subPlan.root = project; + subPlan.tail = expand; + return subPlan; +} + SubPlan GoPlanner::doPlan() { auto qctx = goCtx_->qctx; auto& from = goCtx_->from; @@ -259,6 +286,9 @@ StatusOr GoPlanner::transform(AstContext* astCtx) { subPlan.root = subPlan.tail = pt; return subPlan; } + if (goCtx_->isSimple) { + return doSimplePlan(); + } return doPlan(); } diff --git a/src/graph/planner/ngql/GoPlanner.h b/src/graph/planner/ngql/GoPlanner.h index 9f620778fb1..1a430529039 100644 --- a/src/graph/planner/ngql/GoPlanner.h +++ b/src/graph/planner/ngql/GoPlanner.h @@ -34,6 +34,8 @@ class GoPlanner final : public Planner { private: SubPlan doPlan(); + SubPlan doSimplePlan(); + private: std::unique_ptr buildVertexProps(const ExpressionProps::TagIDPropsMap& propsMap); diff --git a/src/graph/validator/GoValidator.cpp b/src/graph/validator/GoValidator.cpp index 3a0b80d57e5..84eccb6fc0f 100644 --- a/src/graph/validator/GoValidator.cpp +++ b/src/graph/validator/GoValidator.cpp @@ -56,6 +56,7 @@ Status GoValidator::validateImpl() { exprProps.varProps().size() > 1) { return Status::SemanticError("Only support single input in a go sentence."); } + goCtx_->isSimple = isSimpleCase(); NG_RETURN_IF_ERROR(buildColumns()); return Status::OK(); @@ -282,5 +283,42 @@ bool GoValidator::checkDstPropOrVertexExist(const Expression* expr) { return true; } +bool GoValidator::isSimpleCase() { + if (!goCtx_->limits.empty() || !goCtx_->distinct || goCtx_->filter || goCtx_->steps.isMToN() || + goCtx_->from.fromType != FromType::kInstantExpr) { + return false; + } + // Check if the yield cluase uses: + // 1. src tag props, + // 2. or edge props, except the dst id of edge. + // 3. input or var props. + auto& exprProps = goCtx_->exprProps; + if (!exprProps.srcTagProps().empty() || !exprProps.dstTagProps().empty()) { + return false; + } + if (!exprProps.edgeProps().empty()) { + for (auto& edgeProp : exprProps.edgeProps()) { + auto props = edgeProp.second; + if (props.size() != 1 && props.find(kDst) == props.end()) { + return false; + } + } + } + + bool atLeastOneDstId = false; + for (auto& col : goCtx_->yieldExpr->columns()) { + auto expr = col->expr(); + if (expr->kind() != Expression::Kind::kEdgeDst) { + return false; + } + atLeastOneDstId = true; + auto dstIdExpr = static_cast(expr); + if (dstIdExpr->sym() != "*" && goCtx_->over.edgeTypes.size() != 1) { + return false; + } + } + return atLeastOneDstId; +} + } // namespace graph } // namespace nebula diff --git a/src/graph/validator/GoValidator.h b/src/graph/validator/GoValidator.h index 241f88c4a43..5a7a42f9fb9 100644 --- a/src/graph/validator/GoValidator.h +++ b/src/graph/validator/GoValidator.h @@ -44,6 +44,8 @@ class GoValidator final : public Validator { bool checkDstPropOrVertexExist(const Expression* expr); + bool isSimpleCase(); + private: std::unique_ptr goCtx_; diff --git a/tests/tck/features/go/SimpleCase.feature b/tests/tck/features/go/SimpleCase.feature index 8a5bf1f8ba7..625670d2899 100644 --- a/tests/tck/features/go/SimpleCase.feature +++ b/tests/tck/features/go/SimpleCase.feature @@ -16,12 +16,11 @@ Feature: Simple case | 2 | And the execution plan should be: | id | name | dependencies | operator info | - | 6 | Aggregate | 5 | | - | 5 | Dedup | 4 | | + | 5 | Aggregate | 4 | | | 4 | Project | 3 | | - | 3 | ExpandAll | 2 | | - | 2 | Expand | 1 | | - | 1 | Start | | | + | 3 | Dedup | 2 | | + | 2 | Expand | 0 | | + | 0 | Start | | | When profiling query: """ GO FROM "Yao Ming" OVER like YIELD DISTINCT id($$) AS dst, $$.player.age AS age | ORDER BY $-.dst @@ -72,13 +71,12 @@ Feature: Simple case | "Manu Ginobili" | | "Tim Duncan" | And the execution plan should be: - | id | name | dependencies | operator info | - | 6 | Sort | 5 | | - | 5 | Dedup | 4 | | - | 4 | Project | 3 | | - | 3 | ExpandAll | 2 | | - | 2 | Expand | 1 | | - | 1 | Start | | | + | id | name | dependencies | operator info | + | 5 | Sort | 4 | | + | 4 | Project | 3 | | + | 3 | Dedup | 2 | | + | 2 | Expand | 0 | | + | 0 | Start | | | When profiling query: """ GO FROM "Tony Parker" OVER like YIELD DISTINCT 2, id($$) AS a | ORDER BY $-.a @@ -107,12 +105,11 @@ Feature: Simple case | 22 | And the execution plan should be: | id | name | dependencies | operator info | - | 6 | Aggregate | 5 | | - | 5 | Dedup | 4 | | + | 5 | Aggregate | 4 | | | 4 | Project | 3 | | - | 3 | ExpandAll | 2 | | - | 2 | Expand | 1 | | - | 1 | Start | | | + | 3 | Dedup | 2 | | + | 2 | Expand | 0 | | + | 0 | Start | | | When profiling query: """ GO 3 STEPS FROM "Tony Parker" OVER serve BIDIRECT WHERE $$.team.name != "Lakers" YIELD DISTINCT id($$) | YIELD count(*) @@ -397,18 +394,17 @@ Feature: Simple case | 0 | And the execution plan should be: | id | name | dependencies | operator info | - | 13 | Aggregate | 12 | | - | 12 | Dedup | 11 | | - | 11 | Project | 10 | | - | 10 | HashInnerJoin | 5,9 | | - | 5 | Dedup | 4 | | + | 12 | Aggregate | 11 | | + | 11 | Dedup | 10 | | + | 10 | Project | 9 | | + | 9 | HashInnerJoin | 4,8 | | | 4 | Project | 3 | | - | 3 | ExpandAll | 2 | | - | 2 | Expand | 1 | | - | 1 | Start | | | - | 9 | ExpandAll | 8 | | - | 8 | Expand | 7 | | - | 7 | Argument | | | + | 3 | Dedup | 2 | | + | 2 | Expand | 0 | | + | 0 | Start | | | + | 8 | ExpandAll | 7 | | + | 7 | Expand | 6 | | + | 6 | Argument | | | When profiling query: """ GO 1 STEP FROM "Tony Parker" OVER * YIELD distinct id($$) as id| GO 3 STEP FROM $-.id OVER * YIELD distinct id($$) | YIELD COUNT(*) @@ -418,18 +414,17 @@ Feature: Simple case | 22 | And the execution plan should be: | id | name | dependencies | operator info | - | 13 | Aggregate | 12 | | - | 12 | Dedup | 11 | | - | 11 | Project | 10 | | - | 10 | HashInnerJoin | 5,9 | | - | 5 | Dedup | 4 | | + | 12 | Aggregate | 11 | | + | 11 | Dedup | 10 | | + | 10 | Project | 9 | | + | 9 | HashInnerJoin | 4,8 | | | 4 | Project | 3 | | - | 3 | ExpandAll | 2 | | - | 2 | Expand | 1 | | - | 1 | Start | | | - | 9 | ExpandAll | 8 | | - | 8 | Expand | 7 | | - | 7 | Argument | | | + | 3 | Dedup | 2 | | + | 2 | Expand | 0 | | + | 0 | Start | | | + | 8 | ExpandAll | 7 | | + | 7 | Expand | 6 | | + | 6 | Argument | | | Scenario: could not be optimied cases When profiling query: @@ -584,20 +579,19 @@ Feature: Simple case | "Grant Hill" | 46 | "Grant Hill" | And the execution plan should be: | id | name | dependencies | operator info | - | 18 | Sort | 17 | | - | 17 | Dedup | 16 | | - | 16 | Project | 21 | | - | 21 | HashInnerJoin | 5,20 | | - | 5 | Dedup | 4 | | + | 17 | Sort | 16 | | + | 16 | Dedup | 15 | | + | 15 | Project | 20 | | + | 20 | HashInnerJoin | 4,19 | | | 4 | Project | 3 | | - | 3 | ExpandAll | 2 | | - | 2 | Expand | 1 | | - | 1 | Start | | | - | 20 | Filter | 19 | | - | 19 | HashLeftJoin | 9,12 | | - | 9 | ExpandAll | 8 | | - | 8 | Expand | 7 | | - | 7 | Argument | | | - | 12 | Project | 11 | | - | 11 | GetVertices | 10 | | - | 10 | Argument | | | + | 3 | Dedup | 2 | | + | 2 | Expand | 0 | | + | 0 | Start | | | + | 19 | Filter | 18 | | + | 18 | HashLeftJoin | 8,11 | | + | 8 | ExpandAll | 7 | | + | 7 | Expand | 6 | | + | 6 | Argument | | | + | 11 | Project | 10 | | + | 10 | GetVertices | 9 | | + | 9 | Argument | | | diff --git a/tests/tck/features/optimizer/PushFilterDownHashLeftJoinRule.feature b/tests/tck/features/optimizer/PushFilterDownHashLeftJoinRule.feature index c5a7ef3995d..c5a20a0a4a1 100644 --- a/tests/tck/features/optimizer/PushFilterDownHashLeftJoinRule.feature +++ b/tests/tck/features/optimizer/PushFilterDownHashLeftJoinRule.feature @@ -63,22 +63,21 @@ Feature: Push Filter down HashLeftJoin rule | "Boris Diaw" | "Suns" | ["team"] | And the execution plan should be: | id | name | dependencies | operator info | - | 20 | TopN | 17 | | - | 17 | Dedup | 16 | | - | 16 | Project | 23 | | - | 23 | HashInnerJoin | 5,26 | | - | 5 | Dedup | 4 | | + | 19 | TopN | 16 | | + | 16 | Dedup | 15 | | + | 15 | Project | 22 | | + | 22 | HashInnerJoin | 4,25 | | | 4 | Project | 3 | | - | 3 | ExpandAll | 2 | | - | 2 | Expand | 1 | | - | 1 | Start | | | - | 26 | HashLeftJoin | 27,12 | | - | 27 | ExpandAll | 8 | {"filter": "((like.likeness>80) OR like.likeness IS EMPTY)"} | - | 8 | Expand | 7 | | - | 7 | Argument | | | - | 12 | Project | 11 | | - | 11 | GetVertices | 10 | | - | 10 | Argument | | | + | 3 | Dedup | 2 | | + | 2 | Expand | 0 | | + | 0 | Start | | | + | 25 | HashLeftJoin | 26,11 | | + | 26 | ExpandAll | 7 | {"filter": "((like.likeness>80) OR like.likeness IS EMPTY)"} | + | 7 | Expand | 6 | | + | 6 | Argument | | | + | 11 | Project | 10 | | + | 10 | GetVertices | 9 | | + | 9 | Argument | | | Scenario: NOT push filter down HashLeftJoin When profiling query: