You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/11/17 11:48:26 UTC

[GitHub] [doris] xinyiZzz commented on a diff in pull request #14374: [improvement](tcmalloc) add moderate mode and avoid oom with a lot of…

xinyiZzz commented on code in PR #14374:
URL: https://github.com/apache/doris/pull/14374#discussion_r1025088724


##########
be/src/common/daemon.cpp:
##########
@@ -74,28 +74,49 @@ void Daemon::tcmalloc_gc_thread() {
 
     size_t tc_use_memory_min = MemInfo::mem_limit();
     if (config::memory_mode == std::string("performance")) {
+        // a higher tc_use_memory_min to use memory as tcmalloc cache as much as possible
         tc_use_memory_min = std::max(tc_use_memory_min / 10 * 9,
                                      tc_use_memory_min - (size_t)10 * 1024 * 1024 * 1024);
+    } else if (config::memory_mode == std::string("compact")) {
+        // a limited tc_use_memory_min to limit memory used as cache of tcmalloc
+        tc_use_memory_min = std::min((size_t)10 * 1024 * 1024 * 1024, tc_use_memory_min >> 1);
     } else {
+        // a moderate tc_use_memory_min
         tc_use_memory_min >>= 1;
     }
 
-    while (!_stop_background_threads_latch.wait_for(std::chrono::seconds(10))) {
+    int32_t interval_seconds = 2;
+    while (!_stop_background_threads_latch.wait_for(std::chrono::seconds(interval_seconds))) {
         size_t used_size = 0;
-        size_t free_size = 0;
+        size_t alloc_size = 0;
 
+        MallocExtension::instance()->GetNumericProperty("generic.total_physical_bytes",
+                                                        &alloc_size);
         MallocExtension::instance()->GetNumericProperty("generic.current_allocated_bytes",
                                                         &used_size);
-        MallocExtension::instance()->GetNumericProperty("tcmalloc.pageheap_free_bytes", &free_size);
-        size_t alloc_size = used_size + free_size;
-        LOG(INFO) << "tcmalloc.pageheap_free_bytes " << free_size
-                  << ", generic.current_allocated_bytes " << used_size << ", tc_use_memory_min "
+
+        LOG(INFO) << "generic.current_allocated_bytes " << used_size
+                  << ", generic.total_physical_bytes " << alloc_size << ", tc_use_memory_min "
                   << tc_use_memory_min;
 
         if (alloc_size > tc_use_memory_min) {
-            size_t max_free_size = alloc_size * 20 / 100;
-            if (free_size > max_free_size) {
-                MallocExtension::instance()->ReleaseToSystem(free_size - max_free_size);
+            // Limit size of cache of tcmalloc to avoid oom.
+            // alloc_size > mem_limit: release memory aggressively because we are reaching oom.
+            // alloc_size < mem_limit: limit cache size of tcmalloc under  used_size * 20%.
+            size_t max_free_size = 0;
+
+            if (MemInfo::mem_limit() > alloc_size) {
+                max_free_size = MemInfo::mem_limit() - alloc_size;
+                interval_seconds = 2;
+            } else {
+                interval_seconds = 1;
+            }
+
+            max_free_size = std::min(used_size * 20 / 100, max_free_size);
+            size_t free_size = alloc_size - used_size - max_free_size;

Review Comment:
   It seems difficult to understand using `used_size` to calculate `max_free_size`, and then to calculate `free_size`.
   Directly free the percentage of `MemInfo::allocator_cache_mem()`. Seems to be better understood.



-- 
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-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org