diff --git a/src/query/common.cpp b/src/query/common.cpp index 15e21e2d1..defa54c9b 100644 --- a/src/query/common.cpp +++ b/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 &c1, - const std::vector &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 void SwitchAccessor(TAccessor &accessor, GraphView graph_view) { diff --git a/src/query/common.hpp b/src/query/common.hpp index 80c40c964..d1bbe7a00 100644 --- a/src/query/common.hpp +++ b/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) {} - bool operator()(const std::vector &c1, - const std::vector &c2) const; + + template + bool operator()(const std::vector &c1, + const std::vector &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_; } diff --git a/src/query/plan/operator.cpp b/src/query/plan/operator.cpp index 3feef35cb..6da267d90 100644 --- a/src/query/plan/operator.cpp +++ b/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 order_by; + std::vector> 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 output; + std::vector> 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> order_by; + std::vector> 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::vector>> - cache_; + // the cache is filled and sorted (only on first elem) on first Pull + std::vector> cache_; // iterator over the cache_, maintains state between Pulls decltype(cache_.begin()) cache_it_ = cache_.begin(); }; diff --git a/tests/benchmark/query/execution.cpp b/tests/benchmark/query/execution.cpp index e4b2f71b3..d9a3143e8 100644 --- a/tests/benchmark/query/execution.cpp +++ b/tests/benchmark/query/execution.cpp @@ -341,4 +341,50 @@ BENCHMARK_TEMPLATE(Aggregate, MonotonicBufferResource) ->Ranges({{4, 1U << 7U}, {512, 1U << 13U}}) ->Unit(benchmark::kMicrosecond); +template +// 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( + nullptr, symbol_table.CreateSymbol("v", false)); + std::vector symbols; + symbols.reserve(state.range(0)); + // NOLINTNEXTLINE(cert-msc32-c,cert-msc51-cpp) + std::mt19937_64 rg(42); + std::vector 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(rg()); + sort_items.push_back({query::Ordering::ASC, + ast.Create(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();