Rich-T-kid commented on PR #11174:
URL: https://github.com/apache/arrow-rs/pull/11174#issuecomment-6224942383
## tldr; going to add a new method onto the batch Coacler
I thought about changing to this API (`push_batch_for_partition`, which
takes a shared partition_ids array plus a target partition). The rationale is
that we could save both the double row copy and the repeated index copies that
happen for each output partition. In practice, though, each coalescer has to
rescan all N ids to find its rows, so we’d be doing N*P work (~4M comparisons
per 8K batch at 512 partitions).
At the per-record-batch scale, the work we’d be saving is small by
comparison. The per-partition indices come to ~4 bytes per selected row (~32 KB
total for an 8K batch). If those buffers are reused across batches, there are
no repeated allocations and the whole set stays in cache. The double row copy
is the expensive part, and the `push_batch_with_indices` hook already lets
specialized impls skip it. The new API would trade a small cost for a
multiplier that grows with P.
with that in mind, I do think we need a different api because once we
convert the indices to a `dyn Array` we lose ownership of it, meaning we lose
the backing array. we should be generic over the indices size type and aim for
as small as possible units so we can make better use of CPU cache lines.
proposed api
```rust
push_batch_with_indices<T>(src: record_batch, indices: &[T]){}
```
~~~ indices can be Vec<u8>,...Vec<u64>. typically record batches will never
need to represent anything larger than a u16 so we can pack cpu cache lines
better, even more so for very sparse cases such as 64..512 output partitions.
we'll also avoid the 2x memory cost of the alternative which is to create
another vector of the same backing space before turning the original vector
into a `dyn array`~~~
... I dont think we can do this in rust as it would require the entire
struct to be generic.... a downcast should be fine
--
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]
