[opt](memory) All LRU Cache inherit from LRUCachePolicy #28940
Conversation
|
run buildall |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G' |
|
(From new machine)TeamCity pipeline, clickbench performance test result: |
|
run buildall |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
run buildall |
|
clang-tidy review says "All clean, LGTM! 👍" |
|
TeamCity be ut coverage result: |
|
TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G' |
|
(From new machine)TeamCity pipeline, clickbench performance test result: |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
xinyiZzz
force-pushed
the
20231213_fix_cache
branch
from
47cee24 to
b29e997
Compare
xinyiZzz
force-pushed
the
20231213_fix_cache
branch
from
1956fc2 to
386f8ce
Compare
|
run buildall |
|
run buildall |
|
TeamCity be ut coverage result: |
| init(capacity, lru_cache_type, num_shards); | ||
| } | ||
|
|
||
| bool prepare(size_t capacity, uint32_t num_shards) { |
There was a problem hiding this comment.
prepare 和 init 实际是相同的意思。
这个函数，你实现的实际就是check。
There was a problem hiding this comment.
em。。因为把 check init 和 init = true 也加进来了，所以改叫 prepare
| _enable_prune = false; | ||
| return false; | ||
| } | ||
| _is_init = true; |
There was a problem hiding this comment.
为啥在prepare 里更新 is_init=true？ 而不是在init里？ 这yang很混乱。
There was a problem hiding this comment.
因为三个 init 方法要写三次啊 = =
be/src/olap/page_cache.h
Outdated
| @@ -177,20 +177,20 @@ class StoragePageCache { | |||
| Cache* _get_page_cache(segment_v2::PageTypePB page_type) { | |||
| switch (page_type) { | |||
| case segment_v2::DATA_PAGE: { | |||
| if (_data_page_cache) { | |||
| return _data_page_cache->get(); | |||
| if (_data_page_cache->is_init()) { | |||
There was a problem hiding this comment.
上面有个 is_available 方法检查了 nullptr，用于短路 page cache 的使用，而 _data_page_cache->get() 不会返回nullptr
已修改，把 is_available 和 is_init 都删了，不需要短路了
| return _cache.get(); | ||
| } | ||
|
|
||
| void reset() { _cache.reset(); } |
|
(From new machine)TeamCity pipeline, clickbench performance test result: |
|
TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G' |
xinyiZzz
force-pushed
the
20231213_fix_cache
branch
from
a49d9fe to
a63ee59
Compare
|
run buildall |
|
TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G' |
|
run buildall |
|
TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G' |
|
(From new machine)TeamCity pipeline, clickbench performance test result: |
|
run buildall |
|
run buildall |
|
TeamCity be ut coverage result: |
|
PR approved by at least one committer and no changes requested. |
github-actions
bot
added
approved
|
PR approved by anyone and no changes requested. |
|
TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G' |
|
(From new machine)TeamCity pipeline, clickbench performance test result: |
After all LRU Cache inherits from LRUCachePolicy, this will allow prune stale entry, eviction when memory exceeds limit, and define common properties. LRUCache constructor change to private, only allow LRUCachePolicy to construct it. Impl DummyLRUCache, when LRU Cache capacity is 0, will no longer be meaningless insert and evict.
Proposed changes
After all LRU Cache inherits from
LRUCachePolicy, this will allow prune stale entry, eviction when memory exceeds limit, and define common properties.LRUCacheconstructor change to private, only allowLRUCachePolicyto construct it.Impl DummyLRUCache, when LRU Cache capacity is 0, will no longer be meaningless insert and evict.
Further comments
If this is a relatively large or complex change, kick off the discussion at [email protected] by explaining why you chose the solution you did and what alternatives you considered, etc...