Repository navigation
Conversation
|
Thanks for opening a pull request! This pull request has been automatically converted to a draft because its title doesn't match Arrow's required format. If this is not a minor PR, could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project. Then could you also rename the pull request title in the following format? or After updating the title, you can mark the pull request as ready for review. See also: |
|
|
There was a problem hiding this comment.
🟢 Approval recommended
The localized change preserves decoding semantics and has no identified blocking issues; performance gains were not independently verified.
No open issues 🎉
What changed in this PR
This PR hoists ALP exponent/factor retrieval out of the C++ decompression loop to improve Clang vectorization without changing decoding behavior.
Changes:
- Copies exponent/factor metadata once before decoding.
- Documents how reading the byte-sized metadata inside the loop can inhibit vectorization.
| File | Description |
|---|---|
cpp/src/arrow/util/alp/alp_compression.cc |
Reuses cached exponent/factor metadata throughout the decode loop. |
Review effort: Balanced
|
Benchmark result on my EPYC EPYC 9654 and clang++18 |
Rationale for this change
See: https://godbolt.org/z/EPPqn7xco
What changes are included in this PR?
Extract the alp argument out, this brings optimization in clang ( at least clang-18 ).
According to the godbolt, gcc can optimize this well, so using gcc might not seeing this optimization.
Are these changes tested?
No
Are there any user-facing changes?
No
Was AI used for this PR?
AI help me to test the cases in different machines
PR code and description written by:
Reviewed before submission by: