sunchao commented on code in PR #6430:
URL: https://github.com/apache/datafusion-comet/pull/6430#discussion_r4178661396
##########
native/shuffle/src/partitioners/partitioned_batch_iterator.rs:
##########
@@ -239,6 +262,51 @@ impl Iterator for RowIterator<'_> {
}
}
+/// Gather scattered Boolean state without rebuilding Arrow's MutableArrayData
+/// descriptors for every reducer. Clustered selections retain Arrow's range
copies.
+/// The producer supplies matching immutable batches and valid row indices,
just as
+/// for the ordinary Arrow interleave path.
Review Comment:
Filed apache/arrow-rs#11379 and linked it from the helper’s doc comment in
137d3c0. The issue covers recursive/nested Boolean use, sliced offsets, nulls,
bounds/error behavior, and measuring clustered selections before adopting the
specialization. I checked arrow-rs main’s dispatch while filing it: Boolean
still takes the MutableArrayData fallback.
##########
native/shuffle/src/partitioners/partitioned_batch_iterator.rs:
##########
@@ -239,6 +262,51 @@ impl Iterator for RowIterator<'_> {
}
}
+/// Gather scattered Boolean state without rebuilding Arrow's MutableArrayData
+/// descriptors for every reducer. Clustered selections retain Arrow's range
copies.
+/// The producer supplies matching immutable batches and valid row indices,
just as
+/// for the ordinary Arrow interleave path.
+fn interleave_shuffle_batches(
+ batches: &[&RecordBatch],
+ indices: &[(usize, usize)],
+) -> Result<RecordBatch, ArrowError> {
+ let schema = batches[0].schema_ref();
+ // Arrow already copies clustered bitmap ranges efficiently. A false
positive only
+ // selects that existing path; the probe is not used to copy or validate
indices.
+ // A four-row stride detects eight-row runs regardless of their starting
alignment.
+ let clustered = (0..indices.len().saturating_sub(4)).step_by(4).any(|i| {
+ indices[i].0 == indices[i + 4].0 && indices[i].1.checked_add(4) ==
Some(indices[i + 4].1)
+ });
Review Comment:
Raised the probe stride and distance from 4 to 32 in 137d3c0, so short hash
runs retain the direct gather while every 64-row contiguous run is detected
regardless of alignment. Added a probe-only test that checks short-run
dispatch, all 32 alignments, empty/short selections and checked-add overflow.
It passes in a standalone execution of the exact helper/test; full crate
validation is pending.
Also added the requested writer benchmark matrix: decimal SUM state, eight
nullable Booleans and Int64 control, each at 4/200 partitions and
scattered/64-row-clustered keys, with 100 × 8,192 rows, LZ4 and no spill.
Current-head timings have not been collected, so the description removes the
old eight-row-probe timing table rather than attributing those results to this
revision.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
