xinyiZzz commented on code in PR #48924:
URL: https://github.com/apache/doris/pull/48924#discussion_r1992965041
##########
be/src/olap/page_cache.h:
##########
@@ -37,33 +37,54 @@ namespace doris {
class PageCacheHandle;
-template <typename TAllocator>
-class PageBase : private TAllocator, public LRUCacheValueBase {
+template <typename T>
+class MemoryTrackedPageBase : public LRUCacheValueBase {
Review Comment:
MemoryTrackedPageBase 没有被多态使用，可以不要，MemoryTrackedPageWithPageEntity 和
MemoryTrackedPageWithPagePtr 分别都继承自 LRUCacheValueBase 就行
其中的属性：
1. _mem_tracker_by_allocator 可以复用 LRUCacheValueBase 中的 _mem_tracker，而且只有
MemoryTrackedPageWithPageEntity 会用到
2. _size 只有 MemoryTrackedPageWithPageEntity 用了
3. 只有一个 _data 是共用的
MemoryTrackedPageWithPageEntity 和 MemoryTrackedPageWithPagePtr 的名字也可以考虑改下
##########
be/src/olap/page_cache.cpp:
##########
@@ -97,7 +117,46 @@ void StoragePageCache::insert(const CacheKey& key,
DataPage* data, PageCacheHand
auto* cache = _get_page_cache(page_type);
auto* lru_handle = cache->insert(key.encode(), data, data->capacity(), 0,
priority);
+ DCHECK(lru_handle != nullptr);
+ *handle = PageCacheHandle(cache, lru_handle);
+}
+
+template <typename T>
+void StoragePageCache::insert(const CacheKey& key, T data, size_t size,
PageCacheHandle* handle,
+ segment_v2::PageTypePB page_type, bool
in_memory) {
+ static_assert(std::is_same<typename std::remove_cv<T>::type,
+ std::shared_ptr<typename
T::element_type>>::value,
+ "Second argument must be a std::shared_ptr");
+ using ValueType = typename T::element_type; // Type that shared_ptr points
to
+
+ CachePriority priority = CachePriority::NORMAL;
+ if (in_memory) {
+ priority = CachePriority::DURABLE;
+ }
+
+ auto* cache = _get_page_cache(page_type);
+ // Lify cycle of page will be managed by StoragePageCache
+ auto page =
std::make_unique<MemoryTrackedPageWithPagePtr<ValueType>>(size, page_type);
+ // Lify cycle of data will be managed by StoragePageCache and user at the
same time.
+ page->set_data(data);
+
+ auto* lru_handle = cache->insert(key.encode(), page.get(), size, 0,
priority);
Review Comment:
把 139 行构造 MemoryTrackedPageWithPagePtr 传的size，传到这个 insert 的第四个参数
##########
be/src/olap/page_cache.h:
##########
@@ -37,33 +37,54 @@ namespace doris {
class PageCacheHandle;
-template <typename TAllocator>
-class PageBase : private TAllocator, public LRUCacheValueBase {
+template <typename T>
+class MemoryTrackedPageBase : public LRUCacheValueBase {
public:
- PageBase() = default;
- PageBase(size_t b, bool use_cache, segment_v2::PageTypePB page_type);
- PageBase(const PageBase&) = delete;
- PageBase& operator=(const PageBase&) = delete;
- ~PageBase() override;
+ MemoryTrackedPageBase() = default;
+ MemoryTrackedPageBase(size_t b, bool use_cache, segment_v2::PageTypePB
page_type);
- char* data() { return _data; }
+ MemoryTrackedPageBase(const MemoryTrackedPageBase&) = delete;
+ MemoryTrackedPageBase& operator=(const MemoryTrackedPageBase&) = delete;
+ ~MemoryTrackedPageBase() = default;
+
+ T data() { return _data; }
size_t size() { return _size; }
- size_t capacity() { return _capacity; }
+
+protected:
+ T _data;
+ size_t _size = 0;
+ std::shared_ptr<MemTrackerLimiter> _mem_tracker_by_allocator;
Review Comment:
LRUCacheValueBase 里有一个 ` std::shared_ptr<MemTrackerLimiter> _mem_tracker`
可以复用
之前 PageBase 忘记复用了，可以顺手改了
##########
be/src/olap/page_cache.cpp:
##########
@@ -17,38 +17,58 @@
#include "olap/page_cache.h"
+#include <gen_cpp/segment_v2.pb.h>
#include <glog/logging.h>
#include <ostream>
#include "runtime/exec_env.h"
namespace doris {
-template <typename TAllocator>
-PageBase<TAllocator>::PageBase(size_t b, bool use_cache,
segment_v2::PageTypePB page_type)
- : LRUCacheValueBase(), _size(b), _capacity(b) {
+
+template <typename T>
+MemoryTrackedPageBase<T>::MemoryTrackedPageBase(size_t size, bool use_cache,
+ segment_v2::PageTypePB
page_type)
+ : _size(size) {
if (use_cache) {
_mem_tracker_by_allocator =
StoragePageCache::instance()->mem_tracker(page_type);
} else {
_mem_tracker_by_allocator =
thread_context()->thread_mem_tracker_mgr->limiter_mem_tracker();
}
+}
+
+MemoryTrackedPageWithPageEntity::MemoryTrackedPageWithPageEntity(size_t size,
bool use_cache,
+
segment_v2::PageTypePB page_type)
+ : MemoryTrackedPageBase<char*>(size, use_cache, page_type),
_capacity(size) {
{
- SCOPED_SWITCH_THREAD_MEM_TRACKER_LIMITER(_mem_tracker_by_allocator);
- _data = reinterpret_cast<char*>(TAllocator::alloc(_capacity,
ALLOCATOR_ALIGNMENT_16));
+
SCOPED_SWITCH_THREAD_MEM_TRACKER_LIMITER(this->_mem_tracker_by_allocator);
+ this->_data = reinterpret_cast<char*>(
+ Allocator<false>::alloc(this->_capacity,
ALLOCATOR_ALIGNMENT_16));
}
}
-template <typename TAllocator>
-PageBase<TAllocator>::~PageBase() {
- if (_data != nullptr) {
- DCHECK(_capacity != 0 && _size != 0);
- SCOPED_SWITCH_THREAD_MEM_TRACKER_LIMITER(_mem_tracker_by_allocator);
- TAllocator::free(_data, _capacity);
+MemoryTrackedPageWithPageEntity::~MemoryTrackedPageWithPageEntity() {
+ if (this->_data != nullptr) {
+ DCHECK(this->_capacity != 0 && this->_size != 0);
+
SCOPED_SWITCH_THREAD_MEM_TRACKER_LIMITER(this->_mem_tracker_by_allocator);
+ Allocator<false>::free(this->_data, this->_capacity);
}
}
-template class PageBase<Allocator<true>>;
-template class PageBase<Allocator<false>>;
+template <typename T>
+MemoryTrackedPageWithPagePtr<T>::MemoryTrackedPageWithPagePtr(size_t size,
+
segment_v2::PageTypePB page_type)
+ : MemoryTrackedPageBase<std::shared_ptr<T>>(size, true, page_type) {
+ DCHECK(this->_size > 0);
+ this->_size = size;
+ this->_mem_tracker_by_allocator->consume(this->_size);
Review Comment:
LRUCachePolicy::insert 支持手动统计内存，而且分别统计了 key+value 和 value 两部分，在 insert
时传入就行了，不用在外边统计
##########
be/src/olap/rowset/segment_v2/segment.cpp:
##########
@@ -462,6 +463,7 @@ Status Segment::_parse_footer(SegmentFooterPB* footer) {
}
// deserialize footer PB
+ footer = std::make_shared<SegmentFooterPB>();
Review Comment:
参数传进来的 footer 已经初始化了，这里为啥再初始化一次
##########
be/src/olap/rowset/segment_v2/segment.cpp:
##########
@@ -175,8 +175,7 @@ io::UInt128Wrapper Segment::file_cache_key(std::string_view
rowset_id, uint32_t
}
int64_t Segment::get_metadata_size() const {
- return sizeof(Segment) + (_footer_pb ? _footer_pb->ByteSizeLong() : 0) +
Review Comment:
不是所有 _footer_pb 都在 page cache 里，所以这里不能删
_footer_pb->ByteSizeLong()，还要统计在segment内存中， memory profile 计算 segment
内存的时候，再把cache的部分减去，类似：
所以这里先别删 _footer_pb->ByteSizeLong()，后面再看看咋改更好
##########
be/src/olap/page_cache.cpp:
##########
@@ -17,38 +17,58 @@
#include "olap/page_cache.h"
+#include <gen_cpp/segment_v2.pb.h>
#include <glog/logging.h>
#include <ostream>
#include "runtime/exec_env.h"
namespace doris {
-template <typename TAllocator>
-PageBase<TAllocator>::PageBase(size_t b, bool use_cache,
segment_v2::PageTypePB page_type)
- : LRUCacheValueBase(), _size(b), _capacity(b) {
+
+template <typename T>
+MemoryTrackedPageBase<T>::MemoryTrackedPageBase(size_t size, bool use_cache,
+ segment_v2::PageTypePB
page_type)
+ : _size(size) {
if (use_cache) {
_mem_tracker_by_allocator =
StoragePageCache::instance()->mem_tracker(page_type);
} else {
_mem_tracker_by_allocator =
thread_context()->thread_mem_tracker_mgr->limiter_mem_tracker();
}
+}
+
+MemoryTrackedPageWithPageEntity::MemoryTrackedPageWithPageEntity(size_t size,
bool use_cache,
+
segment_v2::PageTypePB page_type)
+ : MemoryTrackedPageBase<char*>(size, use_cache, page_type),
_capacity(size) {
{
- SCOPED_SWITCH_THREAD_MEM_TRACKER_LIMITER(_mem_tracker_by_allocator);
- _data = reinterpret_cast<char*>(TAllocator::alloc(_capacity,
ALLOCATOR_ALIGNMENT_16));
+
SCOPED_SWITCH_THREAD_MEM_TRACKER_LIMITER(this->_mem_tracker_by_allocator);
+ this->_data = reinterpret_cast<char*>(
+ Allocator<false>::alloc(this->_capacity,
ALLOCATOR_ALIGNMENT_16));
}
}
-template <typename TAllocator>
-PageBase<TAllocator>::~PageBase() {
- if (_data != nullptr) {
- DCHECK(_capacity != 0 && _size != 0);
- SCOPED_SWITCH_THREAD_MEM_TRACKER_LIMITER(_mem_tracker_by_allocator);
- TAllocator::free(_data, _capacity);
+MemoryTrackedPageWithPageEntity::~MemoryTrackedPageWithPageEntity() {
+ if (this->_data != nullptr) {
+ DCHECK(this->_capacity != 0 && this->_size != 0);
+
SCOPED_SWITCH_THREAD_MEM_TRACKER_LIMITER(this->_mem_tracker_by_allocator);
+ Allocator<false>::free(this->_data, this->_capacity);
}
}
-template class PageBase<Allocator<true>>;
-template class PageBase<Allocator<false>>;
+template <typename T>
+MemoryTrackedPageWithPagePtr<T>::MemoryTrackedPageWithPagePtr(size_t size,
+
segment_v2::PageTypePB page_type)
+ : MemoryTrackedPageBase<std::shared_ptr<T>>(size, true, page_type) {
+ DCHECK(this->_size > 0);
+ this->_size = size;
+ this->_mem_tracker_by_allocator->consume(this->_size);
+}
+
+template <typename T>
+MemoryTrackedPageWithPagePtr<T>::~MemoryTrackedPageWithPagePtr() {
+ DCHECK(this->_size > 0);
+ this->_mem_tracker_by_allocator->release(this->_size);
Review Comment:
LRUCachePolicy::insert 统计后，LRUCacheValueBase 析构的时候会 release
--
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
