sunchao commented on code in PR #7200:
URL: https://github.com/apache/datafusion-comet/pull/7200#discussion_r4417632037
##########
native/shuffle/benches/shuffle_writer.rs:
##########
@@ -376,6 +376,182 @@ fn partitioning_benchmark(c: &mut Criterion) {
group.finish();
}
+/// The row-level gather on nested columns, through the same place+gather
harness as
+/// [`partitioning_benchmark`] but under hash partitioning, which is what most
shuffles use.
+///
+/// Hash placement scatters rows, so the flush has to gather every output
partition back out a
+/// row at a time with `interleave_record_batch`. For a primitive or string
column that is a copy
+/// of values; for a nested column it also has to rebuild list and map offsets
and recurse into
+/// every child array, and the fixtures above have no list or map columns at
all. The shapes
+/// measured here are `List<Int64>`, `List<Struct<Int64, Utf8>>`, `Map<Utf8,
Int64>` and a flat
+/// `Struct<Int64, Utf8, Float64>`. Each schema puts four columns of its shape
behind an Int64
+/// key, and the hash reads only the key, so the gap between `place` and
`place+gather` is the
+/// gather of the nested columns and not the hash of them. IPC encoding and
the file write stay
+/// out, as above. List and map lengths vary from row to row (0 to 8 and 1 to
7), so the gather
+/// cannot fall into a fixed-stride pattern. Every batch is built separately,
for the reasons
+/// given in [`partitioning_benchmark`].
+fn nested_gather_benchmark(c: &mut Criterion) {
+ const BATCH_SIZE: usize = 8192;
+ const NUM_BATCHES: usize = 8;
+ const NUM_PARTITIONS: usize = 50;
+
+ let partitioning =
+ CometPartitioning::Hash(vec![Arc::new(Column::new("key", 0))],
NUM_PARTITIONS);
+ let mut group = c.benchmark_group("shuffle_gather_nested");
+ for shape in [
+ NestedShape::ListInt64,
+ NestedShape::ListStruct,
+ NestedShape::MapUtf8Int64,
+ NestedShape::Struct,
+ ] {
+ let batches = (0..NUM_BATCHES)
Review Comment:
Could we build each shape only when Criterion selects one of its two cases?
These batches are constructed before either `bench_function` applies its
filter. In a copy instrumented at `nested_shape_batch`, listing only
`shuffle_partitioning/place (plain, HashAll)` printed that one existing
benchmark but invoked the new fixture builder 32 times, eight per shape. The
loops format 2,359,248 payload strings along the way. An unrelated run pays the
same setup cost.
A per-shape `OnceCell` shared by the two callbacks would keep construction
outside `iter_batched`, reuse the batches when both variants run, and let an
unrelated filtered run or `--list` skip construction entirely.
--
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]
