github-actions[bot] commented on code in PR #38168: URL: https://github.com/apache/doris/pull/38168#discussion_r1686238120
########## 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> ########## be/src/runtime/workload_group/workload_group.h: ########## @@ -85,10 +90,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:99: ```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)); ``` -- 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