Skip to content

Commit 34f229b

Browse files
committed
[SPARK-25710][SQL] range should report metrics correctly
## What changes were proposed in this pull request? Currently `Range` reports metrics in batch granularity. This is acceptable, but it's better if we can make it row granularity without performance penalty. Before this PR, the metrics are updated when preparing the batch, which is before we actually consume data. In this PR, the metrics are updated after the data are consumed. There are 2 different cases: 1. The data processing loop has a stop check. The metrics are updated when we need to stop. 2. no stop check. The metrics are updated after the loop. ## How was this patch tested? existing tests and a new benchmark Closes #22698 from cloud-fan/range. Authored-by: Wenchen Fan <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>
1 parent c9ba59d commit 34f229b

4 files changed

Lines changed: 98 additions & 10 deletions

File tree

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
================================================================================================
2+
range
3+
================================================================================================
4+
5+
Java HotSpot(TM) 64-Bit Server VM 1.8.0_161-b12 on Mac OS X 10.13.6
6+
Intel(R) Core(TM) i7-6920HQ CPU @ 2.90GHz
7+
8+
range: Best/Avg Time(ms) Rate(M/s) Per Row(ns) Relative
9+
------------------------------------------------------------------------------------------------
10+
full scan 12674 / 12840 41.4 24.2 1.0X
11+
limit after range 33 / 37 15900.2 0.1 384.4X
12+
filter after range 969 / 985 541.0 1.8 13.1X
13+
count after range 42 / 42 12510.5 0.1 302.4X
14+
count after limit after range 32 / 33 16337.0 0.1 394.9X
15+
16+

sql/core/src/main/scala/org/apache/spark/sql/execution/basicPhysicalOperators.scala

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -452,8 +452,15 @@ case class RangeExec(range: org.apache.spark.sql.catalyst.plans.logical.Range)
452452

453453
val localIdx = ctx.freshName("localIdx")
454454
val localEnd = ctx.freshName("localEnd")
455-
val shouldStop = if (parent.needStopCheck) {
456-
s"if (shouldStop()) { $nextIndex = $value + ${step}L; return; }"
455+
val stopCheck = if (parent.needStopCheck) {
456+
s"""
457+
|if (shouldStop()) {
458+
| $nextIndex = $value + ${step}L;
459+
| $numOutput.add($localIdx + 1);
460+
| $inputMetrics.incRecordsRead($localIdx + 1);
461+
| return;
462+
|}
463+
""".stripMargin
457464
} else {
458465
"// shouldStop check is eliminated"
459466
}
@@ -506,18 +513,18 @@ case class RangeExec(range: org.apache.spark.sql.catalyst.plans.logical.Range)
506513
| $numElementsTodo = 0;
507514
| if ($nextBatchTodo == 0) break;
508515
| }
509-
| $numOutput.add($nextBatchTodo);
510-
| $inputMetrics.incRecordsRead($nextBatchTodo);
511516
| $batchEnd += $nextBatchTodo * ${step}L;
512517
| }
513518
|
514519
| int $localEnd = (int)(($batchEnd - $nextIndex) / ${step}L);
515520
| for (int $localIdx = 0; $localIdx < $localEnd; $localIdx++) {
516521
| long $value = ((long)$localIdx * ${step}L) + $nextIndex;
517522
| ${consume(ctx, Seq(ev))}
518-
| $shouldStop
523+
| $stopCheck
519524
| }
520525
| $nextIndex = $batchEnd;
526+
| $numOutput.add($localEnd);
527+
| $inputMetrics.incRecordsRead($localEnd);
521528
| $taskContext.killTaskIfInterrupted();
522529
| }
523530
""".stripMargin
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
package org.apache.spark.sql.execution.benchmark
19+
20+
import org.apache.spark.benchmark.Benchmark
21+
22+
/**
23+
* Benchmark to measure performance for range operator.
24+
* To run this benchmark:
25+
* {{{
26+
* 1. without sbt:
27+
* bin/spark-submit --class <this class> --jars <spark core test jar> <spark sql test jar>
28+
* 2. build/sbt "sql/test:runMain <this class>"
29+
* 3. generate result: SPARK_GENERATE_BENCHMARK_FILES=1 build/sbt "sql/test:runMain <this class>"
30+
* Results will be written to "benchmarks/RangeBenchmark-results.txt".
31+
* }}}
32+
*/
33+
object RangeBenchmark extends SqlBasedBenchmark {
34+
35+
override def runBenchmarkSuite(): Unit = {
36+
import spark.implicits._
37+
38+
runBenchmark("range") {
39+
val N = 500L << 20
40+
val benchmark = new Benchmark("range", N, output = output)
41+
42+
benchmark.addCase("full scan", numIters = 4) { _ =>
43+
spark.range(N).queryExecution.toRdd.foreach(_ => ())
44+
}
45+
46+
benchmark.addCase("limit after range", numIters = 4) { _ =>
47+
spark.range(N).limit(100).queryExecution.toRdd.foreach(_ => ())
48+
}
49+
50+
benchmark.addCase("filter after range", numIters = 4) { _ =>
51+
spark.range(N).filter('id % 100 === 0).queryExecution.toRdd.foreach(_ => ())
52+
}
53+
54+
benchmark.addCase("count after range", numIters = 4) { _ =>
55+
spark.range(N).count()
56+
}
57+
58+
benchmark.addCase("count after limit after range", numIters = 4) { _ =>
59+
spark.range(N).limit(100).count()
60+
}
61+
62+
benchmark.run()
63+
}
64+
}
65+
}

sql/core/src/test/scala/org/apache/spark/sql/execution/metric/SQLMetricsSuite.scala

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -556,16 +556,16 @@ class SQLMetricsSuite extends SparkFunSuite with SQLMetricsTestUtils with Shared
556556

557557
df.queryExecution.executedPlan.foreach(_.resetMetrics())
558558
// For each partition, we get 2 rows. Then the Filter should produce 2 rows per-partition,
559-
// and Range should produce 1000 rows (one batch) per-partition. Totally Filter produces
560-
// 4 rows, and Range produces 2000 rows.
559+
// and Range should produce 4 rows per-partition ([0, 1, 2, 3] and [15, 16, 17, 18]). Totally
560+
// Filter produces 4 rows, and Range produces 8 rows.
561561
df.queryExecution.toRdd.mapPartitions(_.take(2)).collect()
562-
checkFilterAndRangeMetrics(df, filterNumOutputs = 4, rangeNumOutputs = 2000)
562+
checkFilterAndRangeMetrics(df, filterNumOutputs = 4, rangeNumOutputs = 8)
563563

564564
// Top-most limit will call `CollectLimitExec.executeCollect`, which will only run the first
565-
// task, so totally the Filter produces 2 rows, and Range produces 1000 rows (one batch).
565+
// task, so totally the Filter produces 2 rows, and Range produces 4 rows ([0, 1, 2, 3]).
566566
val df2 = df.limit(2)
567567
df2.collect()
568-
checkFilterAndRangeMetrics(df2, filterNumOutputs = 2, rangeNumOutputs = 1000)
568+
checkFilterAndRangeMetrics(df2, filterNumOutputs = 2, rangeNumOutputs = 4)
569569
}
570570
}
571571

0 commit comments

Comments
 (0)