github-actions[bot] commented on code in PR #39760:
URL: https://github.com/apache/doris/pull/39760#discussion_r1726210968
##########
be/src/runtime/workload_group/workload_group.h:
##########
@@ -86,10 +91,31 @@ class WorkloadGroup : public
std::enable_shared_from_this<WorkloadGroup> {
return _spill_high_watermark.load(std::memory_order_relaxed);
}
- void set_weighted_memory_used(int64_t wg_total_mem_used, double ratio);
+ void set_weighted_memory_ratio(double ratio);
+ bool add_wg_refresh_interval_memory_growth(int64_t size) {
+ // `weighted_mem_used` is a rough memory usage in this group,
+ // because we can only get a precise memory usage by MemTracker which
is not include page cache.
+ auto weighted_mem_used =
+ int64_t((_total_mem_used +
_wg_refresh_interval_memory_growth.load() + size) *
+ _weighted_mem_ratio);
+ if ((weighted_mem_used > ((double)_memory_limit *
+
_spill_high_watermark.load(std::memory_order_relaxed) / 100))) {
+ return false;
Review Comment:
warning: redundant boolean literal in conditional return statement
[readability-simplify-boolean-expr]
be/src/runtime/workload_group/workload_group.h:100:
```diff
- if ((weighted_mem_used > ((double)_memory_limit *
-
_spill_high_watermark.load(std::memory_order_relaxed) / 100))) {
- return false;
- } else {
- _wg_refresh_interval_memory_growth.fetch_add(size);
- return true;
- }
+ return !(weighted_mem_used > ((double)_memory_limit *
+
_spill_high_watermark.load(std::memory_order_relaxed) / 100));
```
##########
be/src/runtime/workload_group/workload_group_manager.cpp:
##########
@@ -148,50 +148,34 @@ void
WorkloadGroupMgr::delete_workload_group_by_ids(std::set<uint64_t> used_wg_i
struct WorkloadGroupMemInfo {
int64_t total_mem_used = 0;
- int64_t weighted_mem_used = 0;
- bool is_low_wartermark = false;
- bool is_high_wartermark = false;
- double mem_used_ratio = 0;
+ std::list<std::shared_ptr<MemTrackerLimiter>> tracker_snapshots =
+ std::list<std::shared_ptr<MemTrackerLimiter>>();
};
-void WorkloadGroupMgr::refresh_wg_memory_info() {
+
+void WorkloadGroupMgr::refresh_wg_weighted_memory_ratio() {
Review Comment:
warning: function 'refresh_wg_weighted_memory_ratio' exceeds recommended
size/complexity thresholds [readability-function-size]
```cpp
void WorkloadGroupMgr::refresh_wg_weighted_memory_ratio() {
^
```
<details>
<summary>Additional context</summary>
**be/src/runtime/workload_group/workload_group_manager.cpp:154:** 87 lines
including whitespace and comments (threshold 80)
```cpp
void WorkloadGroupMgr::refresh_wg_weighted_memory_ratio() {
^
```
</details>
##########
be/src/runtime/query_context.h:
##########
@@ -269,15 +269,16 @@ class QueryContext {
return _running_big_mem_op_num.load(std::memory_order_relaxed);
}
- void set_weighted_mem(int64_t weighted_limit, int64_t
weighted_consumption) {
+ void set_weighted_memory(int64_t weighted_limit, double weighted_ratio) {
std::lock_guard<std::mutex> l(_weighted_mem_lock);
- _weighted_consumption = weighted_consumption;
_weighted_limit = weighted_limit;
+ _weighted_ratio = weighted_ratio;
}
- void get_weighted_mem_info(int64_t& weighted_limit, int64_t&
weighted_consumption) {
+
+ void get_weighted_memory(int64_t& weighted_limit, int64_t&
weighted_consumption) {
Review Comment:
warning: method 'get_weighted_memory' can be made const
[readability-make-member-function-const]
```suggestion
void get_weighted_memory(int64_t& weighted_limit, int64_t&
weighted_consumption) const {
```
##########
be/test/runtime/memory/thread_mem_tracker_mgr_test.cpp:
##########
@@ -239,7 +250,7 @@ TEST(ThreadMemTrackerMgrTest, ScopedCount) {
EXPECT_EQ(scope_mem, size1 + size2 + size1 + size2 + size1);
}
-TEST(ThreadMemTrackerMgrTest, ReserveMemory) {
+TEST_F(ThreadMemTrackerMgrTest, ReserveMemory) {
Review Comment:
warning: function 'TEST_F' exceeds recommended size/complexity thresholds
[readability-function-size]
```cpp
TEST_F(ThreadMemTrackerMgrTest, ReserveMemory) {
^
```
<details>
<summary>Additional context</summary>
**be/test/runtime/memory/thread_mem_tracker_mgr_test.cpp:252:** 97 lines
including whitespace and comments (threshold 80)
```cpp
TEST_F(ThreadMemTrackerMgrTest, ReserveMemory) {
^
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
