Repository navigation
CometColumnarToRowBenchmark measures Spark in all three arms #5624
Description
Describe the bug
CometColumnarToRowBenchmark measures vanilla Spark in all three of its cases, so its published
numbers do not compare what they claim to compare.
The benchmark overrides getSparkSession and builds its own SparkConf without setting
spark.shuffle.manager. spark.comet.shuffle.enabled defaults to true, and
CometSparkSessionExtensions.isCometLoaded bails out in exactly that combination:
if (COMET_SHUFFLE_ENABLED.get(conf) && !isCometShuffleManagerEnabled(conf)) {
logWarning("Comet extension is disabled because spark.shuffle.manager is not set to ...")
return false
}That disables Comet for the whole session - no scan conversion, no operator conversion - so
addC2RBenchmarkCases runs three identical vanilla Spark plans and only the case labels differ.
CometBenchmarkBase.getSparkSession sets the shuffle manager for this reason and carries a comment
explaining it. The override in CometColumnarToRowBenchmark drops it.
Evidence
Two independent checks, both on main (commit 2949fd0, Spark 4.1, release build, M3 Max):
1. The executed plan. Capturing the executed plan for a session configured exactly like this
benchmark's shows Spark's operators in the arms labelled Comet:
*(1) ColumnarToRow
+- FileScan parquet [...] Batched: true, ...
Adding spark.shuffle.manager -> CometShuffleManager to the same session and changing nothing else
makes the same arms plan CometColumnarToRow and CometNativeColumnarToRow.
2. The numbers. Running the benchmark as-is, all three arms land within ~1% in every one of the
ten groups:
Columnar to Row - Struct Types: Best Time(ms) Avg Time(ms) Stdev(ms) Relative
Spark (ColumnarToRowExec) 186 196 12 1.0X
Comet JVM (CometColumnarToRowExec) 186 196 9 1.0X
Comet Native (CometNativeColumnarToRowExec) 186 202 18 1.0X
Columnar to Row - Array Types: Best Time(ms) Avg Time(ms) Stdev(ms) Relative
Spark (ColumnarToRowExec) 197 201 3 1.0X
Comet JVM (CometColumnarToRowExec) 196 200 2 1.0X
Comet Native (CometNativeColumnarToRowExec) 196 200 3 1.0X
That result is not plausible on its own terms: the doc for
spark.comet.exec.columnarToRow.native.enabled says the native conversion "carries a fixed JNI
cost per batch and is slower than the JVM implementation for small batches", yet here it is
identical to both other arms everywhere.
Steps to reproduce
make benchmark-org.apache.spark.sql.benchmark.CometColumnarToRowBenchmark
Expected behavior
The Comet arms should plan Comet operators. Fix is either to set the shuffle manager in the
override, or to call super.getSparkSession and adjust from there the way CometReadBenchmark
does.
Additional context
Two related notes:
-
A second, quieter version of the same trap is in this benchmark:
fixedWidthOnlyBenchmarkandprimitiveTypesBenchmarkboth includetinyint/smallint
columns, which Comet's scan rejects unlessspark.comet.parquet.unsignedSmallIntCheckis
false. Even with the shuffle manager fixed, those two groups fall back to Spark's scan in
every arm.CometTestBasesets that config to false; the benchmarks do not. -
The generalisable fix is a plan assertion, so a silently-Spark arm fails loudly instead of
producing a plausible-looking table. Something like: run each arm once beforebenchmark.run(),
capture the executed plan through aQueryExecutionListener, and print a warning into the
results file when the arm did not plan the operator its label names. Worth considering for
CometBenchmarkBaseso every benchmark inherits it.
I audited the other benchmarks under spark/src/test/scala/org/apache/spark/sql/benchmark/ for the
same problem. Only this one is affected: CometReadBenchmark calls super.getSparkSession,
CometTPCQueryBase sets the shuffle manager, and CometC2RIsolatedBench never builds a
SparkSession at all.
Activity
rich7420 commentedon Sep 5, 2026
take