viirya opened a new pull request, #12177:
URL: https://github.com/apache/arrow-rs/pull/12177
# Which issue does this PR close?
- Closes #12176
# Rationale for this change
`ByteArrayDecoderPlain::read` and the `DELTA_BYTE_ARRAY` decoder call
`OffsetBuffer::try_push` for every value, but it isn't inlined into their
loops. So every value pays for a call, and the lengths and capacities of the
output buffers are reloaded around it. See the issue for details and the
alternatives I measured.
# What changes are included in this PR?
`OffsetBuffer::try_push` is now `#[inline(always)]`. This one attribute is
the whole change, so it is also where all of the gain comes from. Plain
`#[inline]` isn't enough: with it, the disassembly still shows
`ByteArrayDecoderPlain::read::<i32>` calling `try_push` out of line.
# Are these changes tested?
The behaviour doesn't change. It is covered by the existing `OffsetBuffer`
and `ByteArrayDecoderPlain` unit tests and by the `invalid_utf8` reader tests.
Target benchmarks: `arrow_reader`
`arrow_array_reader/{StringArray,BinaryArray}/plain encoded*` and
`arrow_array_reader/StringArray/*delta byte array*` (`run benchmark
arrow_reader`).
Benchmarks were run on an M4 Max, on macOS and in an aarch64 Linux container
(Debian bookworm, glibc) on the same machine. `main` and this PR were built as
separate binaries and run alternately. The machine has some background load, so
treat changes within ±3% as noise.
`cargo bench -p parquet --bench arrow_reader`, median of 3 alternating
rounds:
| case | macOS main | macOS PR | change | Linux main | Linux PR | change |
|---|---|---|---|---|---|---|
| StringArray/plain encoded, mandatory, no NULLs | 456 µs | 389 µs | -15% |
285 µs | 233 µs | -18% |
| StringArray/plain encoded, optional, no NULLs | 477 µs | 398 µs | -17% |
288 µs | 237 µs | -18% |
| StringArray/plain encoded, optional, half NULLs | 340 µs | 303 µs | -11% |
289 µs | 265 µs | -8% |
| BinaryArray/plain encoded, mandatory, no NULLs | 460 µs | 354 µs | -23% |
270 µs | 195 µs | -28% |
| BinaryArray/plain encoded, optional, no NULLs | 463 µs | 355 µs | -23% |
272 µs | 196 µs | -28% |
| BinaryArray/plain encoded, optional, half NULLs | 334 µs | 274 µs | -18% |
277 µs | 244 µs | -12% |
| StringArray/const prefix delta byte array encoded, mandatory, no NULLs |
672 µs | 600 µs | -11% | 649 µs | 644 µs | -1% |
| StringArray/const delta byte array encoded, mandatory, no NULLs | 511 µs |
445 µs | -13% | 422 µs | 340 µs | -19% |
| StringArray/const delta length byte array encoded, mandatory, no NULLs |
154 µs | 154 µs | +0% | 186 µs | 188 µs | +1% |
| StringArray/dictionary encoded, mandatory, no NULLs | 303 µs | 300 µs |
-1% | 177 µs | 178 µs | +0% |
| StringArray/dictionary encoded, optional, half NULLs | 257 µs | 252 µs |
-2% | 229 µs | 232 µs | +1% |
| BinaryArray/dictionary encoded, mandatory, no NULLs, large values | 5.72
ms | 5.70 ms | -0% | 7.74 ms | 7.81 ms | +1% |
The `arrow_reader` PLAIN values are well predicted: they are all about the
same length (35-40 B). For random lengths, which make the copy hard to predict,
I also scanned 262,144 rows of a single PLAIN encoded column (dictionary
disabled) from an in-memory file with `ParquetRecordBatchReader`. Values are
random lowercase ASCII. The column is `required`, or `optional` with N% of rows
NULL. Each number is the median of 41 scans; the table shows the median over 5
alternating rounds on macOS and 8 on Linux:
| column | macOS main | macOS PR | change | Linux main | Linux PR | change |
|---|---|---|---|---|---|---|
| Utf8, 0-16 B, required | 2.50 ms | 2.24 ms | -10% | 1.64 ms | 1.43 ms |
-13% |
| Utf8, 0-16 B, optional 10% | 2.90 ms | 2.68 ms | -8% | 2.11 ms | 1.89 ms |
-10% |
| Utf8, 8 B, required | 952 µs | 770 µs | -19% | 762 µs | 622 µs | -18% |
| Utf8, 0-48 B, required | 3.02 ms | 2.83 ms | -6% | 1.77 ms | 1.54 ms |
-13% |
| Utf8, 17-64 B, required | 3.28 ms | 3.06 ms | -7% | 1.27 ms | 1.09 ms |
-15% |
| Utf8, 17-64 B, optional 10% | 3.68 ms | 3.49 ms | -5% | 1.76 ms | 1.60 ms
| -9% |
| Utf8, 65-256 B, required | 3.00 ms | 2.59 ms | -14% | 2.65 ms | 2.36 ms |
-11% |
| Utf8, 65-256 B, optional 10% | 3.36 ms | 2.99 ms | -11% | 3.52 ms | 3.25
ms | -8% |
| Utf8, 1000-2000 B, required | 16.03 ms | 15.85 ms | -1% | 19.98 ms | 19.73
ms | -1% |
| Utf8, all empty, required | 606 µs | 369 µs | -39% | 542 µs | 342 µs |
-37% |
| Utf8, all empty, optional 10% | 1.17 ms | 942 µs | -19% | 1.12 ms | 904 µs
| -19% |
| Utf8, 0-16 B, optional 90% | 873 µs | 832 µs | -5% | 660 µs | 641 µs | -3%
|
| Utf8, 17-64 B, optional 90% | 940 µs | 919 µs | -2% | 651 µs | 618 µs |
-5% |
| Binary, 0-16 B, required | 2.52 ms | 2.23 ms | -11% | 1.66 ms | 1.39 ms |
-16% |
| Binary, 17-64 B, optional 10% | 3.62 ms | 3.38 ms | -7% | 1.77 ms | 1.55
ms | -12% |
| LargeUtf8, 0-16 B, required | 2.52 ms | 2.27 ms | -10% | 1.60 ms | 1.43 ms
| -11% |
| LargeUtf8, 17-64 B, optional 10% | 3.71 ms | 3.47 ms | -7% | 1.74 ms |
1.64 ms | -6% |
| LargeBinary, 0-16 B, required | 2.55 ms | 2.23 ms | -13% | 1.60 ms | 1.40
ms | -12% |
On Linux, the `LargeUtf8`/`LargeBinary` 17-64 B optional cases are bimodal
for both `main` and this PR. `main` measured 1.74-2.31 ms depending on the
build, and individual rounds varied by up to 30%. So I wouldn't read anything
into those two rows (`LargeBinary` is left out above).
Both the random-length rows and the well-predicted rows (8 B and all empty)
get faster. No input got slower than ±3% on either platform. I haven't measured
x86_64, so a run of the benchmark bot there would be useful.
# Are there any significant user-facing changes?
No.
This pull request and its description were written by Isaac.
--
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]
