sunchao commented on code in PR #6430:
URL: https://github.com/apache/datafusion-comet/pull/6430#discussion_r4353103390
##########
native/shuffle/benches/shuffle_writer.rs:
##########
@@ -191,6 +194,72 @@ fn criterion_benchmark(c: &mut Criterion) {
/// Times one execution of a freshly built writer per iteration. A
`ShuffleWriterExec`
/// publishes its partition offsets once, so it cannot be re-executed;
building it is
/// setup and stays outside the measurement.
+// Include decimal SUM's non-null is_empty state, nullable Boolean-heavy rows,
+// and an Int64-only control. Both low and high partition counts matter because
+// short runs are more common with fewer hash partitions.
+fn boolean_state_benchmark(c: &mut Criterion) {
+ let mut group = c.benchmark_group("shuffle_writer_boolean_state");
+ group.sample_size(10);
+ for shape in ["decimal_sum", "nullable_booleans", "int64_control"] {
+ for partitions in [4, 200] {
+ for key_run in [1, 64] {
+ let schema_batch = boolean_state_batch(8192, shape, key_run);
+ group.bench_function(
+
format!("{shape}/partitions={partitions}/key_run={key_run}"),
+ |b| {
+ bench_end_to_end(b, || {
+ let batches = vec![schema_batch.clone(); 100];
Review Comment:
Fixed in ddeccb4ec. `boolean_state_batch` now takes the batch index and
offsets keys, values and null patterns by `batch_index * rows`. Each case
builds 100 separate batches outside the timed execution, then clones that
vector when constructing each writer. The 64-row key runs stay intact.
The complete benchmark compiled, and all 12 Boolean-state writer cases
passed in Criterion test mode. A temporary harness using the exact fixture
helper with Arrow 59.3.0 verified independent buffers, disjoint key ranges, run
lengths, values/nulls and decimal `is_empty` consistency across 600 batches
(4,915,200 rows). No new performance timings are claimed.
##########
native/shuffle/benches/shuffle_writer.rs:
##########
@@ -191,6 +194,72 @@ fn criterion_benchmark(c: &mut Criterion) {
/// Times one execution of a freshly built writer per iteration. A
`ShuffleWriterExec`
/// publishes its partition offsets once, so it cannot be re-executed;
building it is
/// setup and stays outside the measurement.
+// Include decimal SUM's non-null is_empty state, nullable Boolean-heavy rows,
+// and an Int64-only control. Both low and high partition counts matter because
+// short runs are more common with fewer hash partitions.
+fn boolean_state_benchmark(c: &mut Criterion) {
Review Comment:
Fixed in ddeccb4ec. Both Boolean benchmark helpers now precede the doc
block, which is directly attached to `bench_end_to_end` again. Rust formatting
and benchmark compilation pass.
--
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]