Abhisheklearn12 commented on PR #9768:
URL: https://github.com/apache/arrow-rs/pull/9768#issuecomment-5080218327
sorry for going quiet @Jefffrey, got busy and never pushed the changes.
did come back to your comments though you were right to ask for a benchmark
on that arm, it's actually a big regression. turns out `unpack_dictionary`
never copied the values buffer at all: `From<&GenericByteArray>` reuses the
buffer and `take_byte_view` passes `data_buffers()` straight through, so there
was nothing to optimise away. what the direct path really trades is a
bandwidth-bound view gather for a per-row `make_view` that reads each row's
payload out of the buffer so it only wins while the dictionary is bigger than
the array is long. crossover measured around `rows ≈ 0.6 * values`, and past
that it gets bad fast: 5-8x slower at typical batch shapes, including the `Utf8
-> Utf8View` and `Binary -> BinaryView` arms already on main.
on the `u32::MAX` check without it the direct path panics rather than
failing gracefully (`append_block` asserts), and unpack handles oversized
buffers fine by falling back to `from_iter`.
closing this, will open a fresh one that gates the fast path on cardinality
and adds a benchmark. thanks for the review 🙏
--
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]
