fix(parquet): cut data page byte-budget mini-batches on exact value counts - #10554
fix(parquet): cut data page byte-budget mini-batches on exact value counts#10554adriangb wants to merge 2 commits into
Conversation
967f1b7 to
5af13c0
Compare
|
run benchmark arrow_writer baseline: |
2 similar comments
|
run benchmark arrow_writer baseline: |
|
run benchmark arrow_writer baseline: |
|
run benchmark arrow_writer baseline: |
2 similar comments
|
run benchmark arrow_writer baseline: |
|
run benchmark arrow_writer baseline: |
|
Benchmark for this request failed. Run configurationrun benchmark arrow_writer
baseline:
ref: "bd5237f38ca3229694f42dd62139435d41eb93f5"Last 20 lines of output: Click to expandFile an issue against this benchmark runner |
|
Benchmark for this request failed. Run configurationrun benchmark arrow_writer
baseline:
ref: "bd5237f38ca3229694f42dd62139435d41eb93f5"Last 20 lines of output: Click to expandFile an issue against this benchmark runner |
|
Benchmark for this request failed. Run configurationrun benchmark arrow_writer
baseline:
ref: "bd5237f38ca3229694f42dd62139435d41eb93f5"Last 20 lines of output: Click to expandFile an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing claude/parquet-exact-value-windows-10538 (5af13c0) to main diff Run configurationrun benchmark arrow_writer
baseline:
ref: "main"BENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writer File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing claude/parquet-exact-value-windows-10538 (5af13c0) to main diff Run configurationrun benchmark arrow_writer
baseline:
ref: "main"BENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writer File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing claude/parquet-exact-value-windows-10538 (5af13c0) to main diff Run configurationrun benchmark arrow_writer
baseline:
ref: "main"BENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writer File an issue against this benchmark runner |
|
run benchmark arrow_writer writer_overhead baseline: |
2 similar comments
|
run benchmark arrow_writer writer_overhead baseline: |
|
run benchmark arrow_writer writer_overhead baseline: |
|
run benchmark writer_overhead baseline: |
2 similar comments
|
run benchmark writer_overhead baseline: |
|
run benchmark writer_overhead baseline: |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing claude/parquet-exact-value-windows-10538 (5af13c0) to fd806be diff Run configurationrun benchmark arrow_writer
baseline:
ref: "fd806be5d3c0ac522fd9e3e0b3dd70b08e527dd8"BENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writer File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing claude/parquet-exact-value-windows-10538 (5af13c0) to fd806be diff Run configurationrun benchmark writer_overhead
baseline:
ref: "fd806be5d3c0ac522fd9e3e0b3dd70b08e527dd8"BENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench writer_overhead File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing claude/parquet-exact-value-windows-10538 (5af13c0) to fd806be diff Run configurationrun benchmark writer_overhead
baseline:
ref: "fd806be5d3c0ac522fd9e3e0b3dd70b08e527dd8"CPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing claude/parquet-exact-value-windows-10538 (5af13c0) to fd806be diff Run configurationrun benchmark arrow_writer
baseline:
ref: "fd806be5d3c0ac522fd9e3e0b3dd70b08e527dd8"BENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writer File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing claude/parquet-exact-value-windows-10538 (5af13c0) to fd806be diff Run configurationrun benchmark writer_overhead
baseline:
ref: "fd806be5d3c0ac522fd9e3e0b3dd70b08e527dd8"BENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench writer_overhead File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing claude/parquet-exact-value-windows-10538 (5af13c0) to fd806be diff Run configurationrun benchmark arrow_writer
baseline:
ref: "fd806be5d3c0ac522fd9e3e0b3dd70b08e527dd8"BENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writer File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing claude/parquet-exact-value-windows-10538 (5af13c0) to main diff Run configurationrun benchmark writer_overhead
baseline:
ref: "main"BENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench writer_overhead File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing claude/parquet-exact-value-windows-10538 (5af13c0) to fd806be diff Run configurationrun benchmark writer_overhead
baseline:
ref: "fd806be5d3c0ac522fd9e3e0b3dd70b08e527dd8"BENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench writer_overhead File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing claude/parquet-exact-value-windows-10538 (5af13c0) to fd806be diff Run configurationrun benchmark writer_overhead
baseline:
ref: "fd806be5d3c0ac522fd9e3e0b3dd70b08e527dd8"CPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing claude/parquet-exact-value-windows-10538 (5af13c0) to main diff Run configurationrun benchmark writer_overhead
baseline:
ref: "main"CPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
run benchmark arrow_writer env: |
1 similar comment
|
run benchmark arrow_writer env: |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing claude/parquet-exact-value-windows-10538 (a2e0d90) to 0a8fdd5 diff Run configurationrun benchmark writer_page_windows
baseline:
ref: "0a8fdd505bbf1b551a7c0654f879b4bde7e41f18"BENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench writer_page_windows File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing claude/parquet-exact-value-windows-10538 (a2e0d90) to 0a8fdd5 diff Run configurationrun benchmark writer_page_windows
baseline:
ref: "0a8fdd505bbf1b551a7c0654f879b4bde7e41f18"BENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench writer_page_windows File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing claude/parquet-exact-value-windows-10538 (a2e0d90) to 0a8fdd5 diff Run configurationrun benchmark arrow_writer
env:
BENCH_FILTER: "^(small|medium|large)_string_(shared_prefix|partial_prefix|distinct)"
baseline:
ref: "0a8fdd505bbf1b551a7c0654f879b4bde7e41f18"BENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writer File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing claude/parquet-exact-value-windows-10538 (a2e0d90) to 0a8fdd5 diff Run configurationrun benchmark writer_page_windows
baseline:
ref: "0a8fdd505bbf1b551a7c0654f879b4bde7e41f18"BENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench writer_page_windows File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing 0a8fdd5 (0a8fdd5) to 0a8fdd5 diff Run configurationrun benchmark writer_page_windows
baseline:
ref: "0a8fdd505bbf1b551a7c0654f879b4bde7e41f18"
changed:
ref: "0a8fdd505bbf1b551a7c0654f879b4bde7e41f18"BENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench writer_page_windows File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing 0a8fdd5 (0a8fdd5) to 0a8fdd5 diff Run configurationrun benchmark writer_page_windows
baseline:
ref: "0a8fdd505bbf1b551a7c0654f879b4bde7e41f18"
changed:
ref: "0a8fdd505bbf1b551a7c0654f879b4bde7e41f18"BENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench writer_page_windows File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing 0a8fdd5 (0a8fdd5) to 0a8fdd5 diff Run configurationrun benchmark arrow_writer
env:
BENCH_FILTER: "^(small|medium|large)_string_(shared_prefix|partial_prefix|distinct)"
baseline:
ref: "0a8fdd505bbf1b551a7c0654f879b4bde7e41f18"
changed:
ref: "0a8fdd505bbf1b551a7c0654f879b4bde7e41f18"BENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writer File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing claude/parquet-exact-value-windows-10538 (a2e0d90) to 0a8fdd5 diff Run configurationrun benchmark writer_page_windows
baseline:
ref: "0a8fdd505bbf1b551a7c0654f879b4bde7e41f18"CPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing claude/parquet-exact-value-windows-10538 (a2e0d90) to 0a8fdd5 diff Run configurationrun benchmark writer_page_windows
baseline:
ref: "0a8fdd505bbf1b551a7c0654f879b4bde7e41f18"CPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing 0a8fdd5 (0a8fdd5) to 0a8fdd5 diff Run configurationrun benchmark writer_page_windows
baseline:
ref: "0a8fdd505bbf1b551a7c0654f879b4bde7e41f18"
changed:
ref: "0a8fdd505bbf1b551a7c0654f879b4bde7e41f18"CPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing claude/parquet-exact-value-windows-10538 (a2e0d90) to 0a8fdd5 diff Run configurationrun benchmark writer_page_windows
baseline:
ref: "0a8fdd505bbf1b551a7c0654f879b4bde7e41f18"CPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing 0a8fdd5 (0a8fdd5) to 0a8fdd5 diff Run configurationrun benchmark writer_page_windows
baseline:
ref: "0a8fdd505bbf1b551a7c0654f879b4bde7e41f18"
changed:
ref: "0a8fdd505bbf1b551a7c0654f879b4bde7e41f18"CPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing claude/parquet-exact-value-windows-10538 (a2e0d90) to 0a8fdd5 diff Run configurationrun benchmark arrow_writer
env:
BENCH_FILTER: "^(small|medium|large)_string_(shared_prefix|partial_prefix|distinct)"
baseline:
ref: "0a8fdd505bbf1b551a7c0654f879b4bde7e41f18"BENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writer File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing claude/parquet-exact-value-windows-10538 (a2e0d90) to 0a8fdd5 diff Run configurationrun benchmark arrow_writer
env:
BENCH_FILTER: "^(small|medium|large)_string_(shared_prefix|partial_prefix|distinct)"
baseline:
ref: "0a8fdd505bbf1b551a7c0654f879b4bde7e41f18"BENCH_COMMAND=cargo bench --features=arrow,async,test_common,experimental,object_store --bench arrow_writer File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing claude/parquet-exact-value-windows-10538 (a2e0d90) to 0a8fdd5 diff Run configurationrun benchmark arrow_writer
env:
BENCH_FILTER: "^(small|medium|large)_string_(shared_prefix|partial_prefix|distinct)"
baseline:
ref: "0a8fdd505bbf1b551a7c0654f879b4bde7e41f18"CPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing 0a8fdd5 (0a8fdd5) to 0a8fdd5 diff Run configurationrun benchmark arrow_writer
env:
BENCH_FILTER: "^(small|medium|large)_string_(shared_prefix|partial_prefix|distinct)"
baseline:
ref: "0a8fdd505bbf1b551a7c0654f879b4bde7e41f18"
changed:
ref: "0a8fdd505bbf1b551a7c0654f879b4bde7e41f18"CPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing claude/parquet-exact-value-windows-10538 (a2e0d90) to 0a8fdd5 diff Run configurationrun benchmark arrow_writer
env:
BENCH_FILTER: "^(small|medium|large)_string_(shared_prefix|partial_prefix|distinct)"
baseline:
ref: "0a8fdd505bbf1b551a7c0654f879b4bde7e41f18"CPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: Comparing claude/parquet-exact-value-windows-10538 (a2e0d90) to 0a8fdd5 diff Run configurationrun benchmark arrow_writer
env:
BENCH_FILTER: "^(small|medium|large)_string_(shared_prefix|partial_prefix|distinct)"
baseline:
ref: "0a8fdd505bbf1b551a7c0654f879b4bde7e41f18"CPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
@alamb @etseidl could you take a look at this change? it's not without tradeoffs, but in general I think the tradeoffs are worth it and minor. performance review detailed in #10554 (comment) |
…ache#10745) - Follow-up to apache#10505 / apache#10554 (independent of both; apache#10554 is now stacked on top of this) ## What `compute_min_max` in `arrow_writer/byte_array.rs` copies both the minimum and the maximum of every mini-batch into fresh `ByteArray`s (`to_vec()`) before `encode` has checked whether either one beats the running page min/max. `write_gather` runs once per mini-batch, and a byte-budgeted mini-batch of large values can hold a single value — so every large value was being allocated and copied twice more, on top of the copies the encoder itself needs. This returns the borrowed extremes and copies only when the running value actually changes. Statistics are unchanged; the comparison is the same byte-lexicographic order `ByteArray` uses. It only ever removes copies, so it cannot cost more anywhere. ## Why now Found while profiling apache#10554 (value-exact mini-batch windows), which showed **+12…15%** on `large_string_distinct_nullable/delta_byte_array` across three runner runs against both `main` and apache#10505. `samply` put the whole shift into `memmove` under `ByteArrayEncoder::write_gather`; the encoding work per value is identical, so the cost had to be per-mini-batch — and apache#10554 roughly doubles the mini-batch count on nullable columns. That was this copy. ## Measurements Runner (`c4a-highmem-16`), three runs each way with identical-code controls; full tables and links in the [results comment](apache#10745 (comment)). Isolated (stacked on apache#10554, vs apache#10554 head): | benchmark | mean of 3 | control | |---|---|---| | `large_string_shared_prefix_nullable/delta_byte_array` | **−24.4%** | +0.8 | | `large_string_shared_prefix_nullable_trailing/delta_byte_array` | **−23.2%** | −0.2 | | `large_string_shared_prefix_nullable_dense/delta_byte_array` | **−22.0%** | +0.7 | | `large_string_shared_prefix/delta_byte_array` (non-null) | **−21.9%** | −0.7 | | `medium_string_shared_prefix_nullable/delta_byte_array` | −7.2% | +4.9 | | `large_string_distinct_nullable/delta_byte_array` | −5.4% | −0.8 | | `large_string_distinct/delta_byte_array` | −4.0% | −0.2 | Effect on apache#10554's regression: `large_string_distinct_nullable/delta_byte_array` vs apache#10505 goes from **+14.7% to +5.4%**, and its `_nullable` shape from +18.3% to −13.6%. Alone, vs `main`: the over-limit `DELTA_BYTE_ARRAY` benches are flat — there every over-limit value already opens its own page (the bug apache#10505 fixes), so `flush_data_page` takes the running min/max on every mini-batch and both copies happen regardless. `large_string_non_null/*` (1024 × 256 KiB, default properties) is **−7…−9%** in all three full runs with a flat control, so it pays today wherever a page holds more than one mini-batch of large values. Nothing else moves outside what the main-vs-main control moves on identical code. ## Tests No behaviour change; full `parquet` suite green, `fmt` and `clippy -D warnings` clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…ache#10745) - Follow-up to apache#10505 / apache#10554 (independent of both; apache#10554 is now stacked on top of this) ## What `compute_min_max` in `arrow_writer/byte_array.rs` copies both the minimum and the maximum of every mini-batch into fresh `ByteArray`s (`to_vec()`) before `encode` has checked whether either one beats the running page min/max. `write_gather` runs once per mini-batch, and a byte-budgeted mini-batch of large values can hold a single value — so every large value was being allocated and copied twice more, on top of the copies the encoder itself needs. This returns the borrowed extremes and copies only when the running value actually changes. Statistics are unchanged; the comparison is the same byte-lexicographic order `ByteArray` uses. It only ever removes copies, so it cannot cost more anywhere. ## Why now Found while profiling apache#10554 (value-exact mini-batch windows), which showed **+12…15%** on `large_string_distinct_nullable/delta_byte_array` across three runner runs against both `main` and apache#10505. `samply` put the whole shift into `memmove` under `ByteArrayEncoder::write_gather`; the encoding work per value is identical, so the cost had to be per-mini-batch — and apache#10554 roughly doubles the mini-batch count on nullable columns. That was this copy. ## Measurements Runner (`c4a-highmem-16`), three runs each way with identical-code controls; full tables and links in the [results comment](apache#10745 (comment)). Isolated (stacked on apache#10554, vs apache#10554 head): | benchmark | mean of 3 | control | |---|---|---| | `large_string_shared_prefix_nullable/delta_byte_array` | **−24.4%** | +0.8 | | `large_string_shared_prefix_nullable_trailing/delta_byte_array` | **−23.2%** | −0.2 | | `large_string_shared_prefix_nullable_dense/delta_byte_array` | **−22.0%** | +0.7 | | `large_string_shared_prefix/delta_byte_array` (non-null) | **−21.9%** | −0.7 | | `medium_string_shared_prefix_nullable/delta_byte_array` | −7.2% | +4.9 | | `large_string_distinct_nullable/delta_byte_array` | −5.4% | −0.8 | | `large_string_distinct/delta_byte_array` | −4.0% | −0.2 | Effect on apache#10554's regression: `large_string_distinct_nullable/delta_byte_array` vs apache#10505 goes from **+14.7% to +5.4%**, and its `_nullable` shape from +18.3% to −13.6%. Alone, vs `main`: the over-limit `DELTA_BYTE_ARRAY` benches are flat — there every over-limit value already opens its own page (the bug apache#10505 fixes), so `flush_data_page` takes the running min/max on every mini-batch and both copies happen regardless. `large_string_non_null/*` (1024 × 256 KiB, default properties) is **−7…−9%** in all three full runs with a flat control, so it pays today wherever a page holds more than one mini-batch of large values. Nothing else moves outside what the main-vs-main control moves on identical code. ## Tests No behaviour change; full `parquet` suite green, `fmt` and `clippy -D warnings` clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…ctionary fallback (apache#10836) Adds writer benchmarks for three shapes that drive **mini-batch windowing** and that `arrow_writer` does not reach. When a chunk's values do not all fit in the page byte budget, the column writer splits the chunk into mini-batches. On a nullable column, how wide those windows are depends on the ratio between the value size and `data_page_size_limit` — and the existing suite only samples the two ends of that range. ## What is uncovered today | | `arrow_writer` | gap | | --- | --- | --- | | value size | ~1 KiB (`small_string_*`) or 2 MiB against the 1 MiB default (`large_string_*`) | nothing in between, where *several* values share a page budget | | `data_page_size_limit` | always the 1 MiB default | a smaller limit reaches a one-value window with far less encoding work per value | | how DBA is reached | always pinned via `set_dictionary_enabled(false)` | dictionary spilling into `DELTA_BYTE_ARRAY`, which is how byte-array columns are actually written | ## What is added * **`subpage_*`** — 128 KiB and 512 KiB values against the 1 MiB default. The existing case at this size, `medium_string_shared_prefix_nullable`, is shared-prefix only; the *distinct* case, where deduplication saves nothing and a narrower window is not paid back, is uncovered. A shared-prefix counterpart at 128 KiB varies only the prefix, so a movement present in one and absent in the other is attributable to that. * **`small_page_limit_*`** — 64 KiB values against a 64 KiB `data_page_size_limit`, a realistic setting for selective reads. * **`dictionary_fallback_*`** — a column that starts dictionary-encoded and becomes `DELTA_BYTE_ARRAY` only when the dictionary spills, so the windowing changes part-way through the column. `pinned_delta_byte_array` writes the same data with the dictionary disabled, isolating the dictionary phase and the transition. `plain` variants accompany the `delta_byte_array` ones throughout: `PLAIN` does not compress a value against its predecessor, so it is insensitive to where a page boundary falls and acts as the control for whether a movement is windowing or the machine. ## Notes New `writer_page_windows` bench target rather than adding to `arrow_writer` — that suite is already long enough that a full run has to be split across jobs, and these are heavy (64 MiB written per iteration, sized so times are comparable across value sizes). Benchmarks only; no library code is touched. Motivated by apache#10538 / apache#10554, where these shapes came up as unmeasured. Landing them separately means both sides of that comparison have them, so the change can be measured on the shapes it actually affects. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
| /// | ||
| /// [`write_granular_chunk`]: super::GenericColumnWriter::write_granular_chunk | ||
| #[derive(Debug, Clone, Copy, PartialEq, Eq)] | ||
| pub(crate) enum SubBatch { |
There was a problem hiding this comment.
Maybe something like SubBatchStrategy would be a better name 🤷
| /// [`write_granular_chunk`]: super::GenericColumnWriter::write_granular_chunk | ||
| #[derive(Debug, Clone, Copy, PartialEq, Eq)] | ||
| pub(crate) enum SubBatch { | ||
| /// Cut after exactly this many values, walking definition levels to find |
There was a problem hiding this comment.
Maybe summarize a bit more here, it's getting pretty long. Maybe some can be replaced with a link to the issue and PRs
…ache#10745) - Follow-up to apache#10505 / apache#10554 (independent of both; apache#10554 is now stacked on top of this) ## What `compute_min_max` in `arrow_writer/byte_array.rs` copies both the minimum and the maximum of every mini-batch into fresh `ByteArray`s (`to_vec()`) before `encode` has checked whether either one beats the running page min/max. `write_gather` runs once per mini-batch, and a byte-budgeted mini-batch of large values can hold a single value — so every large value was being allocated and copied twice more, on top of the copies the encoder itself needs. This returns the borrowed extremes and copies only when the running value actually changes. Statistics are unchanged; the comparison is the same byte-lexicographic order `ByteArray` uses. It only ever removes copies, so it cannot cost more anywhere. ## Why now Found while profiling apache#10554 (value-exact mini-batch windows), which showed **+12…15%** on `large_string_distinct_nullable/delta_byte_array` across three runner runs against both `main` and apache#10505. `samply` put the whole shift into `memmove` under `ByteArrayEncoder::write_gather`; the encoding work per value is identical, so the cost had to be per-mini-batch — and apache#10554 roughly doubles the mini-batch count on nullable columns. That was this copy. ## Measurements Runner (`c4a-highmem-16`), three runs each way with identical-code controls; full tables and links in the [results comment](apache#10745 (comment)). Isolated (stacked on apache#10554, vs apache#10554 head): | benchmark | mean of 3 | control | |---|---|---| | `large_string_shared_prefix_nullable/delta_byte_array` | **−24.4%** | +0.8 | | `large_string_shared_prefix_nullable_trailing/delta_byte_array` | **−23.2%** | −0.2 | | `large_string_shared_prefix_nullable_dense/delta_byte_array` | **−22.0%** | +0.7 | | `large_string_shared_prefix/delta_byte_array` (non-null) | **−21.9%** | −0.7 | | `medium_string_shared_prefix_nullable/delta_byte_array` | −7.2% | +4.9 | | `large_string_distinct_nullable/delta_byte_array` | −5.4% | −0.8 | | `large_string_distinct/delta_byte_array` | −4.0% | −0.2 | Effect on apache#10554's regression: `large_string_distinct_nullable/delta_byte_array` vs apache#10505 goes from **+14.7% to +5.4%**, and its `_nullable` shape from +18.3% to −13.6%. Alone, vs `main`: the over-limit `DELTA_BYTE_ARRAY` benches are flat — there every over-limit value already opens its own page (the bug apache#10505 fixes), so `flush_data_page` takes the running min/max on every mini-batch and both copies happen regardless. `large_string_non_null/*` (1024 × 256 KiB, default properties) is **−7…−9%** in all three full runs with a flat control, so it pays today wherever a page holds more than one mini-batch of large values. Nothing else moves outside what the main-vs-main control moves on identical code. ## Tests No behaviour change; full `parquet` suite green, `fmt` and `clippy -D warnings` clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…ctionary fallback (apache#10836) Adds writer benchmarks for three shapes that drive **mini-batch windowing** and that `arrow_writer` does not reach. When a chunk's values do not all fit in the page byte budget, the column writer splits the chunk into mini-batches. On a nullable column, how wide those windows are depends on the ratio between the value size and `data_page_size_limit` — and the existing suite only samples the two ends of that range. ## What is uncovered today | | `arrow_writer` | gap | | --- | --- | --- | | value size | ~1 KiB (`small_string_*`) or 2 MiB against the 1 MiB default (`large_string_*`) | nothing in between, where *several* values share a page budget | | `data_page_size_limit` | always the 1 MiB default | a smaller limit reaches a one-value window with far less encoding work per value | | how DBA is reached | always pinned via `set_dictionary_enabled(false)` | dictionary spilling into `DELTA_BYTE_ARRAY`, which is how byte-array columns are actually written | ## What is added * **`subpage_*`** — 128 KiB and 512 KiB values against the 1 MiB default. The existing case at this size, `medium_string_shared_prefix_nullable`, is shared-prefix only; the *distinct* case, where deduplication saves nothing and a narrower window is not paid back, is uncovered. A shared-prefix counterpart at 128 KiB varies only the prefix, so a movement present in one and absent in the other is attributable to that. * **`small_page_limit_*`** — 64 KiB values against a 64 KiB `data_page_size_limit`, a realistic setting for selective reads. * **`dictionary_fallback_*`** — a column that starts dictionary-encoded and becomes `DELTA_BYTE_ARRAY` only when the dictionary spills, so the windowing changes part-way through the column. `pinned_delta_byte_array` writes the same data with the dictionary disabled, isolating the dictionary phase and the transition. `plain` variants accompany the `delta_byte_array` ones throughout: `PLAIN` does not compress a value against its predecessor, so it is insensitive to where a page boundary falls and acts as the control for whether a movement is windowing or the machine. ## Notes New `writer_page_windows` bench target rather than adding to `arrow_writer` — that suite is already long enough that a full run has to be split across jobs, and these are heavy (64 MiB written per iteration, sized so times are comparable across value sizes). Benchmarks only; no library code is touched. Motivated by apache#10538 / apache#10554, where these shapes came up as unmeasured. Landing them separately means both sides of that comparison have them, so the change can be measured on the shapes it actually affects. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Review feedback on apache#10554: the enum names a windowing strategy, not a sub-batch, and its doc comment had grown long enough to be a wall of prose. Keep the reasoning that a reader needs at the call site and hand the measurements off to links. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017qiKn9F4vJR2xz8JuK645F
|
Thanks for the review @etseidl! From my perspective this is good to merge :) |
Important
Stacked on #10505 and #10745 — do not merge first. The commits below the top one belong to those PRs.
Review only this PR's own commit:
cff2ca07fe...bd66b9ed68.Once #10505 and #10745 merge I'll rebase and this PR's diff becomes clean on its own.
The problem
byte_budget_sub_batch_sizeasks the encoder how many values fit in a page byte budget, then converts that to a level count using the chunk-wide level:value ratio, rounded up:For a chunk with no nulls this is exact. With one null in 17 levels it gives
ceil(17/16) == 2, andwrite_granular_chunkslices the chunk into uniform two-level windows — most of which carry two values, i.e. twice what the budget allowed. The mechanism predates #10505; it dates to #9972.Where the encoding compresses a value against its predecessor, that round-up costs whole values of output. 128 values of 2 MiB at one null in 16:
At 8 MiB values it is 64 MiB against 8 MiB, and the acceptance case in #10538 — 16 identical 64 KiB values with one null — goes from five pages storing ~5 values in full to one page storing ~1.
The fix
Have the chunker return the value count it already computed, and let
write_granular_chunkend a window by walking definition levels until it has covered that many values. No ratio, no rounding.Then apply it only where it changes the bytes written, because it is not free — value-exact windows roughly double the mini-batch count on a nullable column. Two conditions, both required:
The budget must be the data page budget. That one is a constant
data_page_size_limit, so a one-value budget means the value itself overflows a page. The dictionary page budget is the limit minus what the dictionary already holds, so it shrinks toward zero as the dictionary fills and reaches a one-value budget on perfectly ordinary values; cutting exactly there measured +13.0% onstring/defaultand +8.3% onstring/parquet_2.The encoding must compress against the previous value.
PLAINandDELTA_LENGTH_BYTE_ARRAYstore a value identically wherever it lands, so value-exact windows leave their output byte for byte the same while doubling the page count — measured +27.6% on a nullable column for no reduction in output at all. Thecompresses_against_previous_valueflag #10505 added already marks exactly the right set.What the other paths give up
A ratio-scaled window spans
ceil(values × levels / values_in_chunk)levels. Where one value already fills the budget that covers at most two values, whatever the null density, and exactly one wherever the ratio is a whole number. Measured against a 1 MiB limit:PLAINPLAINPLAINPLAINThe floor is what a page must hold: one value. So the concession is a factor of two above an unavoidable minimum, it does not vary with null density, and — the property #9972 exists for — it does not scale with
write_batch_size. Before #9972 a page took a whole mini-batch: 1024 × 2 MiB, roughly 2000× the limit.Measurements
Base is #10505's head (measured at
fd806be5d3; both branches have since been rebased ontomain, with the trees verified identical across the rebase), so these isolate this PR. Local, run base → branch → base on an idle machine, with the two base passes as a per-benchmark noise floor. Benchmarks are the ones added in #10561...._nullable/plain..._nullable_trailing/delta_byte_array..._nullable/delta_byte_array..._nullable_dense/delta_byte_arraylarge_string_distinct_nullable/delta_byte_arraymedium_string_shared_prefix_nullable/delta_byte_arrayTwo costs remain, both on
DELTA_BYTE_ARRAYwhere the byte win does not materialise:Verified byte-identical output — same length, same hash — between base and this PR for dictionary-encoded nullable columns (four shapes) and for repeated columns (both encodings), confirming those paths are untouched rather than merely unchanged in aggregate.
Scope
Repeated columns are unchanged: records cannot span pages, so a record holding several over-limit values still exceeds the budget. That is inherent to the format.
Tests
test_column_writer_delta_byte_array_nullable_shared_prefix_dedup— re-pinned from[2, 2, 2, 2, 9]to[17]and renamed, the layout fix(parquet): keep DELTA_BYTE_ARRAY dedup for values larger than the page size limit #10505 left a marker for.test_column_writer_caps_page_size_with_sparse_nulls— pins two values per page underPLAIN, so it fails both if the encoding gate is dropped (pages would hold one) and if the bound is lost (they would hold many).Full
parquetsuite green (1312 lib + integration),fmtandclippy -D warningsclean.Note on #10505
Its
bool_to_int_with_iftripscargo clippy -- -D warnings, which CI runs — worth fixing on that branch too. Corrected here as part of editing that test.🤖 Generated with Claude Code