Rich-T-kid commented on code in PR #24416:
URL: https://github.com/apache/datafusion/pull/24416#discussion_r3801022535


##########
datafusion/physical-plan/benches/dictionary_group_values.rs:
##########
@@ -172,5 +172,86 @@ fn bench_repeated_intern_emit(c: &mut Criterion) {
     group.finish();
 }
 
-criterion_group!(benches, bench_intern_emit, bench_repeated_intern_emit);
+// GroupOrdering::Full -> GroupValuesColumn::<true>: scalar 
append_val/equal_to path.
+fn bench_scalar_append_equal(c: &mut Criterion) {
+    let mut group = c.benchmark_group("dict_scalar_append_equal");
+    let schema = dict_schema();
+    let null_density = 0.1;
+    let size = SIZES[1];
+
+    let mut cards = CARDS_RELATIVE.to_vec();
+    cards.push(size);
+    for cardinality in cards {
+        let array = make_dict(size, cardinality, null_density, SEED);
+        group.throughput(Throughput::Elements(size as u64));
+        group.bench_function(
+            bench_id("scalar_append_equal", size, cardinality, null_density),
+            |b| {
+                b.iter_batched_ref(
+                    || {
+                        (
+                            new_group_values(
+                                schema.clone(),
+                                &GroupOrdering::Full(GroupOrderingFull::new()),
+                            )
+                            .unwrap(),
+                            Vec::<usize>::with_capacity(size),
+                        )
+                    },
+                    |(gv, groups)| {
+                        gv.intern(std::slice::from_ref(&array), 
groups).unwrap();
+                        black_box(&*groups);
+                        black_box(gv.emit(EmitTo::All).unwrap());
+                    },
+                    BatchSize::SmallInput,
+                );
+            },
+        );
+    }
+    group.finish();
+}
+
+// EmitTo::First exercises the take-n path; two interns + partial emit per 
iteration.

Review Comment:
   ah, i removed the double intern in 
https://github.com/apache/datafusion/pull/24416/changes/529295b9e6c3678582a428020bbb728e2853d3aa
 as I thought it overcomplicated the benchmarks since the same record batches 
would be passed in. Updated the PR description and doc comment to be accurate



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