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

Reply via email to