Use MemoryResource in OrderByCursor

Summary:
Micro benchmarks show an improvement to performance of about 10%
compared to global new & delete.

Reviewers: mtomic, llugovic

Reviewed By: mtomic

Subscribers: pullbot

Differential Revision: https://phabricator.memgraph.io/D2061

src/query/common.cpp

@@ -214,7 +214,7 @@ void ReconstructTypedValue(TypedValue &value) {
   }
 }
 
-namespace {
+namespace impl {
 
 bool TypedValueCompare(const TypedValue &a, const TypedValue &b) {
   // in ordering null comes after everything else
@@ -258,30 +258,7 @@ bool TypedValueCompare(const TypedValue &a, const TypedValue &b) {
   }
 }
 
-}  // namespace
-
-bool TypedValueVectorCompare::operator()(
-    const std::vector<TypedValue> &c1,
-    const std::vector<TypedValue> &c2) const {
-  // ordering is invalid if there are more elements in the collections
-  // then there are in the ordering_ vector
-  DCHECK(c1.size() <= ordering_.size() && c2.size() <= ordering_.size())
-      << "Collections contain more elements then there are orderings";
-
-  auto c1_it = c1.begin();
-  auto c2_it = c2.begin();
-  auto ordering_it = ordering_.begin();
-  for (; c1_it != c1.end() && c2_it != c2.end();
-       c1_it++, c2_it++, ordering_it++) {
-    if (TypedValueCompare(*c1_it, *c2_it)) return *ordering_it == Ordering::ASC;
-    if (TypedValueCompare(*c2_it, *c1_it))
-      return *ordering_it == Ordering::DESC;
-  }
-
-  // at least one collection is exhausted
-  // c1 is less then c2 iff c1 reached the end but c2 didn't
-  return (c1_it == c1.end()) && (c2_it != c2.end());
-}
+}  // namespace impl
 
 template <typename TAccessor>
 void SwitchAccessor(TAccessor &accessor, GraphView graph_view) {

src/query/common.hpp

@@ -30,6 +30,10 @@ enum class GraphView { OLD, NEW };
 /// @throw ReconstructionException if any reconstruction failed.
 void ReconstructTypedValue(TypedValue &value);
 
+namespace impl {
+bool TypedValueCompare(const TypedValue &a, const TypedValue &b);
+}  // namespace impl
+
 /// Custom Comparator type for comparing vectors of TypedValues.
 ///
 /// Does lexicographical ordering of elements based on the above
@@ -40,8 +44,30 @@ class TypedValueVectorCompare final {
   TypedValueVectorCompare() {}
   explicit TypedValueVectorCompare(const std::vector<Ordering> &ordering)
       : ordering_(ordering) {}
-  bool operator()(const std::vector<TypedValue> &c1,
-                  const std::vector<TypedValue> &c2) const;
+
+  template <class TAllocator>
+  bool operator()(const std::vector<TypedValue, TAllocator> &c1,
+                  const std::vector<TypedValue, TAllocator> &c2) const {
+    // ordering is invalid if there are more elements in the collections
+    // then there are in the ordering_ vector
+    DCHECK(c1.size() <= ordering_.size() && c2.size() <= ordering_.size())
+        << "Collections contain more elements then there are orderings";
+
+    auto c1_it = c1.begin();
+    auto c2_it = c2.begin();
+    auto ordering_it = ordering_.begin();
+    for (; c1_it != c1.end() && c2_it != c2.end();
+         c1_it++, c2_it++, ordering_it++) {
+      if (impl::TypedValueCompare(*c1_it, *c2_it))
+        return *ordering_it == Ordering::ASC;
+      if (impl::TypedValueCompare(*c2_it, *c1_it))
+        return *ordering_it == Ordering::DESC;
+    }
+
+    // at least one collection is exhausted
+    // c1 is less then c2 iff c1 reached the end but c2 didn't
+    return (c1_it == c1.end()) && (c2_it != c2.end());
+  }
 
   // TODO: Remove this, member is public
   const auto &ordering() const { return ordering_; }

src/query/plan/operator.cpp

@@ -2866,7 +2866,9 @@ class OrderByCursor : public Cursor {
  public:
   OrderByCursor(const OrderBy &self, database::GraphDbAccessor *db,
                 utils::MemoryResource *mem)
-      : self_(self), input_cursor_(self_.input_->MakeCursor(db, mem)) {}
+      : self_(self),
+        input_cursor_(self_.input_->MakeCursor(db, mem)),
+        cache_(mem) {}
 
   bool Pull(Frame &frame, ExecutionContext &context) override {
     SCOPED_PROFILE_OP("OrderBy");
@@ -2875,26 +2877,27 @@ class OrderByCursor : public Cursor {
       ExpressionEvaluator evaluator(&frame, context.symbol_table,
                                     context.evaluation_context,
                                     context.db_accessor, GraphView::OLD);
+      auto *mem = cache_.get_allocator().GetMemoryResource();
       while (input_cursor_->Pull(frame, context)) {
         // collect the order_by elements
-        std::vector<TypedValue> order_by;
+        std::vector<TypedValue, utils::Allocator<TypedValue>> order_by(mem);
         order_by.reserve(self_.order_by_.size());
         for (auto expression_ptr : self_.order_by_) {
           order_by.emplace_back(expression_ptr->Accept(evaluator));
         }
 
         // collect the output elements
-        std::vector<TypedValue> output;
+        std::vector<TypedValue, utils::Allocator<TypedValue>> output(mem);
         output.reserve(self_.output_symbols_.size());
         for (const Symbol &output_sym : self_.output_symbols_)
           output.emplace_back(frame[output_sym]);
 
-        cache_.emplace_back(std::move(order_by), std::move(output));
+        cache_.push_back(Element{std::move(order_by), std::move(output)});
       }
 
       std::sort(cache_.begin(), cache_.end(),
                 [this](const auto &pair1, const auto &pair2) {
-                  return self_.compare_(pair1.first, pair2.first);
+                  return self_.compare_(pair1.order_by, pair2.order_by);
                 });
 
       did_pull_all_ = true;
@@ -2906,11 +2909,11 @@ class OrderByCursor : public Cursor {
     if (context.db_accessor->should_abort()) throw HintedAbortError();
 
     // place the output values on the frame
-    DCHECK(self_.output_symbols_.size() == cache_it_->second.size())
+    DCHECK(self_.output_symbols_.size() == cache_it_->remember.size())
         << "Number of values does not match the number of output symbols "
            "in OrderBy";
     auto output_sym_it = self_.output_symbols_.begin();
-    for (const TypedValue &output : cache_it_->second)
+    for (const TypedValue &output : cache_it_->remember)
       frame[*output_sym_it++] = output;
 
     cache_it_++;
@@ -2926,15 +2929,17 @@ class OrderByCursor : public Cursor {
   }
 
  private:
+  struct Element {
+    std::vector<TypedValue, utils::Allocator<TypedValue>> order_by;
+    std::vector<TypedValue, utils::Allocator<TypedValue>> remember;
+  };
+
   const OrderBy &self_;
   const UniqueCursorPtr input_cursor_;
   bool did_pull_all_{false};
   // a cache of elements pulled from the input
-  // first pair element is the order-by vector
-  // second pair is the remember vector
-  // the cache is filled and sorted (only on first pair elem) on first Pull
-  std::vector<std::pair<std::vector<TypedValue>, std::vector<TypedValue>>>
-      cache_;
+  // the cache is filled and sorted (only on first elem) on first Pull
+  std::vector<Element, utils::Allocator<Element>> cache_;
   // iterator over the cache_, maintains state between Pulls
   decltype(cache_.begin()) cache_it_ = cache_.begin();
 };

tests/benchmark/query/execution.cpp

@@ -341,4 +341,50 @@ BENCHMARK_TEMPLATE(Aggregate, MonotonicBufferResource)
     ->Ranges({{4, 1U << 7U}, {512, 1U << 13U}})
     ->Unit(benchmark::kMicrosecond);
 
+template <class TMemory>
+// NOLINTNEXTLINE(google-runtime-references)
+static void OrderBy(benchmark::State &state) {
+  query::AstStorage ast;
+  query::Parameters parameters;
+  database::GraphDb db;
+  AddVertices(&db, state.range(1));
+  query::SymbolTable symbol_table;
+  auto scan_all = std::make_shared<query::plan::ScanAll>(
+      nullptr, symbol_table.CreateSymbol("v", false));
+  std::vector<query::Symbol> symbols;
+  symbols.reserve(state.range(0));
+  // NOLINTNEXTLINE(cert-msc32-c,cert-msc51-cpp)
+  std::mt19937_64 rg(42);
+  std::vector<query::SortItem> sort_items;
+  sort_items.reserve(state.range(0));
+  for (int i = 0; i < state.range(0); ++i) {
+    symbols.push_back(symbol_table.CreateSymbol(std::to_string(i), false));
+    auto rand_value = utils::MemcpyCast<int64_t>(rg());
+    sort_items.push_back({query::Ordering::ASC,
+                          ast.Create<query::PrimitiveLiteral>(rand_value)});
+  }
+  query::plan::OrderBy order_by(scan_all, sort_items, symbols);
+  auto dba = db.Access();
+  query::Frame frame(symbol_table.max_position());
+  // Nothing should be used from the EvaluationContext, so leave it empty.
+  query::EvaluationContext evaluation_context;
+  while (state.KeepRunning()) {
+    query::ExecutionContext execution_context{&dba, symbol_table,
+                                              evaluation_context};
+    TMemory memory;
+    auto cursor = order_by.MakeCursor(&dba, memory.get());
+    while (cursor->Pull(frame, execution_context))
+      ;
+  }
+  state.SetItemsProcessed(state.iterations());
+}
+
+BENCHMARK_TEMPLATE(OrderBy, NewDeleteResource)
+    ->Ranges({{4, 1U << 7U}, {512, 1U << 13U}})
+    ->Unit(benchmark::kMicrosecond);
+
+BENCHMARK_TEMPLATE(OrderBy, MonotonicBufferResource)
+    ->Ranges({{4, 1U << 7U}, {512, 1U << 13U}})
+    ->Unit(benchmark::kMicrosecond);
+
 BENCHMARK_MAIN();