From ab56abf4caf047ce66a0f80bfa3b6584dcd3e726 Mon Sep 17 00:00:00 2001 From: Gareth Andrew Lloyd Date: Sat, 9 Sep 2023 15:09:25 +0100 Subject: [PATCH] Optimize scanning vertices (#1227) --- src/query/db_accessor.hpp | 6 +++--- src/query/plan/operator.cpp | 7 +++++-- src/storage/v2/all_vertices_iterable.cpp | 6 +++--- src/storage/v2/all_vertices_iterable.hpp | 2 +- src/storage/v2/inmemory/label_index.hpp | 2 +- src/storage/v2/inmemory/label_property_index.hpp | 2 +- src/storage/v2/vertex_accessor.cpp | 5 +++++ src/storage/v2/vertex_accessor.hpp | 2 ++ src/storage/v2/vertices_iterable.cpp | 2 +- src/storage/v2/vertices_iterable.hpp | 2 +- 10 files changed, 23 insertions(+), 13 deletions(-) diff --git a/src/query/db_accessor.hpp b/src/query/db_accessor.hpp index fabcf1590..229df8f4b 100644 --- a/src/query/db_accessor.hpp +++ b/src/query/db_accessor.hpp @@ -331,17 +331,17 @@ class VerticesIterable final { it_; public: - explicit Iterator(storage::VerticesIterable::Iterator it) : it_(it) {} + explicit Iterator(storage::VerticesIterable::Iterator it) : it_(std::move(it)) {} explicit Iterator(std::unordered_set, std::equal_to, utils::Allocator>::iterator it) : it_(it) {} VertexAccessor operator*() const { - return std::visit([](auto it_) { return VertexAccessor(*it_); }, it_); + return std::visit([](auto &it_) { return VertexAccessor(*it_); }, it_); } Iterator &operator++() { - std::visit([this](auto it_) { this->it_ = ++it_; }, it_); + std::visit([](auto &it_) { ++it_; }, it_); return *this; } diff --git a/src/query/plan/operator.cpp b/src/query/plan/operator.cpp index 72600da9c..615ee4741 100644 --- a/src/query/plan/operator.cpp +++ b/src/query/plan/operator.cpp @@ -444,7 +444,7 @@ class ScanAllCursor : public Cursor { AbortCheck(context); - while (!vertices_ || vertices_it_.value() == vertices_.value().end()) { + while (!vertices_ || vertices_it_.value() == vertices_end_it_.value()) { if (!input_cursor_->Pull(frame, context)) return false; // We need a getter function, because in case of exhausting a lazy // iterable, we cannot simply reset it by calling begin(). @@ -455,6 +455,7 @@ class ScanAllCursor : public Cursor { // vertices _ = get_vertices_(frame, context); vertices_.emplace(std::move(next_vertices.value())); vertices_it_.emplace(vertices_.value().begin()); + vertices_end_it_.emplace(vertices_.value().end()); } #ifdef MG_ENTERPRISE if (license::global_license_checker.IsEnterpriseValidFast() && context.auth_checker && !FindNextVertex(context)) { @@ -469,7 +470,7 @@ class ScanAllCursor : public Cursor { #ifdef MG_ENTERPRISE bool FindNextVertex(const ExecutionContext &context) { - while (vertices_it_.value() != vertices_.value().end()) { + while (vertices_it_.value() != vertices_end_it_.value()) { if (context.auth_checker->Has(*vertices_it_.value(), view_, memgraph::query::AuthQuery::FineGrainedPrivilege::READ)) { return true; @@ -486,6 +487,7 @@ class ScanAllCursor : public Cursor { input_cursor_->Reset(); vertices_ = std::nullopt; vertices_it_ = std::nullopt; + vertices_end_it_ = std::nullopt; } private: @@ -495,6 +497,7 @@ class ScanAllCursor : public Cursor { TVerticesFun get_vertices_; std::optional::type::value_type> vertices_; std::optional vertices_it_; + std::optional vertices_end_it_; const char *op_name_; }; diff --git a/src/storage/v2/all_vertices_iterable.cpp b/src/storage/v2/all_vertices_iterable.cpp index 46fb8e521..a95d306f1 100644 --- a/src/storage/v2/all_vertices_iterable.cpp +++ b/src/storage/v2/all_vertices_iterable.cpp @@ -17,11 +17,11 @@ auto AdvanceToVisibleVertex(utils::SkipList::Iterator it, utils::SkipLis std::optional *vertex, Transaction *tx, View view, Indices *indices, Constraints *constraints, Config::Items config) { while (it != end) { - *vertex = VertexAccessor::Create(&*it, tx, indices, constraints, config, view); - if (!*vertex) { + if (not VertexAccessor::IsVisible(&*it, tx, view)) { ++it; continue; } + *vertex = VertexAccessor{&*it, tx, indices, constraints, config}; break; } return it; @@ -32,7 +32,7 @@ AllVerticesIterable::Iterator::Iterator(AllVerticesIterable *self, utils::SkipLi it_(AdvanceToVisibleVertex(it, self->vertices_accessor_.end(), &self->vertex_, self->transaction_, self->view_, self->indices_, self_->constraints_, self->config_)) {} -VertexAccessor AllVerticesIterable::Iterator::operator*() const { return *self_->vertex_; } +VertexAccessor const &AllVerticesIterable::Iterator::operator*() const { return *self_->vertex_; } AllVerticesIterable::Iterator &AllVerticesIterable::Iterator::operator++() { ++it_; diff --git a/src/storage/v2/all_vertices_iterable.hpp b/src/storage/v2/all_vertices_iterable.hpp index 19b12d50b..8b5121de2 100644 --- a/src/storage/v2/all_vertices_iterable.hpp +++ b/src/storage/v2/all_vertices_iterable.hpp @@ -33,7 +33,7 @@ class AllVerticesIterable final { public: Iterator(AllVerticesIterable *self, utils::SkipList::Iterator it); - VertexAccessor operator*() const; + VertexAccessor const &operator*() const; Iterator &operator++(); diff --git a/src/storage/v2/inmemory/label_index.hpp b/src/storage/v2/inmemory/label_index.hpp index 7b05e8c38..1658050c7 100644 --- a/src/storage/v2/inmemory/label_index.hpp +++ b/src/storage/v2/inmemory/label_index.hpp @@ -67,7 +67,7 @@ class InMemoryLabelIndex : public storage::LabelIndex { public: Iterator(Iterable *self, utils::SkipList::Iterator index_iterator); - VertexAccessor operator*() const { return current_vertex_accessor_; } + VertexAccessor const &operator*() const { return current_vertex_accessor_; } bool operator==(const Iterator &other) const { return index_iterator_ == other.index_iterator_; } bool operator!=(const Iterator &other) const { return index_iterator_ != other.index_iterator_; } diff --git a/src/storage/v2/inmemory/label_property_index.hpp b/src/storage/v2/inmemory/label_property_index.hpp index 69360aa98..00e2c90a5 100644 --- a/src/storage/v2/inmemory/label_property_index.hpp +++ b/src/storage/v2/inmemory/label_property_index.hpp @@ -74,7 +74,7 @@ class InMemoryLabelPropertyIndex : public storage::LabelPropertyIndex { public: Iterator(Iterable *self, utils::SkipList::Iterator index_iterator); - VertexAccessor operator*() const { return current_vertex_accessor_; } + VertexAccessor const &operator*() const { return current_vertex_accessor_; } bool operator==(const Iterator &other) const { return index_iterator_ == other.index_iterator_; } bool operator!=(const Iterator &other) const { return index_iterator_ != other.index_iterator_; } diff --git a/src/storage/v2/vertex_accessor.cpp b/src/storage/v2/vertex_accessor.cpp index 795dfa996..d5168d847 100644 --- a/src/storage/v2/vertex_accessor.cpp +++ b/src/storage/v2/vertex_accessor.cpp @@ -84,6 +84,11 @@ std::optional VertexAccessor::Create(Vertex *vertex, Transaction return VertexAccessor{vertex, transaction, indices, constraints, config}; } +bool VertexAccessor::IsVisible(const Vertex *vertex, const Transaction *transaction, View view) { + const auto [exists, deleted] = detail::IsVisible(vertex, transaction, view); + return exists && !deleted; +} + bool VertexAccessor::IsVisible(View view) const { const auto [exists, deleted] = detail::IsVisible(vertex_, transaction_, view); return exists && (for_deleted_ || !deleted); diff --git a/src/storage/v2/vertex_accessor.hpp b/src/storage/v2/vertex_accessor.hpp index ff88272c3..d612007b9 100644 --- a/src/storage/v2/vertex_accessor.hpp +++ b/src/storage/v2/vertex_accessor.hpp @@ -45,6 +45,8 @@ class VertexAccessor final { static std::optional Create(Vertex *vertex, Transaction *transaction, Indices *indices, Constraints *constraints, Config::Items config, View view); + static bool IsVisible(Vertex const *vertex, Transaction const *transaction, View view); + /// @return true if the object is visible from the current transaction bool IsVisible(View view) const; diff --git a/src/storage/v2/vertices_iterable.cpp b/src/storage/v2/vertices_iterable.cpp index 34d6c76b2..f6ff46da6 100644 --- a/src/storage/v2/vertices_iterable.cpp +++ b/src/storage/v2/vertices_iterable.cpp @@ -210,7 +210,7 @@ void VerticesIterable::Iterator::Destroy() noexcept { } } -VertexAccessor VerticesIterable::Iterator::operator*() const { +VertexAccessor const &VerticesIterable::Iterator::operator*() const { switch (type_) { case Type::ALL: return *all_it_; diff --git a/src/storage/v2/vertices_iterable.hpp b/src/storage/v2/vertices_iterable.hpp index 7cd1fe208..e057e8a38 100644 --- a/src/storage/v2/vertices_iterable.hpp +++ b/src/storage/v2/vertices_iterable.hpp @@ -63,7 +63,7 @@ class VerticesIterable final { ~Iterator(); - VertexAccessor operator*() const; + VertexAccessor const &operator*() const; Iterator &operator++();