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


##########
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.
+fn bench_take_n(c: &mut Criterion) {
+    let mut group = c.benchmark_group("dict_take_n");
+    let schema = dict_schema();
+    let null_density = 0.10;
+    let size = SIZES[1];
+
+    let mut cards = CARDS_RELATIVE.to_vec();
+    cards.push(size);
+    for cardinality in cards {
+        let batch = make_dict(size, cardinality, null_density, SEED);
+        group.throughput(Throughput::Elements((size * N_BATCHES) as u64));
+        group.bench_function(bench_id("take_n", size, cardinality, 
null_density), |b| {
+            b.iter_batched_ref(
+                || {
+                    (
+                        new_group_values(schema.clone(), 
&GroupOrdering::None).unwrap(),
+                        Vec::<usize>::with_capacity(size),
+                    )
+                },
+                |(gv, groups)| {
+                    for _ in 0..N_BATCHES {
+                        gv.intern(std::slice::from_ref(&batch), 
groups).unwrap();
+                        black_box(&*groups);
+                        let emit_n = (size / 2).min(gv.len());

Review Comment:
   that make alot of sense, thank you for pointing this out. updated the PR



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