adriangb opened a new pull request, #10549:
URL: https://github.com/apache/arrow-rs/pull/10549

   # Which issue does this PR close?
   
   None directly. Split out of https://github.com/apache/arrow-rs/pull/10505 so 
that the correctness fix there can be reviewed without an unrelated performance 
change attached to it.
   
   # Rationale for this change
   
   `DELTA_BYTE_ARRAY` stores each value as the number of leading bytes it 
shares with its predecessor plus the remaining suffix, so writing a value runs 
a shared-prefix scan against the previous value. Both encoder paths implement 
that scan as a byte-at-a-time loop:
   
   - `DeltaByteArrayEncoder::put` in `parquet/src/encodings/encoding/mod.rs` 
(the generic `SerializedFileWriter` path)
   - `FallbackEncoder::encode`'s `Delta` arm in 
`parquet/src/arrow/arrow_writer/byte_array.rs` (the `ArrowWriter` path)
   
   The scan runs once per value, and on exactly the data the encoding exists 
for — near-identical consecutive values — it covers essentially the whole 
value. That makes its throughput, not its per-call overhead, the thing that 
matters, and a byte-at-a-time loop is the slowest way to do it.
   
   # What changes are included in this PR?
   
   Extract the two duplicated loops into 
`crate::util::prefix::common_prefix_length` and compare a 32-byte block at a 
time instead of a byte at a time.
   
   32 is the widest block that both aarch64 and x86-64 still expand inline; at 
64 bytes x86-64 drops to an out-of-line `bcmp` call, which costs more than the 
extra width buys. Measured on aarch64, every width from 16 up performs the 
same, so this sits in the middle of a flat optimum rather than on a tuned peak.
   
   No behavior change: the function returns the same prefix length the 
byte-wise loops did, and no page layout, encoding, or file output changes.
   
   # Are these changes tested?
   
   Existing coverage: the full `parquet` suite passes unmodified (1307 tests), 
including the `DELTA_BYTE_ARRAY` round-trip and `arrow_writer_layout` tests 
that would catch any change in prefix lengths.
   
   New coverage: unit tests in `parquet/src/util/prefix.rs` for the boundary 
cases the block loop introduces — empty inputs, prefixes shorter than / equal 
to / longer than one block, a mismatch in the first and last byte of a block, 
and unequal lengths where one input is a strict prefix of the other.
   
   Benchmarked with `parquet/benches/arrow_writer.rs`'s 
`bench_delta_byte_array_writers`, added in #10512. Results in a comment below.
   
   # Are there any user-facing changes?
   
   No API changes and no change to written output. `DELTA_BYTE_ARRAY` writes 
get faster.
   


-- 
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]

Reply via email to