bench(parquet): cover DELTA_BYTE_ARRAY at sub-page-limit value sizes - #10550
Merged
etseidl merged 1 commit intoAug 4, 2026
Merged
Conversation
The existing delta benches use 2 MiB values against the default 1 MiB page limit, where every value is cut onto its own page and prefix state is reset each time -- DELTA_BYTE_ARRAY output there is byte-identical to PLAIN, so the shared-prefix scan never runs to depth. Add 1 KiB values, which sit far below the limit and share a page, across full / partial / no shared prefix.
etseidl
approved these changes
Aug 4, 2026
Contributor
|
Thanks @adriangb! |
etseidl
added a commit
that referenced
this pull request
Aug 4, 2026
#10549) > **Stacked on #10550 This branch is > that PR's benchmark commit plus one commit of its own, so that the numbers > below are reproducible with `cargo bench` on this branch alone. Review only > the second commit here; the diff shrinks to > `+139/-15` once #10550 merges and this rebases onto `main`. # Which issue does this PR close? None directly. Split out of #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 tests. Those round trips are weaker evidence than they look, in two ways. First, they never reach the new code path: `ByteArrayType::test` and `FixedLenByteArrayType::test` feed random values, which share no prefix, and the `ArrowWriter` cases write values a handful of bytes long — so nothing in the suite writes two consecutive values sharing 32 bytes, and the block loop never runs. Second, a round trip is structurally blind to an under-counted prefix: the encoder just emits a correspondingly longer suffix and the decoder reconstructs the same bytes either way. Only over-counting shows up. `test_estimated_data_encoded_size` does assert an exact encoded size, but on 2- and 3-byte values that can never enter the block loop. New coverage, unit level, in `parquet/src/util/prefix.rs`: 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. Plus unequal lengths combined with a mismatch past a block boundary (where truncating to the shorter length interacts with the block loop), non-zero slice start offsets (callers pass sub-slices into shared Arrow buffers, not freshly allocated `Vec`s), and a prefix ending mid-UTF-8-codepoint — byte-level prefixes may split a multi-byte character, which was true of the byte-wise scan too and is worth pinning now that the scan is wider. New coverage, end to end, for both call sites: `test_delta_byte_array_long_shared_prefix{,_fixed_len}` in `parquet/src/encodings/encoding/mod.rs` and `delta_byte_array_long_shared_prefix` in `parquet/src/arrow/arrow_writer/mod.rs`. Each writes values with a 1000-byte shared prefix — not a multiple of the 32-byte block, so the scan has to resolve a partial block — and asserts on the prefix lengths actually written, decoded back out of the page, rather than on a round trip alone. Those assertions were checked for power by mutation: dropping the sub-block tail scan from `common_prefix_length` (so it under-counts by up to 31 bytes) leaves all 156 `arrow::arrow_writer` tests passing on `main`, and fails all three new tests. 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. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Ed Seidl <etseidl@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
None. This is benchmark coverage split out of #10549 so that the performance change proposed there can be reviewed against benchmarks that already exist on
main.Rationale for this change
The existing
DELTA_BYTE_ARRAYwriter benchmarks (added in #10512) write 128 values of 2 MiB each against the default 1 MiBdata_page_size_limit. Every value exceeds the limit, so each one is cut onto its own data page, and each page boundary clears the encoder's previous-value state. Every prefix length is therefore 0.Measured on
main, writing that benchmark's ownlarge_string_shared_prefixdata (256 MiB raw input):At the default limit the
DELTA_BYTE_ARRAYoutput is byte-for-byte whatPLAINproduces — the encoding is doing no work, so those benchmarks cannot measure anything about prefix scanning. That is the known regression #10489.The new benchmarks use 1 KiB values, far below the page limit, so roughly 1000 values share a page and the previous-value state survives across them. That is the regime
DELTA_BYTE_ARRAYis actually deployed in.What changes are included in this PR?
Three new criterion benchmark groups in
bench_small_delta_byte_array_writers, each writing 8192 rows of 1 KiB strings with bothPLAIN(as a control/baseline) andDELTA_BYTE_ARRAY:small_string_shared_prefix— values differ only in a trailing 8-byte counter, so each prefix scan covers nearly the whole value.small_string_partial_prefix— values share their first 512 bytes and then diverge, the realistic sorted-column case (paths, URLs, keys). Uses a newcreate_string_partial_prefix_bench_batchhelper.small_string_distinct— values differ from byte 0, so prefix deduplication saves nothing.No library code is touched.
Are these changes tested?
These are benchmarks. The benchmark binary compiles, and
cargo fmtandcargo clippy -p parquet --benches --all-features -- -D warningspass.The benchmarks were also run, to confirm they resolve real differences rather than noise. They were validated by measuring an actual candidate change against them — the block-wise shared-prefix scan in #10549 — on an A/B/A schedule (baseline, branch, baseline again) so that machine drift is quantified rather than assumed. The
plainrows act as controls, sincePLAINnever calls the prefix scan. Times are means in ms, aarch64:The shared-prefix case resolves a 4.2x difference and the partial-prefix case a 2.7x difference, both far above the largest control excursion. The distinct case is flat, which is the correct outcome — there is no prefix to scan there. One caveat: the
small_string_partial_prefix/plaincontrol had a single noisy reading (0.853 against baselines of 0.603 and 0.639), so that row's noise floor is wider than the others; the delta effect on that bench is still several times larger than that excursion.The baseline columns also show that on
mainthe shared-prefix case costs 2.83 ms withDELTA_BYTE_ARRAYversus 0.68 ms withPLAIN— the encoding is currently about 4x more expensive thanPLAINon exactly the data it exists for.Are there any user-facing changes?
No. Benchmark-only change; no library code is touched.