[C++] GZipCodec::Init does deflateInit and immediately throws it away
- 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.
Performance issue in `GZipCodec`, no crashes/wrong results/..., but I still think it's a bug.
https://github.com/apache/arrow/blob/cd304b22da0ca1dc92edf86e6476d733f5ea5b05/cpp/src/arrow/util/compression_zlib.cc#L497-L501
This is wasteful: `InitCompressor` on line 497 calls `deflateInit2`, which at `windowBits=15` and `memLevel=8` allocates roughly 256 KB. `InitDecompressor` on line 501 calls `EndCompressor` which does `deflateEnd` and frees it again. It's pure waste. The only thing it gives us is some validation on the compression level.
The current code leaves the `GZipCodec` with a valid decompressor (which also allocates, but less) which is fine for the case where the caller wants to use the codec to do decompression, but is extra-wasteful for the case where the caller wants to compress: it allocates a compressor, frees it, allocates a decompressor, frees it, then allocates a compressor again.
I think the correct fix is https://github.com/firebolt-db/arrow/pull/47, I'm happy to file it as a PR here as well.
### Component(s)
C++
Contributor guide
Research direction
Start in cpp/src/arrow/util/compression_zlib.cc at lines 497-501 and trace GZipCodec::InitCompressor, InitDecompressor, and EndCompressor. Compare the current initialization sequence with the referenced proposed fix, then verify that compression-level validation remains intact while the redundant compressor allocation and release are gone.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- performance
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 76/100