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

   # Which issue does this PR close?
   
   Revisits the problem reported in #5250 (closed without merging).
   
   # Rationale for this change
   
   `OffsetBuffer::extend_from_dictionary` reserves `offsets` but not `values`, 
so every gathered dictionary value lands in an unreserved `Vec<u8>` and 
amortized doubling re-copies roughly all gathered data one extra time. For 
**large dictionary values** the extra copy dominates the decode. This regime is 
common in practice: fat binary columns (e.g. images) written by 
parquet-cpp/PyArrow end up ~100% dictionary-encoded because the writer checks 
its dictionary-size limit lazily, so a page can carry a handful of 100 KiB+ 
values behind dictionary keys.
   
   #5250 proposed an **exact-sum** reserve (a pass over the keys summing each 
value's length) and was withdrawn after it measured +10–15% on the crate's 
small-string dictionary benchmarks — the second bounds-checked pass over the 
keys costs more than it saves when values are ~10 bytes. We reproduced that 
rejection with an exact-sum patch on 59.1.0: +8–18% across the three dictionary 
`StringArray` cases.
   
   This PR uses an **O(1) estimate** instead: `keys.len() × (dict_values.len() 
/ dict_entry_count)`. No per-key pass, exact when value lengths are uniform 
(the fat-value case), and an ordinary reservation hint when they are skewed — a 
skewed distribution still grows or over-reserves by at most one doubling's 
worth.
   
   Measurements (Apple M-series, criterion; treat as directional — we'd value a 
run of your benches on your reference hardware):
   
   - Fat values (256 KiB each, dictionary-encoded, snappy): decode CPU drops 
~40% stock→patched; output verified byte-identical at 256 KiB / 4 KiB / 64 B 
values, dict and non-dict.
   - Non-dictionary control: unchanged.
   - Small-string dictionary benches (`arrow_reader`): parity within our noise 
floor — unlike the exact-sum variant, which reproducibly regresses them.
   
   # What changes are included in this PR?
   
   A single reservation hint (14 lines) at the top of `extend_from_dictionary` 
in `parquet/src/arrow/buffer/offset_buffer.rs`. No behavior change: on 
`dict_len == 0` it is skipped, and the estimate only pre-sizes the same `Vec` 
the loop was already growing.
   
   # Are there any user-facing changes?
   
   No.
   


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