Pass decompressed size to parquet Codec::decompress (#2956) #2959
Conversation
Added optional argument uncompressed_size to Coded::decompress to do a better estimation of the required uncompress size. * snappy: Probably no much improvement as `decompress_len` is already accurate. * gzip: No improvement. Ignores the size hint. * brotli: Probably no much improvement. The buffer size will be equal to the uncompressed_size size. * lz4: No improvement. As the buffer is located at the stack there are no extra allocations. Then it probably is better to keep it working as it is. * zstd: No improvement. Ignores the size hint. * lz4_raw: Improvement. The estimation method over-estimates, so knowin the uncompressed size reduces allocations.
A page may contain header, uncompressed size includes the header size. The `decompress` method expects to receive the `uncompress_size` for the compress block, that is without the page headers.
|
It seems that the estimation of lz4 uncompress size can cause overflow for small compress size. Any compress size smaller than 10 will overflow and as though it will panic. So I see too options now:
I would go with the second one, as even if the overflow error is only for small compression sizes, if the compressed size is 1G it will reserve ~250GB which is too much. So I would avoid prediction. What do you think? |
|
Benchmark runs are scheduled for baseline = 94a7f4b and contender = 344c552. 344c552 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 #2956.
Rationale for this change
What changes are included in this PR?
Added optional argument
uncompressed_sizetoCoded::decompressto do a better estimation of the required uncompress size.decompress_lenis already accurate.Are there any user-facing changes?
Breaking changes on
Codectrait. It only affects to users withexperimentalfeature enable.