feat(stats): introduce block efficiency metrics #14654
Conversation
Signed-off-by: MrCroxx <[email protected]>
Signed-off-by: MrCroxx <[email protected]>
Signed-off-by: MrCroxx <[email protected]>
Signed-off-by: MrCroxx <[email protected]>
Signed-off-by: MrCroxx <[email protected]>
Signed-off-by: MrCroxx <[email protected]>
Signed-off-by: MrCroxx <[email protected]>
- local hitmap is updated when iter - global hitmap is updated when local hitmap drops - metrics is updated when block is evicted from block cache Signed-off-by: MrCroxx <[email protected]>
Signed-off-by: MrCroxx <[email protected]>
| } | ||
| } | ||
|
|
||
| pub fn fill(&mut self, start_bit: usize, end_bit: usize) { |
There was a problem hiding this comment.
fill of LocalHitmap and Hitmap is basically the same. Is it possible to unify their code in a macro or generic method?
Or do you have any usage of fill of Hitmap? If you only use the fill of LocalHitmap, we may only keep the fill of LocalHitmap.
| pub fn slice(&self, range: impl RangeBounds<usize>) -> &[u8] { | ||
| let start = range.start().unwrap_or_default(); | ||
| let end = range.end().unwrap_or(self.data_len); | ||
| &self.data[start..end] |
There was a problem hiding this comment.
Shall we forward the idx range to slice directly just like how we originally use the range?
There was a problem hiding this comment.
I didn't found any function of slice that receives a impl RangeBounds<usize> to forward it directly. I'll restore the function to fn data(&self) -> &[u8] as it does include any other operations any more.
|
|
||
| self.value_range = prefix.value_range(); | ||
| self.offset = offset; | ||
| self.entry_len = prefix.entry_len(); | ||
|
|
||
| self.hitmap.fill_with_ratio( |
There was a problem hiding this comment.
May use fraction instead of f64, or we pass (start, end, len) of usize. May increase unnecessary cost on floating point computation for each key.
Signed-off-by: MrCroxx <[email protected]>
Signed-off-by: MrCroxx <[email protected]>
| pub fn fill(&mut self, start_bit: usize, end_bit: usize) { | ||
| const MASK: usize = (1 << 6) - 1; | ||
|
|
||
| let end_bit = std::cmp::min(end_bit, Self::bits()); |
There was a problem hiding this comment.
Should we compare end_bit with Self::bits()-1 if end_bit is inclusive?
There was a problem hiding this comment.
end_bit is exclusive.
| let head_elem = start_bit >> 6; | ||
| let tail_elem = end_bit >> 6; | ||
|
|
||
| for i in head_elem..=std::cmp::min(tail_elem, N - 1) { |
There was a problem hiding this comment.
Is this cmp necessary if we have compared end_bit and Self::bits?
There was a problem hiding this comment.
For end_bit is exclusive, the comparison is needed here.
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
Introduce block efficiency metrics to track the ratio of how much a block is read from between loading and eviction from the block cache by table id.
Checklist
./risedev check(or alias,./risedev c)Documentation
Release note
If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.