neilconway commented on code in PR #24559:
URL: https://github.com/apache/datafusion/pull/24559#discussion_r3831074689


##########
datafusion/functions-aggregate/benches/first_last.rs:
##########
@@ -452,6 +452,103 @@ fn create_list_of_struct_array(n: usize, list_len: usize, 
null_density: f32) ->
     ))
 }
 
+/// Head-to-head for the coalesce-peers rewrite: N independent primitive
+/// `first_value` accumulators (the pre-rewrite plan) vs one struct-valued
+/// accumulator carrying the same N columns (the post-rewrite plan). Uses the
+/// same worst-case ordering as [`update_bench`] so every row forces an
+/// ordering comparison in every accumulator.
+#[expect(clippy::needless_pass_by_value)]
+fn coalesce_comparison_bench(
+    c: &mut Criterion,
+    name: &str,
+    column_values: Vec<ArrayRef>,
+    struct_values: ArrayRef,
+    ord: ArrayRef,
+    num_groups: usize,
+) {
+    let n = ord.len();
+    let group_indices: Vec<usize> = (0..n).map(|i| i % num_groups).collect();
+    let worst_ord: ArrayRef = Arc::new(Int64Array::from(vec![i64::MAX; n]));
+
+    // Pre-rewrite: one accumulator per column.
+    c.bench_function(&format!("{name} separate x{}", column_values.len()), |b| 
{
+        b.iter_batched(
+            || {
+                column_values
+                    .iter()
+                    .map(|values| {
+                        let mut acc = prepare_typed_groups_accumulator(
+                            true,
+                            values.data_type().clone(),
+                        );
+                        acc.update_batch(
+                            &[Arc::clone(values), Arc::clone(&worst_ord)],
+                            &group_indices,
+                            None,
+                            num_groups,
+                        )
+                        .unwrap();
+                        acc
+                    })
+                    .collect::<Vec<_>>()
+            },
+            |mut accumulators| {
+                for _ in 0..100 {
+                    for (acc, values) in 
accumulators.iter_mut().zip(&column_values) {
+                        #[expect(clippy::unit_arg)]
+                        black_box(
+                            acc.update_batch(
+                                &[Arc::clone(values), Arc::clone(&ord)],

Review Comment:
   Feeding the same `ord` array in to every iteration is a little unrealistic, 
no? That will measure comparison cost, but doesn't capture workloads where the 
winner changes over time and the aggregate's running value needs to be updated.



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

Reply via email to