chore: Add shuffle benchmark for deeply nested schemas #2902

andygrove · 2025-12-13T18:41:07Z

Which issue does this PR close?

Closes #.

Rationale for this change

Example schema (partial):

root
 |-- c_1: long (nullable = true)
 |-- c_2: array (nullable = true)
 |    |-- element: long (containsNull = true)
 |-- c_4: struct (nullable = true)
 |    |-- c_5: decimal(10,2) (nullable = true)
 |    |-- c_6: string (nullable = true)
 |    |-- c_7: long (nullable = true)
 |    |-- c_8: array (nullable = true)
 |    |    |-- element: string (containsNull = true)
 |    |-- c_10: array (nullable = true)
 |    |    |-- element: decimal(10,2) (containsNull = true)
 |    |-- c_12: string (nullable = true)
 |-- c_13: struct (nullable = true)
 |    |-- c_14: array (nullable = true)
 |    |    |-- element: long (containsNull = true)
 |    |-- c_16: struct (nullable = true)
 |    |    |-- c_17: long (nullable = true)
 |    |    |-- c_18: long (nullable = true)
 |    |    |-- c_19: string (nullable = true)
 |    |    |-- c_20: string (nullable = true)
 |    |    |-- c_21: decimal(10,2) (nullable = true)
 |    |    |-- c_22: long (nullable = true)
 |    |    |-- c_23: decimal(10,2) (nullable = true)
 |    |    |-- c_24: decimal(10,2) (nullable = true)
 |    |    |-- c_25: long (nullable = true)
...

OpenJDK 64-Bit Server VM 17.0.17+10-Ubuntu-122.04 on Linux 6.8.0-87-generic
AMD Ryzen 9 7950X3D 16-Core Processor
SQL Deeply Nested (depth=2) Shuffle (5 Partition):  Best Time(ms)   Avg Time(ms)   Stdev(ms)    Rate(M/s)   Per Row(ns)   Relative
---------------------------------------------------------------------------------------------------------------------------------
SQL Parquet - Spark                                          426            462          24          0.0       42607.4       1.0X
SQL Parquet - Comet (Spark Shuffle)                          389            431          28          0.0       38927.6       1.1X
SQL Parquet - Comet (JVM Shuffle)                            548            597          48          0.0       54806.4       0.8X
SQL Parquet - Comet (Native Shuffle)                         377            385           8          0.0       37723.2       1.1X

OpenJDK 64-Bit Server VM 17.0.17+10-Ubuntu-122.04 on Linux 6.8.0-87-generic
AMD Ryzen 9 7950X3D 16-Core Processor
SQL Deeply Nested (depth=2) Shuffle (201 Partition):  Best Time(ms)   Avg Time(ms)   Stdev(ms)    Rate(M/s)   Per Row(ns)   Relative
-----------------------------------------------------------------------------------------------------------------------------------
SQL Parquet - Spark                                            451            508          48          0.0       45103.4       1.0X
SQL Parquet - Comet (Spark Shuffle)                            398            406          10          0.0       39764.0       1.1X
SQL Parquet - Comet (JVM Shuffle)                             2158           2181          32          0.0      215831.0       0.2X
SQL Parquet - Comet (Native Shuffle)                           402            435          46          0.0       40230.3       1.1X

What changes are included in this PR?

How are these changes tested?

codecov-commenter · 2025-12-13T19:49:48Z

Codecov Report

❌ Patch coverage is 74.19355% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.44%. Comparing base (f09f8af) to head (6bd7cc6).
⚠️ Report is 775 commits behind head on main.

Files with missing lines	Patch %	Lines
...a/org/apache/comet/testing/FuzzDataGenerator.scala	74.19%	0 Missing and 8 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #2902      +/-   ##
============================================
+ Coverage     56.12%   59.44%   +3.31%     
- Complexity      976     1379     +403     
============================================
  Files           119      167      +48     
  Lines         11743    15384    +3641     
  Branches       2251     2557     +306     
============================================
+ Hits           6591     9145    +2554     
- Misses         4012     4945     +933     
- Partials       1140     1294     +154

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

spark/src/test/scala/org/apache/spark/sql/benchmark/CometShuffleBenchmark.scala

andygrove · 2025-12-15T16:18:35Z

@comphead the addition to the fuzz generator for generating deeply nested schema could be useful to try and reproduce the reported issue about shuffle metrics being inaccurate

comphead · 2025-12-15T17:11:58Z

dev/benchmarks/comet-tpch.sh

    --conf spark.plugins=org.apache.spark.CometPlugin \
    --conf spark.shuffle.manager=org.apache.spark.sql.comet.execution.shuffle.CometShuffleManager \
    --conf spark.comet.exec.replaceSortMergeJoin=true \
+    --conf spark.comet.exec.shuffle.writeBufferSize=32000000 \


can we also benchmark with shuffle batch size and record batch size?

This was an accidental check in. I have reverted it since it is unrelated to nested schemas.

comphead · 2025-12-15T17:13:49Z

maps are not in the scope because Comet dont support grouping by maps yet?

comphead · 2025-12-15T17:16:36Z

Wondering if nested depth can be configured, I can see now 2 level max, in real world examples typically can be up to 5-6.

More than 5-6 is quite rare, but prob would be still nice to benchmark it

andygrove · 2025-12-16T00:25:41Z

Wondering if nested depth can be configured, I can see now 2 level max, in real world examples typically can be up to 5-6.

More than 5-6 is quite rare, but prob would be still nice to benchmark it

The benchmarks currently run with max depth 2 and 4:

for (maxDepth <- Seq(2, 4)) {

However, due to the random schema generation approach, there is no guarantee that the schema will reach these depths. I will see how I can improve this.

andygrove · 2025-12-16T16:05:22Z

maps are not in the scope because Comet dont support grouping by maps yet?

I was being lazy. I have added map support now.

andygrove · 2025-12-16T16:05:43Z

Wondering if nested depth can be configured, I can see now 2 level max, in real world examples typically can be up to 5-6.
More than 5-6 is quite rare, but prob would be still nice to benchmark it

The benchmarks currently run with max depth 2 and 4:
for (maxDepth <- Seq(2, 4)) {
However, due to the random schema generation approach, there is no guarantee that the schema will reach these depths. I will see how I can improve this.

@comphead I have now added minDepth as well.

comphead · 2025-12-16T17:38:11Z

spark/src/main/scala/org/apache/comet/testing/FuzzDataGenerator.scala

+        generators += (() => generateStruct(depth + 1, name))
+      }
+      if (options.generateMap && depth < maxDepth) {
+        generators += (() => generateMap(depth, name))


just wondering why depth is not + 1 here like for arrays and structs?

Thanks, that's incorrect ... I will fix

comphead · 2025-12-16T17:39:00Z

spark/src/test/scala/org/apache/spark/sql/benchmark/CometShuffleBenchmark.scala

+
+    // nested type shuffle
+    val numRows = 1000
+    for (generateArray <- Seq(true, false)) {


should map be here as well?

I added this now

spark/src/main/scala/org/apache/comet/testing/FuzzDataGenerator.scala

comphead

Thanks @andygrove it it looks good to me the way it is

mbutrovich

Thanks @andygrove!

andygrove added 5 commits December 13, 2025 11:14

add helper methods

4774cac

format

f659509

new benchmark

d187eda

new benchmark

70cf9b8

refine

30b984d

andygrove marked this pull request as ready for review December 13, 2025 18:57

andygrove mentioned this pull request Dec 13, 2025

Columnar shuffle with nested types is slower than Spark #2904

Open

andygrove added 4 commits December 13, 2025 12:00

fix

2eb4c12

move schema generation to FuzzDataGenerator

fe1941c

improve

9f2154e

fix

5cf46c1

andygrove added 2 commits December 13, 2025 13:27

improve

e8a5390

improve

2e49468

wForget reviewed Dec 15, 2025

View reviewed changes

spark/src/test/scala/org/apache/spark/sql/benchmark/CometShuffleBenchmark.scala Outdated Show resolved Hide resolved

andygrove added 3 commits December 15, 2025 08:04

address feedback

e82796b

remove unused import

6048f7b

format

bc4560f

comphead reviewed Dec 15, 2025

View reviewed changes

revert change to tpch script

b59a519

andygrove added 2 commits December 16, 2025 08:27

add minDepth

31097f0

move test

f52afdf

andygrove marked this pull request as draft December 16, 2025 15:33

fix

bcfc0db

andygrove marked this pull request as ready for review December 16, 2025 16:04

andygrove added 2 commits December 16, 2025 09:41

fix

6bef657

fix

c32a665

comphead reviewed Dec 16, 2025

View reviewed changes

andygrove added 4 commits December 16, 2025 11:01

address feedback

9fda7f8

format

720f0f8

save

b9f79fa

Merge remote-tracking branch 'origin/no-row-step' into shuffle-benchmark

6d9ae61

andygrove requested a review from mbutrovich December 18, 2025 14:19

mbutrovich reviewed Dec 18, 2025

View reviewed changes

spark/src/main/scala/org/apache/comet/testing/FuzzDataGenerator.scala Outdated Show resolved Hide resolved

comphead approved these changes Dec 18, 2025

View reviewed changes

andygrove added 2 commits December 18, 2025 09:30

remove use of AtomicLong

ade1d98

upmerge

6bd7cc6

mbutrovich approved these changes Dec 18, 2025

View reviewed changes

andygrove merged commit 60c0f1e into apache:main Dec 19, 2025
134 of 137 checks passed

chore: Add shuffle benchmark for deeply nested schemas #2902

chore: Add shuffle benchmark for deeply nested schemas #2902

Uh oh!

Conversation

andygrove commented Dec 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

How are these changes tested?

Uh oh!

codecov-commenter commented Dec 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

andygrove commented Dec 15, 2025

Uh oh!

comphead Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

andygrove Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

comphead commented Dec 15, 2025

Uh oh!

comphead commented Dec 15, 2025

Uh oh!

andygrove commented Dec 16, 2025

Uh oh!

andygrove commented Dec 16, 2025

Uh oh!

andygrove commented Dec 16, 2025

Uh oh!

comphead Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

andygrove Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

andygrove Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

comphead Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

andygrove Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

comphead left a comment

Choose a reason for hiding this comment

Uh oh!

mbutrovich left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

andygrove commented Dec 13, 2025 •

edited

Loading

codecov-commenter commented Dec 13, 2025 •

edited

Loading