prtkgaur opened a new pull request, #51249: URL: https://github.com/apache/arrow/pull/51249
### Rationale for this change `min_delta_` and `last_value_` are members of the same type as `GetInternal`'s output buffer, and that buffer points into memory the caller owns, so the compiler cannot prove the prefix-sum store does not land on either member. It reloads the frame and stores the running value on every value: on aarch64 the loop body is 8 instructions with 4 memory operations per value, where 6 and 2 are enough. ### What changes are included in this PR? In the non-zero-bit-width branch of `DeltaBitPackDecoder<DType>::GetInternal`, hold the running value and the frame in locals across the loop and write `last_value_` back once when the loop ends. The arithmetic is unchanged -- every term stays in the unsigned type, so the documented wrapping behaviour is preserved and no decoded value changes. ### Are these changes tested? By the existing tests: this is a compilation concern, not a behavioural one, and `parquet-encoding-test` already round trips DELTA_BINARY_PACKED over both integer widths including the overflow cases that exercise the wrapping. ### Benchmark `parquet-encoding-benchmark`, the DELTA_BINARY_PACKED decode arms already in the tree. AWS Graviton4, GCC 11.5, `Release`, pinned to one core, 9 repetitions, medians, 65,536 values; main and this PR built and measured in the same sitting. The two `Fixed` arms are the `delta_bit_width_ == 0` fast path, which this diff cannot execute. | benchmark | main | this PR | | |---|--:|--:|--:| | `Decode_Int32_Narrow` | 99.67 us | 78.11 us | **1.28x** | | `Decode_Int32_Wide` | 101.95 us | 80.79 us | **1.26x** | | `Decode_Int64_Narrow` | 82.20 us | 64.41 us | **1.28x** | | `Decode_Int64_Wide` | 321.17 us | 308.65 us | 1.04x | | `Decode_Int32_Fixed` | 19.75 us | 19.75 us | 1.00x | | `Decode_Int64_Fixed` | 30.65 us | 32.20 us | 0.95x | ### Are there any user-facing changes? No. No API change, no format change, and decoded values are identical. **This PR includes breaking changes to public APIs.** (If there are any breaking changes to public APIs, please explain which changes are breaking. If not, you can remove this.) **This PR contains a "Critical Fix".** (If the changes fix either (a) a security vulnerability, (b) a bug that caused incorrect or invalid data to be produced, or (c) a bug that causes a crash (even when the API contract is upheld), please provide explanation. If not, you can remove this.) -- 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]
