apache / apache/arrow

[C++] ZSTD context is re-allocated excessively and bypassing memory pool

Open
#48,187 1 comment 0 reactions 1 assignee Claimed by @Ext3h View on GitHub
Component: C++ Type: bug
Dominant language
C++
Stars
17.1k
Forks
4.3k
Avg merge
3d 13h
Merged PRs (30d)
88

Description

### Describe the bug, including details regarding any error messages, version, and platform.

`arrow::util::internal::ZSTDCodec::Compress` respectively `arrow::util::internal::ZSTDCodec::Decompress` are using a naked `ZSTD_compress` / `ZSTD_decompress` with usually quite small buffers which represents the absolute worst case scenario for the ZSTD API.

In the common use case of using Parquet with 8kB block size, a new ZSTD context of several MB in size is allocated for every single 8kB block, and then immediately released right after.

`ZSTDCodec` should explicitly create a compression/decompression context and explicitly re-use the corresponding context for subsequent calls to `Compress()` / `Decompress()`.

This mirrors the change already applied to the rust implementation of arrow: https://github.com/apache/arrow-rs/issues/8386 / https://github.com/apache/arrow-rs/pull/8405 - the same observations about the performance impact of the current lack of context reuse also applies to the C++ implementation.

In addition to the changes alread applied to the Rust implemetation, there's also `ZSTD_customMem` respectively `ZSTD_createCCtx_advanced` and `ZSTD_createDCtx_advanced` to consider - the `ZSTDCodec` can (and should) be properly slaved to the existing memory pool rather than being left to hit the systems default heap.

### Component(s)

C++

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.