From 82ca62dfcc16d40db63cdca927396ece0d44026d Mon Sep 17 00:00:00 2001
From: Xinyi Zou <zouxinyi02@gmail.com>
Date: Fri, 26 Aug 2022 11:43:19 +0800
Subject: [PATCH] [fix](memory) Fix disable_mem_pools to disable cache #12087

---
 be/src/common/config.h                        |  2 ++
 be/src/runtime/bufferpool/buffer_allocator.cc |  2 +-
 be/src/runtime/mem_pool.cpp                   |  1 +
 be/src/runtime/memory/chunk_allocator.cpp     | 12 +++++++++++-
 4 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/be/src/common/config.h b/be/src/common/config.h
index 74cdb38ab6..54f4629b24 100644
--- a/be/src/common/config.h
+++ b/be/src/common/config.h
@@ -431,6 +431,8 @@ CONF_Int32(min_buffer_size, "1024"); // 1024, The minimum read buffer size (in b
 // With 1024B through 8MB buffers, this is up to ~2GB of buffers.
 CONF_Int32(max_free_io_buffers, "128");
 
+// Whether to disable the memory cache pool,
+// including MemPool, ChunkAllocator, BufferPool, DiskIO free buffer.
 CONF_Bool(disable_mem_pools, "false");
 
 // Whether to allocate chunk using mmap. If you enable this, you'd better to
diff --git a/be/src/runtime/bufferpool/buffer_allocator.cc b/be/src/runtime/bufferpool/buffer_allocator.cc
index 0c59dd0d9f..b9da7b45a5 100644
--- a/be/src/runtime/bufferpool/buffer_allocator.cc
+++ b/be/src/runtime/bufferpool/buffer_allocator.cc
@@ -239,7 +239,7 @@ Status BufferPool::BufferAllocator::AllocateInternal(int64_t len, BufferHandle*
     }
     if (UNLIKELY(len > system_bytes_limit_)) {
         err_stream << "Tried to allocate buffer of " << len << " bytes"
-                   << " > buffer pool limit of  " << MAX_BUFFER_BYTES << " bytes";
+                   << " > buffer pool limit of  " << system_bytes_limit_ << " bytes";
         return Status::InternalError(err_stream.str());
     }
 
diff --git a/be/src/runtime/mem_pool.cpp b/be/src/runtime/mem_pool.cpp
index a2c32f020e..6fae53bba4 100644
--- a/be/src/runtime/mem_pool.cpp
+++ b/be/src/runtime/mem_pool.cpp
@@ -132,6 +132,7 @@ Status MemPool::find_chunk(size_t min_size, bool check_limits) {
     if (config::disable_mem_pools) {
         // Disable pooling by sizing the chunk to fit only this allocation.
         // Make sure the alignment guarantees are respected.
+        // This will generate too many small chunks.
         chunk_size = std::max<size_t>(min_size, alignof(max_align_t));
     } else {
         DCHECK_GE(next_chunk_size_, INITIAL_CHUNK_SIZE);
diff --git a/be/src/runtime/memory/chunk_allocator.cpp b/be/src/runtime/memory/chunk_allocator.cpp
index f62258b216..43acc79538 100644
--- a/be/src/runtime/memory/chunk_allocator.cpp
+++ b/be/src/runtime/memory/chunk_allocator.cpp
@@ -34,6 +34,11 @@
 
 namespace doris {
 
+// <= MIN_CHUNK_SIZE, A large number of small chunks will waste extra storage and increase lock time.
+static constexpr size_t MIN_CHUNK_SIZE = 4096; // 4K
+// >= MAX_CHUNK_SIZE, Large chunks may not be used for a long time, wasting memory.
+static constexpr size_t MAX_CHUNK_SIZE = 64 * (1ULL << 20); // 64M
+
 ChunkAllocator* ChunkAllocator::_s_instance = nullptr;
 
 DEFINE_COUNTER_METRIC_PROTOTYPE_2ARG(chunk_pool_local_core_alloc_count, MetricUnit::NOUNIT);
@@ -199,12 +204,17 @@ Status ChunkAllocator::allocate(size_t size, Chunk* chunk) {
 void ChunkAllocator::free(const Chunk& chunk) {
     DCHECK(chunk.core_id != -1);
     CHECK((chunk.size & (chunk.size - 1)) == 0);
+    if (config::disable_mem_pools) {
+        SystemAllocator::free(chunk.data, chunk.size);
+        return;
+    }
 
     int64_t old_reserved_bytes = _reserved_bytes;
     int64_t new_reserved_bytes = 0;
     do {
         new_reserved_bytes = old_reserved_bytes + chunk.size;
-        if (new_reserved_bytes > _reserve_bytes_limit) {
+        if (chunk.size <= MIN_CHUNK_SIZE || chunk.size >= MAX_CHUNK_SIZE ||
+            new_reserved_bytes > _reserve_bytes_limit) {
             int64_t cost_ns = 0;
             {
                 SCOPED_RAW_TIMER(&cost_ns);