You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2025/06/24 12:35:35 UTC
[I] [coalesce] Special case `BatchCoalescer` / `GenericInProgressArray` when multiple batches are pushed in with the same buffer [arrow-rs]
alamb opened a new issue, #7765:
URL: https://github.com/apache/arrow-rs/issues/7765
**Is your feature request related to a problem or challenge? Please describe what you are trying to do.**
[`GenericInProgressArray`](https://github.com/apache/arrow-rs/blob/2b40d1dfc35862ff350a40dfbc66f8a14f4eea31/arrow-select/src/coalesce/generic.rs#L31) is almost entirely dominated by copying strings to new buffers. It copies to new buffers to avoid accumulating large numbers of buffers that each have only a small number of rows pointed at
It has several optimizations to avoid copying and optimizing this copy when possible
The coalesce kernel has special logic to recycle string view buffers when they are not used much (TODO link)
I have a as yet unproven thesis that we could speed up the coalesce kernel by special casing when the underlying buffer is the same.
The high level idea is that in the case of reading from Parquet the same string buffer will be used for several batches, so if the coalesce kernel detected this maybe we could avoid some copies. I intend to use the `coalesce` kernel to make parquet reading faster
**Describe the solution you'd like**
Make benchmarks kernel faster
**Describe alternatives you've considered**
The first thing I would do is check an actual parquet benchmark that the same `Buffer`s are used for multiple RecordBatches that come out of the reader:
```shell
cargo bench --features=arrow,async --bench arrow_reader_clickbench
```
If that is the case, then I would then make a benchmark that replicates the pattern (e.g. create a record batch with 32K rows, and then slice it up and send it in 8k row chunks)
Then I would try and optimize it. For example check pointer equality and delay the string copies until it saw a new buffer pointer
**Additional context**
<!--
Add any other context or screenshots about the feature request here.
-->
--
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: github-unsubscribe@arrow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [coalesce] Special case `BatchCoalescer` / `GenericInProgressArray` when multiple batches are pushed in with the same buffer [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #7765:
URL: https://github.com/apache/arrow-rs/issues/7765#issuecomment-5798976476
thanks for considering it -- let's close this one then
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [coalesce] Special case `BatchCoalescer` / `GenericInProgressArray` when multiple batches are pushed in with the same buffer [arrow-rs]
Posted by "Rich-T-kid (via GitHub)" <gi...@apache.org>.
Rich-T-kid commented on issue #7765:
URL: https://github.com/apache/arrow-rs/issues/7765#issuecomment-5782560528
>maybe we could avoid any copies if all the slices originate from the same buffer 🤔
thought about this more and no we generally cant do this.
after an aggregation `emit::All()` is called and then slices of arrays are passed to repartionExec. these all share the same backing array but they become non-contiguous because of hashing. we would need to re-allocate to new buffers in this case. the only case where we could just keep the backing buffer is when stretches of the array remain together but any chunk that isn't aligned ruins this.
for example
```
backing buffer : [0-1000]
slices : [0 - 200] [200 - 500]
```
this would be fine to re-use the backing buffer -> [0-500]
but
```
slices : [0-200] [201-500]
```
is not okay to re-use.
being able to avoid a O(N) copy can be huge though, I may be missing cases when we can avoid this work
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [coalesce] Special case `BatchCoalescer` / `GenericInProgressArray` when multiple batches are pushed in with the same buffer [arrow-rs]
Posted by "Rich-T-kid (via GitHub)" <gi...@apache.org>.
Rich-T-kid commented on issue #7765:
URL: https://github.com/apache/arrow-rs/issues/7765#issuecomment-5766564611
maybe we could avoid any copies if all the slices originate from the same buffer 🤔
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [coalesce] Special case `BatchCoalescer` / `GenericInProgressArray` when multiple batches are pushed in with the same buffer [arrow-rs]
Posted by "Rich-T-kid (via GitHub)" <gi...@apache.org>.
Rich-T-kid commented on issue #7765:
URL: https://github.com/apache/arrow-rs/issues/7765#issuecomment-5382150174
take
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [coalesce] Special case `BatchCoalescer` / `GenericInProgressArray` when multiple batches are pushed in with the same buffer [arrow-rs]
Posted by "Rich-T-kid (via GitHub)" <gi...@apache.org>.
Rich-T-kid commented on issue #7765:
URL: https://github.com/apache/arrow-rs/issues/7765#issuecomment-5382147959
was looking for related tickets after working on https://github.com/apache/arrow-rs/pull/10799 & it seems we had the same idea. 🤔 mabey the `byte_view` versions can use a similar optimization if its not there already.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [coalesce] Special case `BatchCoalescer` / `GenericInProgressArray` when multiple batches are pushed in with the same buffer [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb closed issue #7765: [coalesce] Special case `BatchCoalescer` / `GenericInProgressArray` when multiple batches are pushed in with the same buffer
URL: https://github.com/apache/arrow-rs/issues/7765
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org