Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[opt](memory) Support Memory Profile #41310

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[opt](memory) Support Memory Profile #41310

wants to merge 1 commit into from
+806 −482

Conversation

xinyiZzz
Copy link
Contributor

@xinyiZzz xinyiZzz commented Sep 25, 2024 •
edited
Loading

  1. Support Memory Tracker to create Memory Profile snapshots.
  2. Remove BE mem_tracker web page and replace it with more intuitive Memory Profile.
  3. All memory logs also print more intuitive Memory Profile.
    image
    image
    image
    image

@doris-robot
Copy link

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR

Since 2024-03-18, the Document has been moved to doris-website.
See Doris Document.

@xinyiZzz xinyiZzz changed the title [opt](memory) Add Memory Profile [opt](memory) Support Memory Profile Sep 25, 2024
@xinyiZzz
Copy link
Contributor Author

run buildall

2 similar comments
@xinyiZzz
Copy link
Contributor Author

run buildall

@xinyiZzz
Copy link
Contributor Author

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 37.31% (9635/25826)
Line Coverage: 28.71% (79723/277674)
Region Coverage: 28.13% (41218/146508)
Branch Coverage: 24.77% (20988/84730)
Coverage Report: http://coverage.selectdb-in.cc/coverage/1dfee884a61a60c687910e0d9222739edcec7684_1dfee884a61a60c687910e0d9222739edcec7684/report/index.html

@xinyiZzz
Copy link
Contributor Author

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 37.31% (9636/25826)
Line Coverage: 28.72% (79752/277714)
Region Coverage: 28.13% (41219/146513)
Branch Coverage: 24.77% (20992/84744)
Coverage Report: http://coverage.selectdb-in.cc/coverage/80dc0c2dd3615e473cc4ae01baecb5bb01419fa1_80dc0c2dd3615e473cc4ae01baecb5bb01419fa1/report/index.html

@xinyiZzz
Copy link
Contributor Author

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 37.30% (9626/25807)
Line Coverage: 28.71% (79680/277567)
Region Coverage: 28.13% (41182/146378)
Branch Coverage: 24.77% (20976/84678)
Coverage Report: http://coverage.selectdb-in.cc/coverage/e3d8df714118d7974f18bda91e037886016b95a5_e3d8df714118d7974f18bda91e037886016b95a5/report/index.html

@xinyiZzz
Copy link
Contributor Author

run buildall

1 similar comment
@xinyiZzz
Copy link
Contributor Author

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 37.29% (9628/25816)
Line Coverage: 28.70% (79700/277685)
Region Coverage: 28.13% (41193/146456)
Branch Coverage: 24.75% (20976/84738)
Coverage Report: http://coverage.selectdb-in.cc/coverage/8d3b4954d258e0e02f30a5af97759c5008ddcdd4_8d3b4954d258e0e02f30a5af97759c5008ddcdd4/report/index.html

@xinyiZzz
Copy link
Contributor Author

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 37.29% (9626/25816)
Line Coverage: 28.68% (79643/277689)
Region Coverage: 28.12% (41187/146459)
Branch Coverage: 24.75% (20969/84740)
Coverage Report: http://coverage.selectdb-in.cc/coverage/cb1aabf874c02a410ee27b007488e67f624257a4_cb1aabf874c02a410ee27b007488e67f624257a4/report/index.html

@xinyiZzz
Copy link
Contributor Author

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 37.29% (9628/25816)
Line Coverage: 28.69% (79688/277730)
Region Coverage: 28.14% (41222/146491)
Branch Coverage: 24.76% (20987/84768)
Coverage Report: http://coverage.selectdb-in.cc/coverage/1b8a421300211c558d42e0b5fdca0ef5fa646ec3_1b8a421300211c558d42e0b5fdca0ef5fa646ec3/report/index.html

@xinyiZzz
Copy link
Contributor Author

run buildall

3 similar comments
@xinyiZzz
Copy link
Contributor Author

run buildall

@xinyiZzz
Copy link
Contributor Author

run buildall

@xinyiZzz
Copy link
Contributor Author

run buildall

yiguolei
yiguolei reviewed Oct 8, 2024
View reviewed changes
be/src/runtime/memory/global_memory_arbitrator.h
@@ -192,7 +200,7 @@ class GlobalMemoryArbitrator {
}

private:
static std::atomic<int64_t> _s_process_reserved_memory;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不要去s了

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个类里面其他 static 都没有 s，这是当初笔误了

只有 MemInfo 里的 static 加了 s，为了和历史保持一致，其他地方都不要s了吧

yiguolei
yiguolei reviewed Oct 8, 2024
View reviewed changes
be/src/util/runtime_profile.h
@@ -51,8 +51,8 @@ class TRuntimeProfileTree;
#define MACRO_CONCAT(x, y) CONCAT_IMPL(x, y)

#define ADD_LABEL_COUNTER(profile, name) (profile)->add_counter(name, TUnit::NONE)
#define ADD_LABEL_COUNTER_WITH_LEVEL(profile, name, type) \
(profile)->add_counter_with_level(name, TUnit::NONE, type)
#define ADD_LABEL_COUNTER_WITH_LEVEL(profile, name, level) \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个好像没用到

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个是之前写错了，最后一个参数名 type 应该是 level

@xinyiZzz
Copy link
Contributor Author

run buildall

github-actions[bot]
github-actions bot reviewed Oct 10, 2024
View reviewed changes
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

be/src/runtime/memory/memory_profile.cpp
static bvar::Adder<int64_t> memory_all_tasks_memory_bytes("memory_all_tasks_memory_bytes");
static bvar::Adder<int64_t> memory_untracked_memory_bytes("memory_untracked_memory_bytes");

MemoryProfile::MemoryProfile() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: use '= default' to define a trivial default constructor [modernize-use-equals-default]

MemoryProfile::MemoryProfile() {
               ^

be/src/runtime/memory/memory_profile.cpp
_tasks_memory_profile.set(std::make_unique<RuntimeProfile>("TasksMemorySnapshot"));
}

void MemoryProfile::refresh_memory_overview_profile() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: function 'refresh_memory_overview_profile' exceeds recommended size/complexity thresholds [readability-function-size]

void MemoryProfile::refresh_memory_overview_profile() {
                    ^
Additional context

be/src/runtime/memory/memory_profile.cpp:46: 243 lines including whitespace and comments (threshold 80)

void MemoryProfile::refresh_memory_overview_profile() {
                    ^

be/src/runtime/process_profile.cpp

namespace doris {

ProcessProfile::ProcessProfile() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: use '= default' to define a trivial default constructor [modernize-use-equals-default]

ProcessProfile::ProcessProfile() {
                ^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yiguolei yiguolei yiguolei left review comments

@github-actions GitHub Actions GitHub Actions left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@xinyiZzz @doris-robot @yiguolei