From 8353228ba7690f0aad6b2bbb3d46ff31e600a0f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A1nos=20Benjamin=20Antal?= Date: Tue, 25 Oct 2022 13:10:03 +0200 Subject: [PATCH 01/13] Upgrade json lib to 3.11.2 --- libs/setup.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/setup.sh b/libs/setup.sh index d902aadb1..bb737d432 100755 --- a/libs/setup.sh +++ b/libs/setup.sh @@ -116,7 +116,7 @@ declare -A primary_urls=( ["pymgclient"]="http://$local_cache_host/git/pymgclient.git" ["mgconsole"]="http://$local_cache_host/git/mgconsole.git" ["spdlog"]="http://$local_cache_host/git/spdlog" - ["nlohmann"]="http://$local_cache_host/file/nlohmann/json/4f8fba14066156b73f1189a2b8bd568bde5284c5/single_include/nlohmann/json.hpp" + ["nlohmann"]="http://$local_cache_host/file/nlohmann/json/9d69186291aca4f0137b69c1dee313b391ff564c/single_include/nlohmann/json.hpp" ["neo4j"]="http://$local_cache_host/file/neo4j-community-3.2.3-unix.tar.gz" ["librdkafka"]="http://$local_cache_host/git/librdkafka.git" ["protobuf"]="http://$local_cache_host/git/protobuf.git" @@ -141,7 +141,7 @@ declare -A secondary_urls=( ["pymgclient"]="https://github.com/memgraph/pymgclient.git" ["mgconsole"]="http://github.com/memgraph/mgconsole.git" ["spdlog"]="https://github.com/gabime/spdlog" - ["nlohmann"]="https://raw.githubusercontent.com/nlohmann/json/4f8fba14066156b73f1189a2b8bd568bde5284c5/single_include/nlohmann/json.hpp" + ["nlohmann"]="https://raw.githubusercontent.com/nlohmann/json/9d69186291aca4f0137b69c1dee313b391ff564c/single_include/nlohmann/json.hpp" ["neo4j"]="https://s3-eu-west-1.amazonaws.com/deps.memgraph.io/neo4j-community-3.2.3-unix.tar.gz" ["librdkafka"]="https://github.com/edenhill/librdkafka.git" ["protobuf"]="https://github.com/protocolbuffers/protobuf.git" From 1e4c02f8a50f1f29dfdceaab4aaf4df9854dd6ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A1nos=20Benjamin=20Antal?= Date: Tue, 25 Oct 2022 13:37:27 +0200 Subject: [PATCH 02/13] Make profile query work --- src/query/v2/interpreter.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/query/v2/interpreter.cpp b/src/query/v2/interpreter.cpp index c02497030..0ba015217 100644 --- a/src/query/v2/interpreter.cpp +++ b/src/query/v2/interpreter.cpp @@ -1034,6 +1034,7 @@ PreparedQuery PrepareProfileQuery(ParsedQuery parsed_query, bool in_explicit_tra const auto memory_limit = expr::EvaluateMemoryLimit(&evaluator, cypher_query->memory_limit_, cypher_query->memory_scale_); + shard_request_manager->StartTransaction(); auto cypher_query_plan = CypherQueryToPlan( parsed_inner_query.stripped_query.hash(), std::move(parsed_inner_query.ast_storage), cypher_query, parsed_inner_query.parameters, parsed_inner_query.is_cacheable ? &interpreter_context->plan_cache : nullptr, From ee64684b0b1f3a1af1376ce1bd9b31cdca56a843 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A1nos=20Benjamin=20Antal?= Date: Tue, 25 Oct 2022 13:37:57 +0200 Subject: [PATCH 03/13] Add `ScopedCustomProfile` --- src/query/v2/plan/profile.hpp | 5 +++- src/query/v2/plan/scoped_profile.hpp | 44 ++++++++++++++++++++++++++-- 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/src/query/v2/plan/profile.hpp b/src/query/v2/plan/profile.hpp index 74437f82f..c18ae1e56 100644 --- a/src/query/v2/plan/profile.hpp +++ b/src/query/v2/plan/profile.hpp @@ -26,10 +26,13 @@ namespace plan { * Stores profiling statistics for a single logical operator. */ struct ProfilingStats { + static constexpr std::string_view kNumCycles{"num_cycles"}; + int64_t actual_hits{0}; - unsigned long long num_cycles{0}; + uint64_t num_cycles{0}; uint64_t key{0}; const char *name{nullptr}; + nlohmann::json custom_data; // TODO: This should use the allocator for query execution std::vector children; }; diff --git a/src/query/v2/plan/scoped_profile.hpp b/src/query/v2/plan/scoped_profile.hpp index e71cbb047..f70701c4c 100644 --- a/src/query/v2/plan/scoped_profile.hpp +++ b/src/query/v2/plan/scoped_profile.hpp @@ -13,6 +13,8 @@ #include +#include + #include "query/v2/context.hpp" #include "query/v2/plan/profile.hpp" #include "utils/likely.hpp" @@ -20,6 +22,35 @@ namespace memgraph::query::v2::plan { +class ScopedCustomProfile { + public: + explicit ScopedCustomProfile(const std::string_view custom_data_name, ExecutionContext &context) + : custom_data_name_(custom_data_name), start_time_{utils::ReadTSC()}, context_{&context} {} + + ScopedCustomProfile(const ScopedCustomProfile &) = delete; + ScopedCustomProfile(ScopedCustomProfile &&) = delete; + ScopedCustomProfile &operator=(const ScopedCustomProfile &) = delete; + ScopedCustomProfile &operator=(ScopedCustomProfile &&) = delete; + + ~ScopedCustomProfile() { + if (nullptr != context_->stats_root) { + auto &custom_data = context_->stats_root->custom_data[custom_data_name_]; + if (!custom_data.is_object()) { + custom_data = nlohmann::json::object(); + } + const auto elapsed = utils::ReadTSC() - start_time_; + auto &num_cycles_json = custom_data[ProfilingStats::kNumCycles]; + const auto num_cycles = num_cycles_json.is_null() ? 0 : num_cycles_json.get(); + num_cycles_json = num_cycles + elapsed; + } + } + + private: + std::string_view custom_data_name_; + uint64_t start_time_; + ExecutionContext *context_; +}; + /** * A RAII class used for profiling logical operators. Instances of this class * update the profiling data stored within the `ExecutionContext` object and build @@ -29,7 +60,7 @@ namespace memgraph::query::v2::plan { class ScopedProfile { public: ScopedProfile(uint64_t key, const char *name, query::v2::ExecutionContext *context) noexcept : context_(context) { - if (UNLIKELY(context_->is_profile_query)) { + if (UNLIKELY(IsProfiling())) { root_ = context_->stats_root; // Are we the root logical operator? @@ -60,8 +91,13 @@ class ScopedProfile { } } + ScopedProfile(const ScopedProfile &) = delete; + ScopedProfile(ScopedProfile &&) = delete; + ScopedProfile &operator=(const ScopedProfile &) = delete; + ScopedProfile &operator=(ScopedProfile &&) = delete; + ~ScopedProfile() noexcept { - if (UNLIKELY(context_->is_profile_query)) { + if (UNLIKELY(IsProfiling())) { stats_->num_cycles += utils::ReadTSC() - start_time_; // Restore the old root ("pop") @@ -70,10 +106,12 @@ class ScopedProfile { } private: + [[nodiscard]] bool IsProfiling() const { return context_->is_profile_query; } + query::v2::ExecutionContext *context_; ProfilingStats *root_{nullptr}; ProfilingStats *stats_{nullptr}; - unsigned long long start_time_{0}; + uint64_t start_time_{0}; }; } // namespace memgraph::query::v2::plan From 5939fb2b0cadd14bfee3081f57625ab709e533ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A1nos=20Benjamin=20Antal?= Date: Tue, 25 Oct 2022 14:35:26 +0200 Subject: [PATCH 04/13] Start transaction properly --- src/query/v2/interpreter.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/query/v2/interpreter.cpp b/src/query/v2/interpreter.cpp index 0ba015217..0fd11fdde 100644 --- a/src/query/v2/interpreter.cpp +++ b/src/query/v2/interpreter.cpp @@ -901,7 +901,6 @@ PreparedQuery PrepareCypherQuery(ParsedQuery parsed_query, std::mapStartTransaction(); auto plan = CypherQueryToPlan( parsed_query.stripped_query.hash(), std::move(parsed_query.ast_storage), cypher_query, parsed_query.parameters, parsed_query.is_cacheable ? &interpreter_context->plan_cache : nullptr, shard_request_manager); @@ -1034,7 +1033,6 @@ PreparedQuery PrepareProfileQuery(ParsedQuery parsed_query, bool in_explicit_tra const auto memory_limit = expr::EvaluateMemoryLimit(&evaluator, cypher_query->memory_limit_, cypher_query->memory_scale_); - shard_request_manager->StartTransaction(); auto cypher_query_plan = CypherQueryToPlan( parsed_inner_query.stripped_query.hash(), std::move(parsed_inner_query.ast_storage), cypher_query, parsed_inner_query.parameters, parsed_inner_query.is_cacheable ? &interpreter_context->plan_cache : nullptr, @@ -1512,6 +1510,11 @@ Interpreter::PrepareResult Interpreter::Prepare(const std::string &query_string, ParsedQuery parsed_query = ParseQuery(query_string, params, &interpreter_context_->ast_cache, interpreter_context_->config.query); query_execution->summary["parsing_time"] = parsing_timer.Elapsed().count(); + if (!in_explicit_transaction_ && + (utils::Downcast(parsed_query.query) || utils::Downcast(parsed_query.query) || + utils::Downcast(parsed_query.query))) { + shard_request_manager_->StartTransaction(); + } utils::Timer planning_timer; PreparedQuery prepared_query; From 8ebc7048197d28a8c0443bdc9338e6948ec2b703 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A1nos=20Benjamin=20Antal?= Date: Tue, 25 Oct 2022 14:37:18 +0200 Subject: [PATCH 05/13] Fix profile queries with ScanAll --- src/query/v2/plan/operator.cpp | 36 ++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/src/query/v2/plan/operator.cpp b/src/query/v2/plan/operator.cpp index 3c36559c9..62a927cfd 100644 --- a/src/query/v2/plan/operator.cpp +++ b/src/query/v2/plan/operator.cpp @@ -370,27 +370,29 @@ class DistributedScanAllAndFilterCursor : public Cursor { bool Pull(Frame &frame, ExecutionContext &context) override { SCOPED_PROFILE_OP(op_name_); auto &shard_manager = *context.shard_request_manager; - if (MustAbort(context)) { - throw HintedAbortError(); - } - using State = msgs::ExecutionState; - - if (request_state_.state == State::INITIALIZING) { - if (!input_cursor_->Pull(frame, context)) { - return false; + while (true) { + if (MustAbort(context)) { + throw HintedAbortError(); } - } + using State = msgs::ExecutionState; - if (current_vertex_it == current_batch.end()) { - if (request_state_.state == State::COMPLETED || !MakeRequest(shard_manager)) { - ResetExecutionState(); - return Pull(frame, context); + if (request_state_.state == State::INITIALIZING) { + if (!input_cursor_->Pull(frame, context)) { + return false; + } } - } - frame[output_symbol_] = TypedValue(std::move(*current_vertex_it)); - ++current_vertex_it; - return true; + if (current_vertex_it == current_batch.end()) { + if (request_state_.state == State::COMPLETED || !MakeRequest(shard_manager)) { + ResetExecutionState(); + continue; + } + } + + frame[output_symbol_] = TypedValue(std::move(*current_vertex_it)); + ++current_vertex_it; + return true; + } } void Shutdown() override { input_cursor_->Shutdown(); } From 5784c0d473f364f5860a1a8df60098ed43f3ddc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A1nos=20Benjamin=20Antal?= Date: Tue, 25 Oct 2022 15:26:42 +0200 Subject: [PATCH 06/13] Return the custom data for profile queries --- src/query/v2/interpreter.cpp | 2 +- src/query/v2/plan/profile.cpp | 65 ++++++++++++++++++++++++----------- src/query/v2/plan/profile.hpp | 2 ++ 3 files changed, 47 insertions(+), 22 deletions(-) diff --git a/src/query/v2/interpreter.cpp b/src/query/v2/interpreter.cpp index 0fd11fdde..f82d5ee01 100644 --- a/src/query/v2/interpreter.cpp +++ b/src/query/v2/interpreter.cpp @@ -1040,7 +1040,7 @@ PreparedQuery PrepareProfileQuery(ParsedQuery parsed_query, bool in_explicit_tra auto rw_type_checker = plan::ReadWriteTypeChecker(); rw_type_checker.InferRWType(const_cast(cypher_query_plan->plan())); - return PreparedQuery{{"OPERATOR", "ACTUAL HITS", "RELATIVE TIME", "ABSOLUTE TIME"}, + return PreparedQuery{{"OPERATOR", "ACTUAL HITS", "RELATIVE TIME", "ABSOLUTE TIME", "CUSTOM DATA"}, std::move(parsed_query.required_privileges), [plan = std::move(cypher_query_plan), parameters = std::move(parsed_inner_query.parameters), summary, dba, interpreter_context, execution_memory, memory_limit, shard_request_manager, diff --git a/src/query/v2/plan/profile.cpp b/src/query/v2/plan/profile.cpp index 26c5280d4..3bd0953ba 100644 --- a/src/query/v2/plan/profile.cpp +++ b/src/query/v2/plan/profile.cpp @@ -24,18 +24,17 @@ namespace memgraph::query::v2::plan { namespace { -unsigned long long IndividualCycles(const ProfilingStats &cumulative_stats) { +uint64_t IndividualCycles(const ProfilingStats &cumulative_stats) { return cumulative_stats.num_cycles - std::accumulate(cumulative_stats.children.begin(), cumulative_stats.children.end(), 0ULL, [](auto acc, auto &stats) { return acc + stats.num_cycles; }); } -double RelativeTime(unsigned long long num_cycles, unsigned long long total_cycles) { - return static_cast(num_cycles) / total_cycles; +double RelativeTime(uint64_t num_cycles, uint64_t total_cycles) { + return static_cast(num_cycles) / static_cast(total_cycles); } -double AbsoluteTime(unsigned long long num_cycles, unsigned long long total_cycles, - std::chrono::duration total_time) { +double AbsoluteTime(uint64_t num_cycles, uint64_t total_cycles, std::chrono::duration total_time) { return (RelativeTime(num_cycles, total_cycles) * static_cast>(total_time)) .count(); } @@ -50,26 +49,29 @@ namespace { class ProfilingStatsToTableHelper { public: - ProfilingStatsToTableHelper(unsigned long long total_cycles, std::chrono::duration total_time) + ProfilingStatsToTableHelper(uint64_t total_cycles, std::chrono::duration total_time) : total_cycles_(total_cycles), total_time_(total_time) {} void Output(const ProfilingStats &cumulative_stats) { auto cycles = IndividualCycles(cumulative_stats); + auto custom_data_copy = cumulative_stats.custom_data; + ConvertCyclesToTime(custom_data_copy); - rows_.emplace_back(std::vector{ - TypedValue(FormatOperator(cumulative_stats.name)), TypedValue(cumulative_stats.actual_hits), - TypedValue(FormatRelativeTime(cycles)), TypedValue(FormatAbsoluteTime(cycles))}); + rows_.emplace_back( + std::vector{TypedValue(FormatOperator(cumulative_stats.name)), + TypedValue(cumulative_stats.actual_hits), TypedValue(FormatRelativeTime(cycles)), + TypedValue(FormatAbsoluteTime(cycles)), TypedValue(custom_data_copy.dump())}); for (size_t i = 1; i < cumulative_stats.children.size(); ++i) { Branch(cumulative_stats.children[i]); } - if (cumulative_stats.children.size() >= 1) { + if (!cumulative_stats.children.empty()) { Output(cumulative_stats.children[0]); } } - std::vector> rows() { return rows_; } + std::vector> rows() const { return rows_; } private: void Branch(const ProfilingStats &cumulative_stats) { @@ -80,7 +82,28 @@ class ProfilingStatsToTableHelper { --depth_; } - std::string Format(const char *str) { + double AbsoluteTime(const uint64_t cycles) const { return plan::AbsoluteTime(cycles, total_cycles_, total_time_); } + + double RelativeTime(const uint64_t cycles) const { return plan::RelativeTime(cycles, total_cycles_); } + + void ConvertCyclesToTime(nlohmann::json &custom_data) const { + const auto convert_cycles_in_json = [this](nlohmann::json &json) { + if (!json.is_object()) { + return; + } + if (auto it = json.find(ProfilingStats::kNumCycles); it != json.end()) { + auto num_cycles = it.value().get(); + json[ProfilingStats::kAbsoluteTime] = AbsoluteTime(num_cycles); + json[ProfilingStats::kRelativeTime] = RelativeTime(num_cycles); + } + }; + + for (auto &json : custom_data) { + convert_cycles_in_json(json); + } + } + + std::string Format(const char *str) const { std::ostringstream ss; for (int64_t i = 0; i < depth_; ++i) { ss << "| "; @@ -89,21 +112,21 @@ class ProfilingStatsToTableHelper { return ss.str(); } - std::string Format(const std::string &str) { return Format(str.c_str()); } + std::string Format(const std::string &str) const { return Format(str.c_str()); } - std::string FormatOperator(const char *str) { return Format(std::string("* ") + str); } + std::string FormatOperator(const char *str) const { return Format(std::string("* ") + str); } - std::string FormatRelativeTime(unsigned long long num_cycles) { - return fmt::format("{: 10.6f} %", RelativeTime(num_cycles, total_cycles_) * 100); + std::string FormatRelativeTime(uint64_t num_cycles) const { + return fmt::format("{: 10.6f} %", RelativeTime(num_cycles) * 100); } - std::string FormatAbsoluteTime(unsigned long long num_cycles) { - return fmt::format("{: 10.6f} ms", AbsoluteTime(num_cycles, total_cycles_, total_time_)); + std::string FormatAbsoluteTime(uint64_t num_cycles) const { + return fmt::format("{: 10.6f} ms", AbsoluteTime(num_cycles)); } int64_t depth_{0}; std::vector> rows_; - unsigned long long total_cycles_; + uint64_t total_cycles_; std::chrono::duration total_time_; }; @@ -126,7 +149,7 @@ class ProfilingStatsToJsonHelper { using json = nlohmann::json; public: - ProfilingStatsToJsonHelper(unsigned long long total_cycles, std::chrono::duration total_time) + ProfilingStatsToJsonHelper(uint64_t total_cycles, std::chrono::duration total_time) : total_cycles_(total_cycles), total_time_(total_time) {} void Output(const ProfilingStats &cumulative_stats) { return Output(cumulative_stats, &json_); } @@ -151,7 +174,7 @@ class ProfilingStatsToJsonHelper { } json json_; - unsigned long long total_cycles_; + uint64_t total_cycles_; std::chrono::duration total_time_; }; diff --git a/src/query/v2/plan/profile.hpp b/src/query/v2/plan/profile.hpp index c18ae1e56..da25be33c 100644 --- a/src/query/v2/plan/profile.hpp +++ b/src/query/v2/plan/profile.hpp @@ -27,6 +27,8 @@ namespace plan { */ struct ProfilingStats { static constexpr std::string_view kNumCycles{"num_cycles"}; + static constexpr std::string_view kRelativeTime{"relative_time"}; + static constexpr std::string_view kAbsoluteTime{"absolute_time"}; int64_t actual_hits{0}; uint64_t num_cycles{0}; From 1703cd039d0310cca06d5dd67f23335489f89a7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A1nos=20Benjamin=20Antal?= Date: Tue, 25 Oct 2022 20:15:06 +0200 Subject: [PATCH 07/13] Populate custom data of profile query with request wait times --- src/query/v2/plan/operator.cpp | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/src/query/v2/plan/operator.cpp b/src/query/v2/plan/operator.cpp index 62a927cfd..692638338 100644 --- a/src/query/v2/plan/operator.cpp +++ b/src/query/v2/plan/operator.cpp @@ -155,7 +155,13 @@ uint64_t ComputeProfilingKey(const T *obj) { } // namespace -#define SCOPED_PROFILE_OP(name) ScopedProfile profile{ComputeProfilingKey(this), name, &context}; +#define SCOPED_PROFILE_OP(name) \ + ScopedProfile profile { ComputeProfilingKey(this), name, &context } + +#define SCOPED_CUSTOM_PROFILE(name) \ + ScopedCustomProfile custom_profile { name, context } + +#define SCOPED_REQUEST_WAIT_PROFILE SCOPED_CUSTOM_PROFILE("request_wait") class DistributedCreateNodeCursor : public Cursor { public: @@ -168,7 +174,10 @@ class DistributedCreateNodeCursor : public Cursor { SCOPED_PROFILE_OP("CreateNode"); if (input_cursor_->Pull(frame, context)) { auto &shard_manager = context.shard_request_manager; - shard_manager->Request(state_, NodeCreationInfoToRequest(context, frame)); + { + SCOPED_REQUEST_WAIT_PROFILE; + shard_manager->Request(state_, NodeCreationInfoToRequest(context, frame)); + } return true; } @@ -361,14 +370,18 @@ class DistributedScanAllAndFilterCursor : public Cursor { using VertexAccessor = accessors::VertexAccessor; - bool MakeRequest(msgs::ShardRequestManagerInterface &shard_manager) { - current_batch = shard_manager.Request(request_state_); + bool MakeRequest(msgs::ShardRequestManagerInterface &shard_manager, ExecutionContext &context) { + { + SCOPED_REQUEST_WAIT_PROFILE; + current_batch = shard_manager.Request(request_state_); + } current_vertex_it = current_batch.begin(); return !current_batch.empty(); } bool Pull(Frame &frame, ExecutionContext &context) override { SCOPED_PROFILE_OP(op_name_); + auto &shard_manager = *context.shard_request_manager; while (true) { if (MustAbort(context)) { @@ -383,7 +396,7 @@ class DistributedScanAllAndFilterCursor : public Cursor { } if (current_vertex_it == current_batch.end()) { - if (request_state_.state == State::COMPLETED || !MakeRequest(shard_manager)) { + if (request_state_.state == State::COMPLETED || !MakeRequest(shard_manager, context)) { ResetExecutionState(); continue; } @@ -2367,7 +2380,10 @@ class DistributedCreateExpandCursor : public Cursor { } auto &shard_manager = context.shard_request_manager; ResetExecutionState(); - shard_manager->Request(state_, ExpandCreationInfoToRequest(context, frame)); + { + SCOPED_REQUEST_WAIT_PROFILE; + shard_manager->Request(state_, ExpandCreationInfoToRequest(context, frame)); + } return true; } @@ -2498,7 +2514,10 @@ class DistributedExpandCursor : public Cursor { request.edge_properties.emplace(); request.src_vertices.push_back(vertex.Id()); msgs::ExecutionState request_state; - auto result_rows = context.shard_request_manager->Request(request_state, std::move(request)); + auto result_rows = std::invoke([&context, &request_state, &request]() mutable { + SCOPED_REQUEST_WAIT_PROFILE; + return context.shard_request_manager->Request(request_state, std::move(request)); + }); MG_ASSERT(result_rows.size() == 1); auto &result_row = result_rows.front(); From 6a31c49432a922c6233f082bb3e1c65bf7476b44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A1nos=20Benjamin=20Antal?= Date: Tue, 25 Oct 2022 20:30:29 +0200 Subject: [PATCH 08/13] Flatten nested conditional statements --- src/query/v2/plan/operator.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/query/v2/plan/operator.cpp b/src/query/v2/plan/operator.cpp index 0415c9d7d..64362bf18 100644 --- a/src/query/v2/plan/operator.cpp +++ b/src/query/v2/plan/operator.cpp @@ -397,11 +397,10 @@ class DistributedScanAllAndFilterCursor : public Cursor { request_state_.label = label_.has_value() ? std::make_optional(shard_manager.LabelToName(*label_)) : std::nullopt; - if (current_vertex_it == current_batch.end()) { - if (request_state_.state == State::COMPLETED || !MakeRequest(shard_manager, context)) { - ResetExecutionState(); - continue; - } + if (current_vertex_it == current_batch.end() && + (request_state_.state == State::COMPLETED || !MakeRequest(shard_manager, context))) { + ResetExecutionState(); + continue; } frame[output_symbol_] = TypedValue(std::move(*current_vertex_it)); From 39c9c215b130d7bf5001f4495a4c2bd3b0ea7d94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A1nos=20Benjamin=20Antal?= Date: Wed, 26 Oct 2022 11:14:24 +0200 Subject: [PATCH 09/13] Suppress clang-tidy warnings for --- src/query/v2/plan/operator.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/query/v2/plan/operator.cpp b/src/query/v2/plan/operator.cpp index 64362bf18..5af60b9eb 100644 --- a/src/query/v2/plan/operator.cpp +++ b/src/query/v2/plan/operator.cpp @@ -155,12 +155,15 @@ uint64_t ComputeProfilingKey(const T *obj) { } // namespace +// NOLINTNEXTLINE(cppcoreguidelines-macro-usage) #define SCOPED_PROFILE_OP(name) \ ScopedProfile profile { ComputeProfilingKey(this), name, &context } +// NOLINTNEXTLINE(cppcoreguidelines-macro-usage) #define SCOPED_CUSTOM_PROFILE(name) \ ScopedCustomProfile custom_profile { name, context } +// NOLINTNEXTLINE(cppcoreguidelines-macro-usage) #define SCOPED_REQUEST_WAIT_PROFILE SCOPED_CUSTOM_PROFILE("request_wait") class DistributedCreateNodeCursor : public Cursor { From 281ae158ecd2db494082896b335c3ec8e2791b8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A1nos=20Benjamin=20Antal?= Date: Wed, 26 Oct 2022 11:21:27 +0200 Subject: [PATCH 10/13] Make `ReadTSC` `noexcept` --- src/utils/tsc.cpp | 2 +- src/utils/tsc.hpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/utils/tsc.cpp b/src/utils/tsc.cpp index fa87f9754..378c6e9be 100644 --- a/src/utils/tsc.cpp +++ b/src/utils/tsc.cpp @@ -18,7 +18,7 @@ extern "C" { #include "utils/tsc.hpp" namespace memgraph::utils { -uint64_t ReadTSC() { return rdtsc(); } +uint64_t ReadTSC() noexcept { return rdtsc(); } std::optional GetTSCFrequency() { // init is only needed for fetching frequency diff --git a/src/utils/tsc.hpp b/src/utils/tsc.hpp index 40344ddaf..1c0f7a5f0 100644 --- a/src/utils/tsc.hpp +++ b/src/utils/tsc.hpp @@ -18,7 +18,7 @@ namespace memgraph::utils { // TSC stands for Time-Stamp Counter -uint64_t ReadTSC(); +uint64_t ReadTSC() noexcept; std::optional GetTSCFrequency(); From 534e3652710493662f996bcb3eda2853c57bb85b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A1nos=20Benjamin=20Antal?= Date: Wed, 26 Oct 2022 11:22:27 +0200 Subject: [PATCH 11/13] Suppress warning about exception escape for destructor --- src/query/v2/plan/scoped_profile.hpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/query/v2/plan/scoped_profile.hpp b/src/query/v2/plan/scoped_profile.hpp index f70701c4c..8dc228606 100644 --- a/src/query/v2/plan/scoped_profile.hpp +++ b/src/query/v2/plan/scoped_profile.hpp @@ -32,6 +32,9 @@ class ScopedCustomProfile { ScopedCustomProfile &operator=(const ScopedCustomProfile &) = delete; ScopedCustomProfile &operator=(ScopedCustomProfile &&) = delete; + // If an exception is thrown in any of these functions that signals a problem that is much bigger than we could handle + // it here, thus we don't attempt to handle it. + // NOLINTNEXTLINE(bugprone-exception-escape) ~ScopedCustomProfile() { if (nullptr != context_->stats_root) { auto &custom_data = context_->stats_root->custom_data[custom_data_name_]; From d5700ab5ffaa6e1f6d5fad45a1c4142134861d25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A1nos=20Benjamin=20Antal?= Date: Mon, 31 Oct 2022 15:50:50 +0100 Subject: [PATCH 12/13] Use `[[unlikely]]` attribute --- src/query/v2/plan/scoped_profile.hpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/query/v2/plan/scoped_profile.hpp b/src/query/v2/plan/scoped_profile.hpp index 8dc228606..3aa4bc569 100644 --- a/src/query/v2/plan/scoped_profile.hpp +++ b/src/query/v2/plan/scoped_profile.hpp @@ -36,7 +36,7 @@ class ScopedCustomProfile { // it here, thus we don't attempt to handle it. // NOLINTNEXTLINE(bugprone-exception-escape) ~ScopedCustomProfile() { - if (nullptr != context_->stats_root) { + if (nullptr != context_->stats_root) [[unlikely]] { auto &custom_data = context_->stats_root->custom_data[custom_data_name_]; if (!custom_data.is_object()) { custom_data = nlohmann::json::object(); @@ -63,7 +63,7 @@ class ScopedCustomProfile { class ScopedProfile { public: ScopedProfile(uint64_t key, const char *name, query::v2::ExecutionContext *context) noexcept : context_(context) { - if (UNLIKELY(IsProfiling())) { + if (IsProfiling()) [[unlikely]] { root_ = context_->stats_root; // Are we the root logical operator? @@ -100,7 +100,7 @@ class ScopedProfile { ScopedProfile &operator=(ScopedProfile &&) = delete; ~ScopedProfile() noexcept { - if (UNLIKELY(IsProfiling())) { + if (IsProfiling()) [[unlikely]] { stats_->num_cycles += utils::ReadTSC() - start_time_; // Restore the old root ("pop") From f9e2a66961dc36f18558834d5724e6bc90707d39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A1nos=20Benjamin=20Antal?= Date: Mon, 31 Oct 2022 15:52:29 +0100 Subject: [PATCH 13/13] Add `const` qualifier to function parameters --- src/query/v2/plan/profile.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/query/v2/plan/profile.cpp b/src/query/v2/plan/profile.cpp index 3bd0953ba..cb4340ed7 100644 --- a/src/query/v2/plan/profile.cpp +++ b/src/query/v2/plan/profile.cpp @@ -30,11 +30,12 @@ uint64_t IndividualCycles(const ProfilingStats &cumulative_stats) { [](auto acc, auto &stats) { return acc + stats.num_cycles; }); } -double RelativeTime(uint64_t num_cycles, uint64_t total_cycles) { +double RelativeTime(const uint64_t num_cycles, const uint64_t total_cycles) { return static_cast(num_cycles) / static_cast(total_cycles); } -double AbsoluteTime(uint64_t num_cycles, uint64_t total_cycles, std::chrono::duration total_time) { +double AbsoluteTime(const uint64_t num_cycles, const uint64_t total_cycles, + const std::chrono::duration total_time) { return (RelativeTime(num_cycles, total_cycles) * static_cast>(total_time)) .count(); }