You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by yi...@apache.org on 2023/06/12 02:00:47 UTC
[doris] branch master updated: [profile](remove child) child is for node, should not be used to organize counters (#20676)
This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new a6f625676b [profile](remove child) child is for node, should not be used to organize counters (#20676)
a6f625676b is described below
commit a6f625676bd2a64c280440c461b5877651731454
Author: yiguolei <67...@qq.com>
AuthorDate: Mon Jun 12 10:00:35 2023 +0800
[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 <yi...@gmail.com>
---
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 | 25 ++++-----
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 +
.../apache/doris/common/util/RuntimeProfile.java | 4 ++
gensrc/thrift/Metrics.thrift | 2 +
17 files changed, 45 insertions(+), 98 deletions(-)
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 <rapidjson/document.h>
-#include <rapidjson/rapidjson.h>
-
-#include <string>
-
-#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 T>
-typename std::enable_if<!std::is_arithmetic<T>::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<std::string>(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 T>
-typename boost::enable_if_c<boost::is_arithmetic<T>::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();
- }
-
- _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);
+ _memory_usage_counter = ADD_LABEL_COUNTER(runtime_profile(), "MemoryUsage");
+
+ _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<int64_t> _origin_cols;
RuntimeProfile::Counter* _evaluation_timer;
+ RuntimeProfile::Counter* _memory_usage_counter;
RuntimeProfile::HighWaterMarkCounter* _blocks_memory_usage;
std::vector<bool> _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<bool> _is_asc_order;
std::vector<bool> _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<QueryStatisticsRecvr> _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<Double, String> 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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org