Repository navigation
GH-50822 [C++][Parquet] Make DELTA_BYTE_ARRAY value reconstruction storage-agnostic - #50823
marcin-krystianc wants to merge 2 commits into
Conversation
|
|
7d36f89 to
550ba67
Compare
8d863c7 to
1433e54
Compare
bfac4bf to
dd5c36a
Compare
adamreeve
left a comment
There was a problem hiding this comment.
Thanks for this @marcin-krystianc
Looking at the benchmark results, I see that DeltaDecodingDenseFLBA is faster than it was before, and now consistently faster than the non-dense path, whereas before it was always slower. But the non-dense path results are slower in some cases than before. Is that just noise, or is it repeatable, and if so, is it possible to get back some of the performance for those rows?
I suspect it's not just noise, because it looks like the performance change is correlated with the prefix percent (10% prefixed is consistently slower, but 90% and 99% are mostly faster).
Could you also check before and after results for the BM_DeltaDecodingByteArray benchmarks?
| return max_values; | ||
| } | ||
|
|
||
| int GetInternal(ByteArray* buffer, int max_values) { |
There was a problem hiding this comment.
DecodeArrow still uses this path for the FLBA type, via DecodeArrowDense. Would it be possible to use DecodeDense for reading FLBA into Arrow, or would that be a much more complicated change?
| return GetInternalImpl(output, max_values); | ||
| } | ||
|
|
||
| int DecodePointers(FixedLenByteArray* buffer, int max_values) { |
There was a problem hiding this comment.
Nit: DecodeFixedLenByteArray might be a more suitable name.
Rationale for this change
Follow-up to #50335.
What changes are included in this PR?
Adds two output policy structs,
ByteArrayOutputandDenseOutputfor DeltaByteArrayFLBADecoder:ByteArrayOutputreproduces the previous behavior: it decodes values into aByteArray*bufferDenseOutputwrites each reconstructed fixed-length value directly into a caller-provideduint8_t*bufferAdds benchmarks to test the
DeltaByteArrayFLBADecoderperformance.Are these changes tested?
Yes
Are there any user-facing changes?
No