github-actions[bot] commented on code in PR #60897:
URL: https://github.com/apache/doris/pull/60897#discussion_r2928860819
##########
be/src/storage/segment/condition_cache.cpp:
##########
@@ -49,4 +49,32 @@ void ConditionCache::insert(const CacheKey& key,
std::shared_ptr<std::vector<boo
result->capacity(), result->capacity(),
CachePriority::NORMAL));
Review Comment:
Bug: `std::vector<bool>::capacity()` returns the number of *elements*
(bits), not the number of bytes consumed. Since `std::vector<bool>` is
bit-packed, a vector with capacity 2048 uses ~256 bytes of storage, but reports
capacity 2048. This means the LRU cache charge is inflated by ~8x, causing the
cache to hold far fewer entries than its configured memory limit allows.
The same issue exists on line 76 for the `ExternalCacheKey` overload.
Suggested fix:
```cpp
size_t charge = (result->capacity() + 7) / 8 + sizeof(CacheValue);
ConditionCacheHandle(
this,
LRUCachePolicy::insert(key.encode(),
(void*)cache_value_ptr.release(),
charge, charge, CachePriority::NORMAL));
```
Or use `result->size()` if you prefer exact element count, but still divide
by 8 to get byte-level charge.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
