adriangb commented on PR #10554:
URL: https://github.com/apache/arrow-rs/pull/10554#issuecomment-5404837593

   ### Full benchmark run after rebase onto `main` (with #10505 and #10745 
landed), with controls
   
   Rebased onto `main` (`3577093f`); #10505 dropped as merged, so this PR is 
now the single commit `c7591d17` and the numbers below isolate it against a 
base that **already contains both #10505 and #10745**. That is the first time 
this change has been measured on its own.
   
   20 runner jobs on `c4a-highmem-16`, everything against merge-base 
`3577093f26591aee806a9133c891b6b8e5255477`, ×3 per configuration plus a 
main-vs-main control of each:
   
   - **Filtered** to the DBA groups (`BENCH_FILTER: 
^(small|medium|large)_string_(shared_prefix|partial_prefix|distinct)`): 
[1](https://github.com/apache/arrow-rs/pull/10554#issuecomment-5402234308) 
[2](https://github.com/apache/arrow-rs/pull/10554#issuecomment-5402234521) 
[3](https://github.com/apache/arrow-rs/pull/10554#issuecomment-5402235638) · 
controls 
[1](https://github.com/apache/arrow-rs/pull/10554#issuecomment-5402239355) 
[2](https://github.com/apache/arrow-rs/pull/10554#issuecomment-5402238231)
   - **Full `arrow_writer`, half A** (`^[a-l]`): 
[1](https://github.com/apache/arrow-rs/pull/10554#issuecomment-5404810586) 
[2](https://github.com/apache/arrow-rs/pull/10554#issuecomment-5404823372) 
[3](https://github.com/apache/arrow-rs/pull/10554#issuecomment-5404818498) · 
control 
[1](https://github.com/apache/arrow-rs/pull/10554#issuecomment-5404808696)
   - **Full `arrow_writer`, half B** (`^[m-z]`): 
[1](https://github.com/apache/arrow-rs/pull/10554#issuecomment-5404777428) 
[2](https://github.com/apache/arrow-rs/pull/10554#issuecomment-5404770389) 
[3](https://github.com/apache/arrow-rs/pull/10554#issuecomment-5404770972) · 
control 
[1](https://github.com/apache/arrow-rs/pull/10554#issuecomment-5404772075)
   - **`writer_overhead`**: 
[1](https://github.com/apache/arrow-rs/pull/10554#issuecomment-5402286627) 
[2](https://github.com/apache/arrow-rs/pull/10554#issuecomment-5402282501) 
[3](https://github.com/apache/arrow-rs/pull/10554#issuecomment-5402335158) · 
control 
[1](https://github.com/apache/arrow-rs/pull/10554#issuecomment-5402325059)
   
   The full suite had to be split in half: it now runs ~8700s against the 
runner's 7200s job deadline, and four unsplit attempts timed out (forcing thin 
LTO didn't help — the cost is benchmark runtime, not build). The split is 
`^[a-l]`/`^[m-z]` on the group name rather than an explicit group list, so it 
stays exhaustive: the suite is 241 benchmarks / 43 groups today against 209 in 
the August run, and an enumerated filter would have silently dropped the 32 new 
ones.
   
   #### What actually moves
   
   Two independent job configurations, six PR runs each way, four controls:
   
   | benchmark | filtered (×3) | mean | half A (×3) | mean | ctrl |
   | --- | --- | --- | --- | --- | --- |
   | `large_string_shared_prefix_nullable_trailing/delta_byte_array` | −40.8 / 
−40.4 / −40.8 | **−40.7%** | −39.5 / −44.4 / −38.7 | **−40.8%** | +0.6 / −0.4 / 
+2.1 |
   | `large_string_shared_prefix_nullable/delta_byte_array` | −6.8 / −15.4 / 
−6.5 | **−9.5%** | −4.9 / −8.1 / −1.9 | **−5.0%** | +0.2 / −0.7 / +1.9 |
   | `large_string_distinct_nullable/delta_byte_array` | +4.6 / +8.3 / +5.0 | 
**+6.0%** | +6.2 / +2.2 / +12.1 | **+6.8%** | +0.3 / −0.2 / +3.1 |
   | `large_string_shared_prefix_nullable_dense/delta_byte_array` | −0.9 / 
+11.5 / −2.3 | +2.8% | +0.5 / −4.8 / +3.2 | −0.4% | +0.9 / +0.0 / +1.4 |
   | `large_string_shared_prefix_list/delta_byte_array` | −3.3 / −4.6 / −2.0 | 
−3.3% | +0.4 / −2.4 / +1.8 | −0.1% | +0.7 / −0.5 / +2.8 |
   | `medium_string_shared_prefix_nullable/delta_byte_array` | −1.4 / −1.7 / 
−2.0 | −1.7% | (half B) −12.0 / −1.8 / +1.0 | −4.3% | −2.3 / −3.7 / −1.8 |
   
   - **`_trailing` is the win this PR exists for**: −40% in all six runs, 
spread under 6 points, against controls of ±2%. On top of the file going 120 
MiB → 2 MiB. This is *better* than the −25% measured before #10745 landed.
   - **`_nullable` is a −5…−9% win**, negative in all six runs, for the 16.8 MB 
→ 2.1 MB size reduction. Magnitude is loose; sign is not.
   - **`distinct_nullable` is the one real regression**: positive in all six 
runs, ~+6–7%, no size reduction to show for it (the values share no prefix). 
**The PR description's +11.3% is stale** — it was measured before #10745, which 
removed the per-mini-batch min/max copy that caused most of it. It should be 
updated to ~+6%.
   - `_dense`, `_list` and `medium` disagree in sign between the two 
configurations and sit inside their controls — flat.
   - `writer_overhead` is flat: −0.8% / +0.0% / −1.0%, control +0.2…+2.7%. 
Notably `5000_cols` behaved this time, so the −18…−20% it reported in earlier 
runs was the bimodality, as suspected.
   
   #### The dictionary gate holds
   
   The reason the change is scoped to the data page budget was the +13.0% / 
+8.3% on `string/default` / `string/parquet_2`. The tight dictionary benches 
confirm it: `string_dictionary_low_cardinality_{20,100,400}` are flat across 
every codec (−1.5% … +0.0%, controls the same).
   
   #### What to ignore in the full-suite output
   
   Two categories, both non-findings:
   
   **Void — the identical-code control moves them ≥20%.** `string_ree/*` 
(control −39.6% on `parquet_2`), `string_dictionary/*` (control −31.4% on 
`default`), `string/{default,parquet_2}`, `string_non_null/*`. These are the 
bimodal benches on this runner; nothing can be read off them in either 
direction.
   
   **Artifacts — not reachable by this change.** Several primitive/struct 
groups move consistently across all three runs against a near-zero control: 
`struct_sparse_99pct_null/cdc` −10.6%, `struct_all_null/cdc` −9.9%, 
`primitive_sparse_99pct_null/cdc` −7.2%, `primitive_all_null/cdc` −3.2%. 
Three-run consistency would normally be convincing, but there is no code path: 
`compresses_against_previous_value()` defaults to `false` and is overridden 
true only for `DELTA_BYTE_ARRAY` with no active dictionary, and the dictionary 
branch in `byte_budget_sub_batch_size` hardcodes `value_exact = false`. 
Primitive, bool, struct and dictionary-active columns cannot reach 
`SubBatch::Values`. This is the baseline-first tilt this runner shows on 
untouched groups.
   
   **Bottom line:** one large intended win (`_trailing` −40%, 120 MiB → 2 MiB), 
one secondary win (`_nullable` −5…−9%, 8× smaller file), one real regression 
(`distinct_nullable` +6–7%, no size change, down from +11–15% now that #10745 
has landed), and nothing else in the 241-benchmark suite moving outside what 
identical code moves.
   


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