From 072a4db4c6572c95a745b392a9eccff559b67b5b Mon Sep 17 00:00:00 2001 From: Xinyi Zou Date: Fri, 30 Aug 2024 17:28:24 +0800 Subject: [PATCH] 7 --- be/src/olap/page_cache.cpp | 9 +-------- be/src/runtime/thread_context.h | 21 ++++++++++++++------- be/src/vec/common/allocator.cpp | 28 ++++++++-------------------- be/src/vec/common/allocator.h | 7 +++++++ 4 files changed, 30 insertions(+), 35 deletions(-) diff --git a/be/src/olap/page_cache.cpp b/be/src/olap/page_cache.cpp index 64e7c9c5b24ca3..68c130bb93fa77 100644 --- a/be/src/olap/page_cache.cpp +++ b/be/src/olap/page_cache.cpp @@ -30,20 +30,13 @@ PageBase::PageBase(size_t b, bool use_cache, segment_v2::PageTypePB LRUCacheValueBase(), _size(b), _capacity(b) { - if (use_cache) { - SCOPED_SWITCH_THREAD_MEM_TRACKER_LIMITER( - StoragePageCache::instance()->mem_tracker(page_type)); - _data = reinterpret_cast(TAllocator::alloc(_capacity, ALLOCATOR_ALIGNMENT_16)); - } else { - _data = reinterpret_cast(TAllocator::alloc(_capacity, ALLOCATOR_ALIGNMENT_16)); - } + _data = reinterpret_cast(TAllocator::alloc(_capacity, ALLOCATOR_ALIGNMENT_16)); } template PageBase::~PageBase() { if (_data != nullptr) { DCHECK(_capacity != 0 && _size != 0); - SCOPED_SWITCH_THREAD_MEM_TRACKER_LIMITER(TAllocator::mem_tracker_); TAllocator::free(_data, _capacity); } } diff --git a/be/src/runtime/thread_context.h b/be/src/runtime/thread_context.h index 6158f0535bed01..fa0b283d1e45c5 100644 --- a/be/src/runtime/thread_context.h +++ b/be/src/runtime/thread_context.h @@ -452,8 +452,10 @@ class SwitchThreadMemTrackerLimiter { const std::shared_ptr& mem_tracker) { DCHECK(mem_tracker); doris::ThreadLocalHandle::create_thread_local_if_not_exits(); - _old_mem_tracker = thread_context()->thread_mem_tracker_mgr->limiter_mem_tracker(); - thread_context()->thread_mem_tracker_mgr->attach_limiter_tracker(mem_tracker); + if (mem_tracker != thread_context()->thread_mem_tracker_mgr->limiter_mem_tracker()) { + _old_mem_tracker = thread_context()->thread_mem_tracker_mgr->limiter_mem_tracker(); + thread_context()->thread_mem_tracker_mgr->attach_limiter_tracker(mem_tracker); + } } explicit SwitchThreadMemTrackerLimiter(const doris::QueryThreadContext& query_thread_context) { @@ -461,18 +463,23 @@ class SwitchThreadMemTrackerLimiter { DCHECK(thread_context()->task_id() == query_thread_context.query_id); // workload group alse not change DCHECK(query_thread_context.query_mem_tracker); - _old_mem_tracker = thread_context()->thread_mem_tracker_mgr->limiter_mem_tracker(); - thread_context()->thread_mem_tracker_mgr->attach_limiter_tracker( - query_thread_context.query_mem_tracker); + if (query_thread_context.query_mem_tracker != + thread_context()->thread_mem_tracker_mgr->limiter_mem_tracker()) { + _old_mem_tracker = thread_context()->thread_mem_tracker_mgr->limiter_mem_tracker(); + thread_context()->thread_mem_tracker_mgr->attach_limiter_tracker( + query_thread_context.query_mem_tracker); + } } ~SwitchThreadMemTrackerLimiter() { - thread_context()->thread_mem_tracker_mgr->detach_limiter_tracker(_old_mem_tracker); + if (_old_mem_tracker != nullptr) { + thread_context()->thread_mem_tracker_mgr->detach_limiter_tracker(_old_mem_tracker); + } doris::ThreadLocalHandle::del_thread_local_if_count_is_zero(); } private: - std::shared_ptr _old_mem_tracker; + std::shared_ptr _old_mem_tracker {nullptr}; }; class AddThreadMemTrackerConsumer { diff --git a/be/src/vec/common/allocator.cpp b/be/src/vec/common/allocator.cpp index cd8afe6c69fdd7..a92c3eae278ccb 100644 --- a/be/src/vec/common/allocator.cpp +++ b/be/src/vec/common/allocator.cpp @@ -48,10 +48,6 @@ Allocator::Allocator( const std::shared_ptr& tracker) { if (tracker) { mem_tracker_ = tracker; - } else { - // Note that when constructing an object that inherits Allocator, the memory tracker may not be attached - // in tls. memory tracker may be attached in tls only when the memory is allocated for the first time. - mem_tracker_ = doris::thread_context()->thread_mem_tracker_mgr->limiter_mem_tracker(); } } @@ -224,29 +220,21 @@ void Allocator::memory_ template void Allocator::consume_memory( size_t size) const { - CONSUME_THREAD_MEM_TRACKER(size); + if (mem_tracker_) { + SCOPED_SWITCH_THREAD_MEM_TRACKER_LIMITER(mem_tracker_); + CONSUME_THREAD_MEM_TRACKER(size); + } else { + CONSUME_THREAD_MEM_TRACKER(size); + } } template void Allocator::release_memory( size_t size) const { - doris::ThreadContext* thread_context = doris::thread_context(true); - if ((thread_context && thread_context->thread_mem_tracker()->label() != "Orphan") || - mem_tracker_ == nullptr) { - // If thread_context exist and the label of thread_mem_tracker not equal to `Orphan`, - // this means that in the scope of SCOPED_ATTACH_TASK, - // so thread_mem_tracker should be used to release memory. - // If mem_tracker_ is nullptr there is a scenario where an object that inherits Allocator - // has never called alloc, but free memory. - // in phmap, the memory alloced by an object may be transferred to another object and then free. - // in this case, thread context must attach a memory tracker other than Orphan, - // otherwise memory tracking will be wrong. + if (mem_tracker_) { + SCOPED_SWITCH_THREAD_MEM_TRACKER_LIMITER(mem_tracker_); RELEASE_THREAD_MEM_TRACKER(size); } else { - // if thread_context does not exist or the label of thread_mem_tracker is equal to - // `Orphan`, it usually happens during object destruction. This means that - // the scope of SCOPED_ATTACH_TASK has been left, so release memory using Allocator tracker. - SCOPED_SWITCH_THREAD_MEM_TRACKER_LIMITER(mem_tracker_); RELEASE_THREAD_MEM_TRACKER(size); } } diff --git a/be/src/vec/common/allocator.h b/be/src/vec/common/allocator.h index c6c4d89c121004..df6612522b8b75 100644 --- a/be/src/vec/common/allocator.h +++ b/be/src/vec/common/allocator.h @@ -234,6 +234,13 @@ class Allocator { // alloc will continue to execute, so the consume memtracker is forced. void memory_check(size_t size) const; // Increases consumption of this tracker by 'bytes'. + // some special cases: + // 1. objects that inherit Allocator will not be shared by multiple queries. + // non-compliant: page cache, ORC ByteBuffer. + // 2. objects that inherit Allocator will only free memory allocated by themselves. + // non-compliant: phmap, the memory alloced by an object may be transferred to another object and then free. + // 3. the memory tracker in TLS is the same during the construction of objects that inherit Allocator + // and during subsequent memory allocation. void consume_memory(size_t size) const; void release_memory(size_t size) const; void throw_bad_alloc(const std::string& err) const;