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 68b3556d51e [fix](core) fix error sort in profile #34465
68b3556d51e is described below
commit 68b3556d51e563b997bcbc3db6744e68c781ba51
Author: Mryange <59914473+mrya...@users.noreply.github.com>
AuthorDate: Tue May 7 22:43:13 2024 +0800
[fix](core) fix error sort in profile #34465
---
be/src/pipeline/pipeline_fragment_context.cpp | 6 +++---
be/src/runtime/fragment_mgr.cpp | 2 +-
be/src/runtime/runtime_state.cpp | 20 ++------------------
be/src/runtime/runtime_state.h | 2 +-
be/src/util/runtime_profile.cpp | 11 -----------
be/src/util/runtime_profile.h | 10 ----------
.../doris/common/profile/ExecutionProfile.java | 1 +
7 files changed, 8 insertions(+), 44 deletions(-)
diff --git a/be/src/pipeline/pipeline_fragment_context.cpp
b/be/src/pipeline/pipeline_fragment_context.cpp
index 9946d7fff97..84039f2b263 100644
--- a/be/src/pipeline/pipeline_fragment_context.cpp
+++ b/be/src/pipeline/pipeline_fragment_context.cpp
@@ -348,7 +348,7 @@ Status PipelineFragmentContext::_build_pipeline_tasks(
_total_tasks = 0;
int target_size = request.local_params.size();
_tasks.resize(target_size);
- auto& pipeline_id_to_profile =
_runtime_state->build_pipeline_profile(_pipelines.size());
+ auto pipeline_id_to_profile =
_runtime_state->build_pipeline_profile(_pipelines.size());
for (size_t i = 0; i < target_size; i++) {
const auto& local_params = request.local_params[i];
@@ -1510,7 +1510,7 @@ void PipelineFragmentContext::_close_fragment_instance() {
// After add the operation, the print out like that:
// UNION_NODE (id=0):(Active: 56.720us, non-child: 82.53%)
// We can easily know the exec node execute time without child time
consumed.
- for (const auto& runtime_profile_ptr :
_runtime_state->pipeline_id_to_profile()) {
+ for (auto runtime_profile_ptr :
_runtime_state->pipeline_id_to_profile()) {
runtime_profile_ptr->pretty_print(&ss);
}
@@ -1620,7 +1620,7 @@ PipelineFragmentContext::collect_realtime_profile_x()
const {
}
// pipeline_id_to_profile is initialized in prepare stage
- for (auto& pipeline_profile : _runtime_state->pipeline_id_to_profile()) {
+ for (auto pipeline_profile : _runtime_state->pipeline_id_to_profile()) {
auto profile_ptr = std::make_shared<TRuntimeProfileTree>();
pipeline_profile->to_thrift(profile_ptr.get());
res.push_back(profile_ptr);
diff --git a/be/src/runtime/fragment_mgr.cpp b/be/src/runtime/fragment_mgr.cpp
index e70edc390fa..7a3170b7370 100644
--- a/be/src/runtime/fragment_mgr.cpp
+++ b/be/src/runtime/fragment_mgr.cpp
@@ -266,7 +266,7 @@ void FragmentMgr::coordinator_callback(const
ReportStatusRequest& req) {
}
if (enable_profile) {
- for (auto& pipeline_profile :
req.runtime_state->pipeline_id_to_profile()) {
+ for (auto pipeline_profile :
req.runtime_state->pipeline_id_to_profile()) {
TDetailedReportParams detailed_param;
detailed_param.__isset.fragment_instance_id = false;
detailed_param.__isset.profile = true;
diff --git a/be/src/runtime/runtime_state.cpp b/be/src/runtime/runtime_state.cpp
index 5ccddff5a3f..7c3b24e2234 100644
--- a/be/src/runtime/runtime_state.cpp
+++ b/be/src/runtime/runtime_state.cpp
@@ -557,26 +557,10 @@ bool RuntimeState::is_nereids() const {
std::vector<std::shared_ptr<RuntimeProfile>>
RuntimeState::pipeline_id_to_profile() {
std::shared_lock lc(_pipeline_profile_lock);
- std::vector<std::shared_ptr<RuntimeProfile>> pipelines_profile;
- pipelines_profile.reserve(_pipeline_id_to_profile.size());
- // The sort here won't change the structure of _pipeline_id_to_profile;
- // it sorts the children of each element in sort_pipeline_id_to_profile,
- // and these children are locked.
- for (auto& pipeline_profile : _pipeline_id_to_profile) {
- DCHECK(pipeline_profile);
- // pipeline 0
- // pipeline task 0
- // pipeline task 1
- // pipleine task 2
- // .......
- // sort by pipeline task total time
- pipeline_profile->sort_children_by_total_time();
- pipelines_profile.push_back(pipeline_profile);
- }
- return pipelines_profile;
+ return _pipeline_id_to_profile;
}
-std::vector<std::shared_ptr<RuntimeProfile>>&
RuntimeState::build_pipeline_profile(
+std::vector<std::shared_ptr<RuntimeProfile>>
RuntimeState::build_pipeline_profile(
std::size_t pipeline_size) {
std::unique_lock lc(_pipeline_profile_lock);
if (!_pipeline_id_to_profile.empty()) {
diff --git a/be/src/runtime/runtime_state.h b/be/src/runtime/runtime_state.h
index 1c4ddc862ad..bc78d6e094c 100644
--- a/be/src/runtime/runtime_state.h
+++ b/be/src/runtime/runtime_state.h
@@ -581,7 +581,7 @@ public:
std::vector<std::shared_ptr<RuntimeProfile>> pipeline_id_to_profile();
- std::vector<std::shared_ptr<RuntimeProfile>>&
build_pipeline_profile(std::size_t pipeline_size);
+ std::vector<std::shared_ptr<RuntimeProfile>>
build_pipeline_profile(std::size_t pipeline_size);
void set_task_execution_context(std::shared_ptr<TaskExecutionContext>
context) {
_task_execution_context_inited = true;
diff --git a/be/src/util/runtime_profile.cpp b/be/src/util/runtime_profile.cpp
index ca94329bc85..7c9d40ba3ed 100644
--- a/be/src/util/runtime_profile.cpp
+++ b/be/src/util/runtime_profile.cpp
@@ -717,15 +717,4 @@ void RuntimeProfile::print_child_counters(const
std::string& prefix,
}
}
-void RuntimeProfile::sort_children_by_total_time() {
- std::lock_guard<std::mutex> l(_children_lock);
- auto cmp = [](const std::pair<RuntimeProfile*, bool>& L,
- const std::pair<RuntimeProfile*, bool>& R) {
- const RuntimeProfile* L_profile = L.first;
- const RuntimeProfile* R_profile = R.first;
- return L_profile->_counter_total_time.value() >
R_profile->_counter_total_time.value();
- };
- std::sort(_children.begin(), _children.end(), cmp);
-}
-
} // namespace doris
diff --git a/be/src/util/runtime_profile.h b/be/src/util/runtime_profile.h
index 5360a0e991c..c28329fe5da 100644
--- a/be/src/util/runtime_profile.h
+++ b/be/src/util/runtime_profile.h
@@ -293,16 +293,6 @@ public:
/// otherwise appended after other child profiles.
RuntimeProfile* create_child(const std::string& name, bool indent = true,
bool prepend = false);
- // Sorts all children according to a custom comparator. Does not
- // invalidate pointers to profiles.
- template <class Compare>
- void sort_childer(const Compare& cmp) {
- std::lock_guard<std::mutex> l(_children_lock);
- std::sort(_children.begin(), _children.end(), cmp);
- }
-
- void sort_children_by_total_time();
-
// Merges the src profile into this one, combining counters that have an
identical
// path. Info strings from profiles are not merged. 'src' would be a const
if it
// weren't for locking.
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/common/profile/ExecutionProfile.java
b/fe/fe-core/src/main/java/org/apache/doris/common/profile/ExecutionProfile.java
index 46a6367104a..dc91210eeb7 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/common/profile/ExecutionProfile.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/common/profile/ExecutionProfile.java
@@ -352,6 +352,7 @@ public class ExecutionProfile {
profile.setIsDone(true);
}
pipelineIdx++;
+ profile.sortChildren();
fragmentProfiles.get(params.fragment_id).addChild(profile);
}
// TODO ygl: is this right? there maybe multi Backends, what does
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
