From a6f625676bd2a64c280440c461b5877651731454 Mon Sep 17 00:00:00 2001 From: yiguolei <676222867@qq.com> Date: Mon, 12 Jun 2023 10:00:35 +0800 Subject: [PATCH] [profile](remove child) child is for node, should not be used to organize counters (#20676) Currently, there are many profiles using add child profile to orgnanize profile into blocks. But it is wrong. Child profile will have a total time counter. Actually, what we should use is just a label. - MemoryUsage: - HashTable: 23.98 KB - SerializeKeyArena: 446.75 KB Add a new macro ADD_LABEL_COUNTER to add just a label in the profile. --------- Co-authored-by: yiguolei --- be/src/util/json_util.h | 64 ------------------- be/src/util/pretty_printer.h | 6 +- be/src/util/runtime_profile.h | 1 + be/src/vec/exec/join/vhash_join_node.cpp | 23 +++---- be/src/vec/exec/join/vhash_join_node.h | 1 + be/src/vec/exec/scan/vscan_node.cpp | 8 +-- be/src/vec/exec/scan/vscan_node.h | 1 + be/src/vec/exec/vaggregation_node.cpp | 10 +-- be/src/vec/exec/vaggregation_node.h | 1 + be/src/vec/exec/vanalytic_eval_node.cpp | 6 +- be/src/vec/exec/vanalytic_eval_node.h | 1 + be/src/vec/exec/vsort_node.cpp | 6 +- be/src/vec/exec/vsort_node.h | 1 + be/src/vec/runtime/vdata_stream_recvr.cpp | 5 +- be/src/vec/runtime/vdata_stream_recvr.h | 1 + .../doris/common/util/RuntimeProfile.java | 4 ++ gensrc/thrift/Metrics.thrift | 2 + 17 files changed, 44 insertions(+), 97 deletions(-) delete mode 100644 be/src/util/json_util.h diff --git a/be/src/util/json_util.h b/be/src/util/json_util.h deleted file mode 100644 index a2aba38c59..0000000000 --- a/be/src/util/json_util.h +++ /dev/null @@ -1,64 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. -// This file is copied from -// https://github.com/apache/impala/blob/branch-2.9.0/be/src/util/json-util.h -// and modified by Doris - -#pragma once - -#include -#include - -#include - -#include "util/pretty_printer.h" - -namespace doris { - -/// ToJsonValue() converts 'value' into a rapidjson::Value in 'out_val'. The type of -/// 'out_val' depends on the value of 'type'. If type != TUnit::NONE and 'value' is -/// numeric, 'value' will be set to a string which is the pretty-printed representation of -/// 'value' (e.g. conversion to MB etc). Otherwise the value is directly copied into -/// 'out_val'. -template -typename std::enable_if::value, void>::type ToJsonValue( - const T& value, const TUnit::type unit, rapidjson::Document* document, - rapidjson::Value* out_val) { - *out_val = value; -} - -/// Specialisation for std::string which requires explicit use of 'document's allocator to -/// copy into out_val. -template <> -void ToJsonValue(const std::string& value, const TUnit::type unit, - rapidjson::Document* document, rapidjson::Value* out_val); - -/// Does pretty-printing if 'value' is numeric, and type is not NONE, otherwise constructs -/// a json object containing 'value' as a literal. -template -typename boost::enable_if_c::value, void>::type ToJsonValue( - const T& value, const TUnit::type unit, rapidjson::Document* document, - rapidjson::Value* out_val) { - if (unit != TUnit::NONE) { - const std::string& s = PrettyPrinter::print(value, unit); - ToJsonValue(s, unit, document, out_val); - } else { - *out_val = value; - } -} - -} // namespace doris diff --git a/be/src/util/pretty_printer.h b/be/src/util/pretty_printer.h index 2f48eae33a..5e0d6fdfc9 100644 --- a/be/src/util/pretty_printer.h +++ b/be/src/util/pretty_printer.h @@ -56,7 +56,11 @@ public: ss.flags(std::ios::fixed); switch (unit) { case TUnit::NONE: { - ss << value; + // TUnit::NONE is used as a special counter, it is just a label + // - PeakMemoryUsage: + // - BuildKeyArena: 0 + // - ProbeKeyArena: 0 + // So do not need output its value return ss.str(); } diff --git a/be/src/util/runtime_profile.h b/be/src/util/runtime_profile.h index 6e18f08ac9..96d14d6540 100644 --- a/be/src/util/runtime_profile.h +++ b/be/src/util/runtime_profile.h @@ -51,6 +51,7 @@ class TRuntimeProfileTree; #define CONCAT_IMPL(x, y) x##y #define MACRO_CONCAT(x, y) CONCAT_IMPL(x, y) +#define ADD_LABEL_COUNTER(profile, name) (profile)->add_counter(name, TUnit::NONE) #define ADD_COUNTER(profile, name, type) (profile)->add_counter(name, type) #define ADD_TIMER(profile, name) (profile)->add_counter(name, TUnit::TIME_NS) #define ADD_CHILD_COUNTER(profile, name, type, parent) (profile)->add_counter(name, type, parent) diff --git a/be/src/vec/exec/join/vhash_join_node.cpp b/be/src/vec/exec/join/vhash_join_node.cpp index e47cda7d1d..1628af157e 100644 --- a/be/src/vec/exec/join/vhash_join_node.cpp +++ b/be/src/vec/exec/join/vhash_join_node.cpp @@ -437,21 +437,16 @@ Status HashJoinNode::prepare(RuntimeState* state) { } } - //some profile record of build phase are useless when it's shared hash table so add in faker profile - RuntimeProfile* memory_usage = nullptr; - if (_should_build_hash_table) { - memory_usage = runtime_profile()->create_child("PeakMemoryUsage", true, true); - runtime_profile()->add_child(memory_usage, false, nullptr); - } else { - memory_usage = faker_runtime_profile(); - } + _memory_usage_counter = ADD_LABEL_COUNTER(runtime_profile(), "MemoryUsage"); - _build_blocks_memory_usage = ADD_COUNTER(memory_usage, "BuildBlocks", TUnit::BYTES); - _hash_table_memory_usage = ADD_COUNTER(memory_usage, "HashTable", TUnit::BYTES); - _build_arena_memory_usage = - memory_usage->AddHighWaterMarkCounter("BuildKeyArena", TUnit::BYTES); - _probe_arena_memory_usage = - memory_usage->AddHighWaterMarkCounter("ProbeKeyArena", TUnit::BYTES); + _build_blocks_memory_usage = + ADD_CHILD_COUNTER(runtime_profile(), "BuildBlocks", TUnit::BYTES, "MemoryUsage"); + _hash_table_memory_usage = + ADD_CHILD_COUNTER(runtime_profile(), "HashTable", TUnit::BYTES, "MemoryUsage"); + _build_arena_memory_usage = runtime_profile()->AddHighWaterMarkCounter( + "BuildKeyArena", TUnit::BYTES, "MemoryUsage"); + _probe_arena_memory_usage = runtime_profile()->AddHighWaterMarkCounter( + "ProbeKeyArena", TUnit::BYTES, "MemoryUsage"); // Build phase _build_phase_profile = runtime_profile()->create_child("BuildPhase", true, true); diff --git a/be/src/vec/exec/join/vhash_join_node.h b/be/src/vec/exec/join/vhash_join_node.h index 408dacce02..7430bd6ab1 100644 --- a/be/src/vec/exec/join/vhash_join_node.h +++ b/be/src/vec/exec/join/vhash_join_node.h @@ -293,6 +293,7 @@ private: RuntimeProfile::Counter* _open_timer; RuntimeProfile::Counter* _allocate_resource_timer; + RuntimeProfile::Counter* _memory_usage_counter; RuntimeProfile::Counter* _build_blocks_memory_usage; RuntimeProfile::Counter* _hash_table_memory_usage; RuntimeProfile::HighWaterMarkCounter* _build_arena_memory_usage; diff --git a/be/src/vec/exec/scan/vscan_node.cpp b/be/src/vec/exec/scan/vscan_node.cpp index 692b65376d..086ef30612 100644 --- a/be/src/vec/exec/scan/vscan_node.cpp +++ b/be/src/vec/exec/scan/vscan_node.cpp @@ -271,11 +271,11 @@ Status VScanNode::_init_profile() { _scanner_profile.reset(new RuntimeProfile("VScanner")); runtime_profile()->add_child(_scanner_profile.get(), true, nullptr); - auto* memory_usage = _scanner_profile->create_child("PeakMemoryUsage", true, true); - _runtime_profile->add_child(memory_usage, false, nullptr); + _memory_usage_counter = ADD_LABEL_COUNTER(_scanner_profile, "MemoryUsage"); _queued_blocks_memory_usage = - memory_usage->AddHighWaterMarkCounter("QueuedBlocks", TUnit::BYTES); - _free_blocks_memory_usage = memory_usage->AddHighWaterMarkCounter("FreeBlocks", TUnit::BYTES); + _scanner_profile->AddHighWaterMarkCounter("QueuedBlocks", TUnit::BYTES, "MemoryUsage"); + _free_blocks_memory_usage = + _scanner_profile->AddHighWaterMarkCounter("FreeBlocks", TUnit::BYTES, "MemoryUsage"); _newly_create_free_blocks_num = ADD_COUNTER(_scanner_profile, "NewlyCreateFreeBlocksNum", TUnit::UNIT); // time of transfer thread to wait for block from scan thread diff --git a/be/src/vec/exec/scan/vscan_node.h b/be/src/vec/exec/scan/vscan_node.h index 69692b032d..ac2034e44e 100644 --- a/be/src/vec/exec/scan/vscan_node.h +++ b/be/src/vec/exec/scan/vscan_node.h @@ -365,6 +365,7 @@ protected: // Max num of scanner thread RuntimeProfile::Counter* _max_scanner_thread_num = nullptr; + RuntimeProfile::Counter* _memory_usage_counter; RuntimeProfile::HighWaterMarkCounter* _queued_blocks_memory_usage; RuntimeProfile::HighWaterMarkCounter* _free_blocks_memory_usage; diff --git a/be/src/vec/exec/vaggregation_node.cpp b/be/src/vec/exec/vaggregation_node.cpp index bb2ce8377e..553b227d62 100644 --- a/be/src/vec/exec/vaggregation_node.cpp +++ b/be/src/vec/exec/vaggregation_node.cpp @@ -323,11 +323,11 @@ void AggregationNode::_init_hash_method(const VExprContextSPtrs& probe_exprs) { } Status AggregationNode::prepare_profile(RuntimeState* state) { - auto* memory_usage = runtime_profile()->create_child("PeakMemoryUsage", true, true); - runtime_profile()->add_child(memory_usage, false, nullptr); - _hash_table_memory_usage = ADD_COUNTER(memory_usage, "HashTable", TUnit::BYTES); - _serialize_key_arena_memory_usage = - memory_usage->AddHighWaterMarkCounter("SerializeKeyArena", TUnit::BYTES); + _memory_usage_counter = ADD_LABEL_COUNTER(runtime_profile(), "MemoryUsage"); + _hash_table_memory_usage = + ADD_CHILD_COUNTER(runtime_profile(), "HashTable", TUnit::BYTES, "MemoryUsage"); + _serialize_key_arena_memory_usage = runtime_profile()->AddHighWaterMarkCounter( + "SerializeKeyArena", TUnit::BYTES, "MemoryUsage"); _build_timer = ADD_TIMER(runtime_profile(), "BuildTime"); _build_table_convert_timer = ADD_TIMER(runtime_profile(), "BuildConvertToPartitionedTime"); diff --git a/be/src/vec/exec/vaggregation_node.h b/be/src/vec/exec/vaggregation_node.h index a0f95dd05c..8b3bb0e82d 100644 --- a/be/src/vec/exec/vaggregation_node.h +++ b/be/src/vec/exec/vaggregation_node.h @@ -944,6 +944,7 @@ private: RuntimeProfile::Counter* _hash_table_input_counter; RuntimeProfile::Counter* _max_row_size_counter; + RuntimeProfile::Counter* _memory_usage_counter; RuntimeProfile::Counter* _hash_table_memory_usage; RuntimeProfile::HighWaterMarkCounter* _serialize_key_arena_memory_usage; diff --git a/be/src/vec/exec/vanalytic_eval_node.cpp b/be/src/vec/exec/vanalytic_eval_node.cpp index cef242e688..41783a6267 100644 --- a/be/src/vec/exec/vanalytic_eval_node.cpp +++ b/be/src/vec/exec/vanalytic_eval_node.cpp @@ -167,9 +167,9 @@ Status VAnalyticEvalNode::prepare(RuntimeState* state) { RETURN_IF_ERROR(ExecNode::prepare(state)); DCHECK(child(0)->row_desc().is_prefix_of(_row_descriptor)); - auto* memory_usage = runtime_profile()->create_child("PeakMemoryUsage", true, true); - runtime_profile()->add_child(memory_usage, false, nullptr); - _blocks_memory_usage = memory_usage->AddHighWaterMarkCounter("Blocks", TUnit::BYTES); + _memory_usage_counter = ADD_LABEL_COUNTER(runtime_profile(), "MemoryUsage"); + _blocks_memory_usage = + runtime_profile()->AddHighWaterMarkCounter("Blocks", TUnit::BYTES, "MemoryUsage"); _evaluation_timer = ADD_TIMER(runtime_profile(), "EvaluationTime"); SCOPED_TIMER(_evaluation_timer); diff --git a/be/src/vec/exec/vanalytic_eval_node.h b/be/src/vec/exec/vanalytic_eval_node.h index bd08ef2dca..d232c28ff9 100644 --- a/be/src/vec/exec/vanalytic_eval_node.h +++ b/be/src/vec/exec/vanalytic_eval_node.h @@ -184,6 +184,7 @@ private: std::vector _origin_cols; RuntimeProfile::Counter* _evaluation_timer; + RuntimeProfile::Counter* _memory_usage_counter; RuntimeProfile::HighWaterMarkCounter* _blocks_memory_usage; std::vector _change_to_nullable_flags; diff --git a/be/src/vec/exec/vsort_node.cpp b/be/src/vec/exec/vsort_node.cpp index b7ad692413..a4501e82f3 100644 --- a/be/src/vec/exec/vsort_node.cpp +++ b/be/src/vec/exec/vsort_node.cpp @@ -115,9 +115,9 @@ Status VSortNode::prepare(RuntimeState* state) { RETURN_IF_ERROR(ExecNode::prepare(state)); _runtime_profile->add_info_string("TOP-N", _limit == -1 ? "false" : "true"); - auto* memory_usage = _runtime_profile->create_child("PeakMemoryUsage", true, true); - _runtime_profile->add_child(memory_usage, false, nullptr); - _sort_blocks_memory_usage = ADD_COUNTER(memory_usage, "SortBlocks", TUnit::BYTES); + _memory_usage_counter = ADD_LABEL_COUNTER(runtime_profile(), "MemoryUsage"); + _sort_blocks_memory_usage = + ADD_CHILD_COUNTER(runtime_profile(), "SortBlocks", TUnit::BYTES, "MemoryUsage"); RETURN_IF_ERROR(_vsort_exec_exprs.prepare(state, child(0)->row_desc(), _row_descriptor)); return Status::OK(); diff --git a/be/src/vec/exec/vsort_node.h b/be/src/vec/exec/vsort_node.h index 2196e5e318..7798493102 100644 --- a/be/src/vec/exec/vsort_node.h +++ b/be/src/vec/exec/vsort_node.h @@ -86,6 +86,7 @@ private: std::vector _is_asc_order; std::vector _nulls_first; + RuntimeProfile::Counter* _memory_usage_counter; RuntimeProfile::Counter* _sort_blocks_memory_usage; bool _use_topn_opt = false; diff --git a/be/src/vec/runtime/vdata_stream_recvr.cpp b/be/src/vec/runtime/vdata_stream_recvr.cpp index c65a669f9b..9ea496fd81 100644 --- a/be/src/vec/runtime/vdata_stream_recvr.cpp +++ b/be/src/vec/runtime/vdata_stream_recvr.cpp @@ -320,9 +320,8 @@ VDataStreamRecvr::VDataStreamRecvr( } // Initialize the counters - auto* memory_usage = _profile->create_child("PeakMemoryUsage", true, true); - _profile->add_child(memory_usage, false, nullptr); - _blocks_memory_usage = memory_usage->AddHighWaterMarkCounter("Blocks", TUnit::BYTES); + _memory_usage_counter = ADD_LABEL_COUNTER(_profile, "MemoryUsage"); + _blocks_memory_usage = _profile->AddHighWaterMarkCounter("Blocks", TUnit::BYTES, "MemoryUsage"); _bytes_received_counter = ADD_COUNTER(_profile, "BytesReceived", TUnit::BYTES); _local_bytes_received_counter = ADD_COUNTER(_profile, "LocalBytesReceived", TUnit::BYTES); diff --git a/be/src/vec/runtime/vdata_stream_recvr.h b/be/src/vec/runtime/vdata_stream_recvr.h index c2374d23a7..3e85a649c5 100644 --- a/be/src/vec/runtime/vdata_stream_recvr.h +++ b/be/src/vec/runtime/vdata_stream_recvr.h @@ -152,6 +152,7 @@ private: RuntimeProfile::Counter* _data_arrival_timer; RuntimeProfile::Counter* _decompress_timer; RuntimeProfile::Counter* _decompress_bytes; + RuntimeProfile::Counter* _memory_usage_counter; RuntimeProfile::HighWaterMarkCounter* _blocks_memory_usage; std::shared_ptr _sub_plan_query_statistics_recvr; diff --git a/fe/fe-core/src/main/java/org/apache/doris/common/util/RuntimeProfile.java b/fe/fe-core/src/main/java/org/apache/doris/common/util/RuntimeProfile.java index 075893e498..084b69f135 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/common/util/RuntimeProfile.java +++ b/fe/fe-core/src/main/java/org/apache/doris/common/util/RuntimeProfile.java @@ -322,6 +322,10 @@ public class RuntimeProfile { StringBuilder builder = new StringBuilder(); long tmpValue = value; switch (type) { + case NONE: { + // Do nothing, it is just a label + break; + } case UNIT: { Pair pair = DebugUtil.getUint(tmpValue); if (pair.second.isEmpty()) { diff --git a/gensrc/thrift/Metrics.thrift b/gensrc/thrift/Metrics.thrift index 78f0b6dc8c..ba554e2204 100644 --- a/gensrc/thrift/Metrics.thrift +++ b/gensrc/thrift/Metrics.thrift @@ -30,6 +30,8 @@ enum TUnit { TIME_NS, DOUBLE_VALUE, // No units at all, may not be a numerical quantity + // It is used as a label now, so do not treat it as + // a real counter. NONE, TIME_MS, TIME_S