Local shared poc

src/query/db_accessor.hpp

@@ -14,6 +14,7 @@
 #include <optional>
 #include <type_traits>
 
+#include <boost/smart_ptr/local_shared_ptr.hpp>
 #include <cppitertools/filter.hpp>
 #include <cppitertools/imap.hpp>
 
@@ -21,6 +22,7 @@
 #include "storage/v2/id_types.hpp"
 #include "storage/v2/property_value.hpp"
 #include "storage/v2/result.hpp"
+#include "storage/v2/vertex_accessor.hpp"
 #include "utils/pmr/unordered_set.hpp"
 #include "utils/variant_helpers.hpp"
 
@@ -120,20 +122,28 @@ class VertexAccessor final {
 
  public:
   // It can affect performance if we use std::unique_ptr here.
-  std::unique_ptr<storage::VertexAccessor> impl_;
+  // std::unique_ptr<storage::VertexAccessor> impl_;
+  // local_shared_ptr and all its local copies must reside in the same thread
+  boost::local_shared_ptr<storage::VertexAccessor> impl_;
+
+  // explicit VertexAccessor(std::unique_ptr<storage::VertexAccessor> impl) : impl_(std::move(impl)) {}
+  // increase reference count
+  explicit VertexAccessor(boost::local_shared_ptr<storage::VertexAccessor> impl) : impl_(impl) {}
+
+  // VertexAccessor(const VertexAccessor &impl) : impl_(impl.impl_->Copy()){};
+  VertexAccessor(const VertexAccessor &impl) : impl_(impl.impl_){};
 
-  explicit VertexAccessor(storage::VertexAccessor *impl) : impl_(impl) {}
-  explicit VertexAccessor(std::unique_ptr<storage::VertexAccessor> impl) : impl_(std::move(impl)) {}
-  VertexAccessor(const VertexAccessor &impl) : impl_(impl.impl_->Copy()){};
   explicit VertexAccessor(VertexAccessor *impl) : VertexAccessor(*impl) {}
 
-  ~VertexAccessor() = default;
-
   VertexAccessor &operator=(const VertexAccessor &other) {
-    impl_ = other.impl_->Copy();
+    // impl_ = other.impl_->Copy();
+    // return *this;
+    impl_ = other.impl_;
     return *this;
   }
 
+  ~VertexAccessor() = default;
+
   bool IsVisible(storage::View view) const { return impl_->IsVisible(view); }
 
   auto Labels(storage::View view) const { return impl_->Labels(view); }
@@ -295,15 +305,14 @@ class VerticesIterable final {
     VertexAccessor operator*() const {
       return std::visit(
           memgraph::utils::Overloaded{
-              [](storage::VerticesIterable::Iterator it_) { return VertexAccessor(*it_); },
-              // [](storage::VerticesIterable::Iterator it_) { return VertexAccessor(*it_); },
+              [](storage::VerticesIterable::Iterator it_) { return VertexAccessor((*it_)->Copy()); },
               [](std::unordered_set<VertexAccessor, std::hash<VertexAccessor>, std::equal_to<void>,
                                     utils::Allocator<VertexAccessor>>::iterator it_) { return VertexAccessor(*it_); }},
           it_);
     }
 
     Iterator &operator++() {
-      std::visit([this](auto it_) { this->it_ = ++it_; }, it_);
+      std::visit([](auto &it_) { ++it_; }, it_);
       return *this;
     }
 

src/storage/v2/indices.cpp

@@ -331,10 +331,6 @@ LabelIndex::Iterable::Iterator::Iterator(Iterable *self, utils::SkipList<Entry>:
   AdvanceUntilValid();
 }
 
-std::unique_ptr<VertexAccessor> LabelIndex::Iterable::Iterator::operator*() {
-  return std::move(current_vertex_accessor_);
-}
-
 LabelIndex::Iterable::Iterator &LabelIndex::Iterable::Iterator::operator++() {
   ++index_iterator_;
   AdvanceUntilValid();
@@ -488,10 +484,6 @@ LabelPropertyIndex::Iterable::Iterator::Iterator(Iterable *self, utils::SkipList
   AdvanceUntilValid();
 }
 
-std::unique_ptr<VertexAccessor> LabelPropertyIndex::Iterable::Iterator::operator*() {
-  return std::move(current_vertex_accessor_);
-}
-
 LabelPropertyIndex::Iterable::Iterator &LabelPropertyIndex::Iterable::Iterator::operator++() {
   ++index_iterator_;
   AdvanceUntilValid();

src/storage/v2/indices.hpp

@@ -86,7 +86,7 @@ class LabelIndex {
       Iterator(Iterator &&other) = default;
       ~Iterator() = default;
 
-      std::unique_ptr<VertexAccessor> operator*();
+      VertexAccessor *operator*() const { return current_vertex_accessor_.get(); }
 
       bool operator==(const Iterator &other) const { return index_iterator_ == other.index_iterator_; }
       bool operator!=(const Iterator &other) const { return index_iterator_ != other.index_iterator_; }
@@ -197,7 +197,7 @@ class LabelPropertyIndex {
       Iterator(Iterator &&other) = default;
       ~Iterator() = default;
 
-      std::unique_ptr<VertexAccessor> operator*();
+      VertexAccessor *operator*() const { return current_vertex_accessor_.get(); }
 
       bool operator==(const Iterator &other) const { return index_iterator_ == other.index_iterator_; }
       bool operator!=(const Iterator &other) const { return index_iterator_ != other.index_iterator_; }

src/storage/v2/storage.cpp

@@ -40,7 +40,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_)) {}
 
-std::unique_ptr<VertexAccessor> AllVerticesIterable::Iterator::operator*() const { return std::move(self_->vertex_); }
+VertexAccessor *AllVerticesIterable::Iterator::operator*() const { return self_->vertex_.get(); }
 
 AllVerticesIterable::Iterator &AllVerticesIterable::Iterator::operator++() {
   ++it_;
@@ -228,7 +228,7 @@ void VerticesIterable::Iterator::Destroy() noexcept {
   }
 }
 
-std::unique_ptr<VertexAccessor> VerticesIterable::Iterator::operator*() {
+VertexAccessor *VerticesIterable::Iterator::operator*() const {
   switch (type_) {
     case Type::ALL:
       return *all_it_;

src/storage/v2/storage.hpp

@@ -58,7 +58,7 @@ class AllVerticesIterable final {
    public:
     Iterator(AllVerticesIterable *self, utils::SkipList<Vertex>::Iterator it);
 
-    std::unique_ptr<VertexAccessor> operator*() const;
+    VertexAccessor *operator*() const;
 
     Iterator &operator++();
 
@@ -130,7 +130,7 @@ class VerticesIterable final {
 
     ~Iterator();
 
-    std::unique_ptr<VertexAccessor> operator*();
+    VertexAccessor *operator*() const;
 
     Iterator &operator++();
 

tests/mgbench/benchmark.py

@@ -29,7 +29,6 @@ QUERY_COUNT_LOWER_BOUND = 30
 
 
 def parse_args():
-
     parser = argparse.ArgumentParser(
         description="Memgraph benchmark executor.",
         formatter_class=argparse.ArgumentDefaultsHelpFormatter,
@@ -205,7 +204,6 @@ def warmup(condition: str, client: runners.BaseRunner, queries: list = None):
 def mixed_workload(
     vendor: runners.BaseRunner, client: runners.BaseClient, dataset, group, queries, benchmark_context: BenchmarkContext
 ):
-
     num_of_queries = benchmark_context.mode_config[0]
     percentage_distribution = benchmark_context.mode_config[1:]
     if sum(percentage_distribution) != 100:
@@ -233,7 +231,7 @@ def mixed_workload(
         "analytical": [],
     }
 
-    for (_, funcname) in queries[group]:
+    for _, funcname in queries[group]:
         for key in queries_by_type.keys():
             if key in funcname:
                 queries_by_type[key].append(funcname)
@@ -349,7 +347,6 @@ def get_query_cache_count(
     config_key: list,
     benchmark_context: BenchmarkContext,
 ):
-
     cached_count = config.get_value(*config_key)
     if cached_count is None:
         log.info(
@@ -403,7 +400,6 @@ def get_query_cache_count(
 
 
 if __name__ == "__main__":
-
     args = parse_args()
     vendor_specific_args = helpers.parse_kwargs(args.vendor_specific)
 
@@ -593,7 +589,7 @@ if __name__ == "__main__":
                         ret = client.execute(
                             queries=get_queries(func, count),
                             num_workers=benchmark_context.num_workers_for_benchmark,
-                            time_dependent_execution=benchmark_context.time_depended_execution,
+                            time_dependent_execution=benchmark_context.time_dependent_execution,
                         )[0]
                     else:
                         ret = client.execute(
@@ -647,7 +643,6 @@ if __name__ == "__main__":
                 vendor_runner.stop("authorization")
 
                 for query, funcname in queries[group]:
-
                     log.info(
                         "Running query:",
                         "{}/{}/{}/{}".format(group, query, funcname, WITH_FINE_GRAINED_AUTHORIZATION),

tests/unit/storage_v2_constraints.cpp

@@ -87,7 +87,7 @@ TEST_F(ConstraintsTest, ExistenceConstraintsCreateFailure1) {
   {
     auto acc = storage->Access();
     for (auto vertex : acc->Vertices(View::OLD)) {
-      ASSERT_NO_ERROR(acc->DeleteVertex(vertex.get()));
+      ASSERT_NO_ERROR(acc->DeleteVertex(vertex));
     }
     ASSERT_NO_ERROR(acc->Commit());
   }
@@ -168,7 +168,7 @@ TEST_F(ConstraintsTest, ExistenceConstraintsViolationOnCommit) {
       ASSERT_NO_ERROR(vertex->SetProperty(prop1, PropertyValue()));
     }
     for (auto vertex : acc->Vertices(View::OLD)) {
-      ASSERT_NO_ERROR(acc->DeleteVertex(vertex.get()));
+      ASSERT_NO_ERROR(acc->DeleteVertex(vertex));
     }
 
     ASSERT_NO_ERROR(acc->Commit());
@@ -247,7 +247,7 @@ TEST_F(ConstraintsTest, UniqueConstraintsCreateFailure1) {
   {
     auto acc = storage->Access();
     for (auto vertex : acc->Vertices(View::OLD)) {
-      ASSERT_NO_ERROR(acc->DeleteVertex(vertex.get()));
+      ASSERT_NO_ERROR(acc->DeleteVertex(vertex));
     }
     ASSERT_NO_ERROR(acc->Commit());
   }

tests/unit/storage_v2_durability.cpp

@@ -1455,7 +1455,7 @@ TEST_P(DurabilityTest, WalCreateAndRemoveEverything) {
     }
     auto acc = store->Access();
     for (auto vertex : acc->Vertices(memgraph::storage::View::OLD)) {
-      ASSERT_TRUE(acc->DetachDeleteVertex(vertex.get()).HasValue());
+      ASSERT_TRUE(acc->DetachDeleteVertex(vertex).HasValue());
     }
     ASSERT_FALSE(acc->Commit().HasError());
   }
@@ -1646,7 +1646,7 @@ TEST_P(DurabilityTest, WalCreateAndRemoveOnlyBaseDataset) {
       ASSERT_TRUE(has_indexed.HasValue());
       auto has_unindexed = vertex->HasLabel(label_unindexed, memgraph::storage::View::OLD);
       if (!*has_indexed && !*has_unindexed) continue;
-      ASSERT_TRUE(acc->DetachDeleteVertex(vertex.get()).HasValue());
+      ASSERT_TRUE(acc->DetachDeleteVertex(vertex).HasValue());
     }
     ASSERT_FALSE(acc->Commit().HasError());
   }

tests/unit/storage_v2_indices.cpp

@@ -270,7 +270,7 @@ TEST_F(IndexTest, LabelIndexBasic) {
   for (auto vertex : acc->Vertices(View::OLD)) {
     int64_t id = vertex->GetProperty(prop_id, View::OLD)->ValueInt();
     if (id % 2 == 0) {
-      ASSERT_NO_ERROR(acc->DeleteVertex(vertex.get()));
+      ASSERT_NO_ERROR(acc->DeleteVertex(vertex));
     }
   }
 
@@ -457,7 +457,7 @@ TEST_F(IndexTest, LabelPropertyIndexBasic) {
   for (auto vertex : acc->Vertices(View::OLD)) {
     int64_t id = vertex->GetProperty(prop_id, View::OLD)->ValueInt();
     if (id % 2 == 0) {
-      ASSERT_NO_ERROR(acc->DeleteVertex(vertex.get()));
+      ASSERT_NO_ERROR(acc->DeleteVertex(vertex));
     }
   }