feat(storage): replace lru-cache so that we can control memory usage precisely #1994
Conversation
Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>
Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>
Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>
Little-Wallace
force-pushed
the
wallace/block-cache
branch
from
62945b3
to
ff625c6
Compare
Codecov Report
@@ Coverage Diff @@
## main #1994 +/- ##
==========================================
+ Coverage 70.82% 70.87% +0.04%
==========================================
Files 639 639
Lines 81173 81361 +188
==========================================
+ Hits 57494 57664 +170
- Misses 23679 23697 +18
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>
Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>
Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>
Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>
Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>
Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>
Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>
| for sender in que { | ||
| (*ptr).add_ref(); | ||
| let _ = sender.send(CachableEntry { | ||
| cache: self.clone(), |
There was a problem hiding this comment.
Can we implement Clone for CachableEntry?
There was a problem hiding this comment.
Seems we have implemented it. We can use entry.clone() instead.
There was a problem hiding this comment.
no. it will lock again.
There was a problem hiding this comment.
Same as the above comments. If we use AtomicUsize for reference counting, a clone won't involve a lock.
| } else { | ||
| 0 | ||
| } | ||
| hash as usize % self.shards.len() |
There was a problem hiding this comment.
Why use mod instead of right shift?
There was a problem hiding this comment.
They are similar
There was a problem hiding this comment.
Seems that mod is more expensive than a right shift.
And if we use the original right shift, we use the high bits of the hash value, which can be separated from the bits we use for hash table, and the load can probably be more dispersed.
src/storage/src/hummock/cache.rs
Outdated
| @@ -573,6 +597,11 @@ impl<K: LruKey, T: LruValue> LruCache<K, T> { | |||
| drop(data); | |||
| } | |||
|
|
|||
| unsafe fn add_ref(&self, handle: *mut LruHandle<K, T>) { | |||
There was a problem hiding this comment.
May want to AtomicUsize for reference count like Arc to avoid locking here
There was a problem hiding this comment.
no. You can see that whether we use atomicusize or not, we must lock the shard because of other operations
There was a problem hiding this comment.
If we use atomicusize, the clone of cache entry don't need to acquire the lock.
Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>
Signed-off-by: Little-Wallace <bupt2013211450@gmail.com>
…ingwave into wallace/block-cache
What's changed and what's your intention?
TableHolderandBlockHolderfor Iterator so that the memory in cache will not be released.Following #1884
Checklist
Refer to a related PR or issue link (optional)
part of #1773