Implement Skip for DeltaBitPackDecoder #2393
Conversation
|
@tustvold PATL😊 |
parquet/src/encodings/decoding.rs
Outdated
| let batch_to_skip = self.mini_block_remaining.min(to_skip - skip); | ||
|
|
||
| for i in 0..batch_to_skip { | ||
| if let Some(v) = self.bit_reader.get_value::<T::T>(bit_width) { |
There was a problem hiding this comment.
get_value is pretty slow, it would be better to read in batches of the miniblock size (typically 32 or 64)
parquet/src/encodings/decoding.rs
Outdated
| let bit_width = self.mini_block_bit_widths[self.mini_block_idx] as usize; | ||
| let batch_to_skip = self.mini_block_remaining.min(to_skip - skip); | ||
|
|
||
| for i in 0..batch_to_skip { |
There was a problem hiding this comment.
The key optimisation that I think would be good to include is if you don't need to read any values from a block, you don't need to keep track of last_value for the block, and can just skip over the values
There was a problem hiding this comment.
Effectively you can only skip efficiently in multiples of the block size, as otherwise you are still having to decode all the value
@tustvold after checking with the code, i think there is only one [first value] in each col chunk, so we need read all values to calculate the last_value? 🤔
Am i right?
There was a problem hiding this comment.
Yup... Darn... I guess we just have to decode each block 😞
There was a problem hiding this comment.
Maybe i can try to use a suitable skip_buffer, try not have performance downgrade.
Thanks for your useful guidance! 😭 there is a regressed ! (this use |
|
there is no performance downgrade and avoid allocate large buffer when skip large amount values. |
|
Looks good to me, thank you 👍 |
|
Benchmark runs are scheduled for baseline = 42e1068 and contender = d11b388. d11b388 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Closes #2281 .
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?