This is an automated email from the ASF dual-hosted git repository.

yiguolei pushed a commit to branch branch-2.1
in repository https://gitbox.apache.org/repos/asf/doris.git


The following commit(s) were added to refs/heads/branch-2.1 by this push:
     new dec5f0ca988 Revert "[fix](profile) Fix reporting the profile while 
building the p… (#34498)
dec5f0ca988 is described below

commit dec5f0ca988ab27a8d51ed498daf7af7c21cd6a7
Author: Mryange <[email protected]>
AuthorDate: Tue May 7 22:58:50 2024 +0800

    Revert "[fix](profile) Fix reporting the profile while building the p… 
(#34498)
    
    * Revert "[fix](profile) Fix reporting the profile while building the 
pipeline profile. (#34215)"
    
    This reverts commit eb0d963389e1b7d150cbc18c927091648e0a60f7.
    
    * Revert "[feature](profile) sort pipelineX task by total time #34053"
    
    This reverts commit 67b394f2b0dddab3801d2faa82a91c52ef875e76.
---
 .../pipeline_x/pipeline_x_fragment_context.cpp     | 12 ++++++-
 be/src/runtime/runtime_state.cpp                   | 40 ----------------------
 be/src/runtime/runtime_state.h                     |  9 ++---
 be/src/util/runtime_profile.cpp                    | 12 -------
 be/src/util/runtime_profile.h                      |  2 --
 5 files changed, 13 insertions(+), 62 deletions(-)

diff --git a/be/src/pipeline/pipeline_x/pipeline_x_fragment_context.cpp 
b/be/src/pipeline/pipeline_x/pipeline_x_fragment_context.cpp
index 0fcf86978c9..5c92091e3bf 100644
--- a/be/src/pipeline/pipeline_x/pipeline_x_fragment_context.cpp
+++ b/be/src/pipeline/pipeline_x/pipeline_x_fragment_context.cpp
@@ -492,7 +492,17 @@ Status PipelineXFragmentContext::_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->pipeline_id_to_profile();
+    DCHECK(pipeline_id_to_profile.empty());
+    pipeline_id_to_profile.resize(_pipelines.size());
+    {
+        size_t pip_idx = 0;
+        for (auto& pipeline_profile : pipeline_id_to_profile) {
+            pipeline_profile =
+                    std::make_unique<RuntimeProfile>("Pipeline : " + 
std::to_string(pip_idx));
+            pip_idx++;
+        }
+    }
 
     for (size_t i = 0; i < target_size; i++) {
         const auto& local_params = request.local_params[i];
diff --git a/be/src/runtime/runtime_state.cpp b/be/src/runtime/runtime_state.cpp
index b158ad0b43c..75d06adc561 100644
--- a/be/src/runtime/runtime_state.cpp
+++ b/be/src/runtime/runtime_state.cpp
@@ -562,44 +562,4 @@ bool RuntimeState::is_nereids() const {
     return _query_ctx->is_nereids();
 }
 
-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;
-}
-
-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()) {
-        throw Exception(ErrorCode::INTERNAL_ERROR,
-                        "build_pipeline_profile can only be called once.");
-    }
-    _pipeline_id_to_profile.resize(pipeline_size);
-    {
-        size_t pip_idx = 0;
-        for (auto& pipeline_profile : _pipeline_id_to_profile) {
-            pipeline_profile =
-                    std::make_shared<RuntimeProfile>("Pipeline : " + 
std::to_string(pip_idx));
-            pip_idx++;
-        }
-    }
-    return _pipeline_id_to_profile;
-}
-
 } // end namespace doris
diff --git a/be/src/runtime/runtime_state.h b/be/src/runtime/runtime_state.h
index e4bec15bf63..4b5d3641705 100644
--- a/be/src/runtime/runtime_state.h
+++ b/be/src/runtime/runtime_state.h
@@ -30,7 +30,6 @@
 #include <functional>
 #include <memory>
 #include <mutex>
-#include <shared_mutex>
 #include <string>
 #include <utility>
 #include <vector>
@@ -575,9 +574,7 @@ public:
 
     void resize_op_id_to_local_state(int operator_size);
 
-    std::vector<std::shared_ptr<RuntimeProfile>> pipeline_id_to_profile();
-
-    std::vector<std::shared_ptr<RuntimeProfile>>& 
build_pipeline_profile(std::size_t pipeline_size);
+    auto& pipeline_id_to_profile() { return _pipeline_id_to_profile; }
 
     void set_task_execution_context(std::shared_ptr<TaskExecutionContext> 
context) {
         _task_execution_context_inited = true;
@@ -754,9 +751,7 @@ private:
     // true if max_filter_ratio is 0
     bool _load_zero_tolerance = false;
 
-    // only to lock _pipeline_id_to_profile
-    std::shared_mutex _pipeline_profile_lock;
-    std::vector<std::shared_ptr<RuntimeProfile>> _pipeline_id_to_profile;
+    std::vector<std::unique_ptr<RuntimeProfile>> _pipeline_id_to_profile;
 
     // prohibit copies
     RuntimeState(const RuntimeState&);
diff --git a/be/src/util/runtime_profile.cpp b/be/src/util/runtime_profile.cpp
index ca94329bc85..782009d4438 100644
--- a/be/src/util/runtime_profile.cpp
+++ b/be/src/util/runtime_profile.cpp
@@ -25,7 +25,6 @@
 #include <rapidjson/stringbuffer.h>
 #include <rapidjson/writer.h>
 
-#include <algorithm>
 #include <iomanip>
 #include <iostream>
 
@@ -717,15 +716,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..c1756f6c63e 100644
--- a/be/src/util/runtime_profile.h
+++ b/be/src/util/runtime_profile.h
@@ -301,8 +301,6 @@ public:
         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.


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to