Ext3h commented on code in PR #48192:
URL: https://github.com/apache/arrow/pull/48192#discussion_r2559600380
##########
cpp/src/arrow/util/compression_zstd.cc:
##########
@@ -187,9 +202,14 @@ class ZSTDCodec : public Codec {
DCHECK_EQ(output_buffer_len, 0);
output_buffer = &empty_buffer;
}
-
- size_t ret = ZSTD_decompress(output_buffer,
static_cast<size_t>(output_buffer_len),
- input, static_cast<size_t>(input_len));
+ // Decompression context for ZSTD contains several large heap allocations.
Review Comment:
... after checking the Rust implementation, it really should have used a
`thread_local!` scoped context as well. That went badly, where it's now
creating that ZSTD context even if LZ4 is selected, it's creating one distinct
context per usage location, and it's still creating a new context for a lot of
potentially short-lived objects. Also it missed that there is not just a need
for the `CompressionContext` but also the `DecompressionContext` specifically
when talking about the IPC library which uses compression in both directions...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
