Faster byte range cache - #6683
Conversation
Partition cached ranges by path using an FxHashMap so range lookups only compare integer offsets within the selected file.
Resolve each path's cache when opening a file so range reads avoid repeated path lookups.
| if Self::get_block(&self.blocks, byte_range.start, byte_range.end).is_none() { | ||
| Self::merge_ranges(&mut self.blocks, byte_range.start, byte_range.end)?; | ||
| } | ||
| let (block_start, value) = Self::get_block(&self.blocks, byte_range.start, byte_range.end)?; |
There was a problem hiding this comment.
if get_block was Some the first time, we call it again immediately rather than keeping the result ?!
There was a problem hiding this comment.
changed it to a single lookup
| fn get_block( | ||
| blocks: &BTreeMap<usize, CacheValue>, |
There was a problem hiding this comment.
why does this not take &self? borrow checker issues?
There was a problem hiding this comment.
no reason really, changed it to self
not relevant here, but fun fact: passing function parameters proves to the compiler these function parameter don't alias and produces better assembly
There was a problem hiding this comment.
so passing 3-4 args instead of a single self that contains those 3-4 args can generate better assembly because there are more NoAliases in the llvm IR?
that's... surprising
There was a problem hiding this comment.
Yes, more info here: rust-lang/rust#149670
I encountered it with a 30% regression on term aggregations
They could prove they don't alias, but it's too hard apparently
3 participants
Optimize byte range cache lookups.
In regex queries we have a lot of
FileHandle::read_byteslookups, which are passed toNeedMutByteRangeCache::get_slice. The path comparison was very expensive in these cases, although a lookup will never cross path boundaries.This PR removes the repeated path lookups by moving it to one layer above to the
FileHandle