[GitHub] spark pull request #22522: [SPARK-25510][TEST] Create new trait replace Benc...
Github user wangyum closed the pull request at: https://github.com/apache/spark/pull/22522 --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22522: [SPARK-25510][TEST] Create new trait replace Benc...
GitHub user wangyum reopened a pull request: https://github.com/apache/spark/pull/22522 [SPARK-25510][TEST] Create new trait replace BenchmarkWithCodegen ## What changes were proposed in this pull request? We need create a new trait to replace `BenchmarkWithCodegen` as `BenchmarkWithCodegen` extends from `SparkFunSuite`. For example. when doing `AggregateBenchmark` refactor. Before this change: ```scala object AggregateBenchmark extends BenchmarkBase { lazy val sparkSession = SparkSession.builder .master("local[1]") .appName(this.getClass.getSimpleName) .config("spark.sql.shuffle.partitions", 1) .config("spark.sql.autoBroadcastJoinThreshold", 1) .getOrCreate() /** Runs function `f` with whole stage codegen on and off. */ def runBenchmark(name: String, cardinality: Long)(f: => Unit): Unit = { val benchmark = new Benchmark(name, cardinality, output = output) benchmark.addCase(s"$name wholestage off", numIters = 2) { iter => sparkSession.conf.set("spark.sql.codegen.wholeStage", value = false) f } benchmark.addCase(s"$name wholestage on", numIters = 5) { iter => sparkSession.conf.set("spark.sql.codegen.wholeStage", value = true) f } benchmark.run() } override def benchmark(): Unit = { runBenchmark("aggregate without grouping") { val N = 500L << 22 runBenchmark("agg w/o group", N) { sparkSession.range(N).selectExpr("sum(id)").collect() } } ... ``` After this change: ```scala object AggregateBenchmark extends SqlBasedBenchmark { override def benchmark(): Unit = { runBenchmark("aggregate without grouping") { val N = 500L << 22 runBenchmark("agg w/o group", N) { sparkSession.range(N).selectExpr("sum(id)").collect() } } ... ``` All these benchmarks will use this trait: ``` AggregateBenchmark BenchmarkWideTable JoinBenchmark MiscBenchmark ObjectHashAggregateExecBenchmark SortBenchmark UnsafeArrayDataBenchmark ``` ## How was this patch tested? manual tests You can merge this pull request into a Git repository by running: $ git pull https://github.com/wangyum/spark SPARK-25510 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/spark/pull/22522.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #22522 commit 275cc6c5f8f106eb339c7ed01734e279a223705e Author: Yuming Wang Date: 2018-09-21T17:36:57Z Create new BenchmarkWithCodegen trait doesn't extends SparkFunSuite commit c62a3be61cdac0e3dffbd5d0ce4a431c2ef2f931 Author: Yuming Wang Date: 2018-09-29T00:26:47Z Merge remote-tracking branch 'upstream/master' into SPARK-25510 commit 20668adc843815e97b3d1950ed2ab86bdb66540f Author: Yuming Wang Date: 2018-09-29T00:47:28Z Rename RunBenchmarkWithCodegen to SqlBasedBenchmark --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22522: [SPARK-25510][TEST] Create new trait replace Benc...
Github user wangyum closed the pull request at: https://github.com/apache/spark/pull/22522 --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22522: [SPARK-25510][TEST] Create new trait replace Benc...
Github user wangyum commented on a diff in the pull request: https://github.com/apache/spark/pull/22522#discussion_r219674900 --- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/benchmark/RunBenchmarkWithCodegen.scala --- @@ -0,0 +1,52 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + *http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.spark.sql.execution.benchmark + +import org.apache.spark.benchmark.Benchmark +import org.apache.spark.sql.SparkSession + +/** + * Common base trait for micro benchmarks that are supposed to run standalone (i.e. not together + * with other benchmarks). + */ +private[benchmark] trait RunBenchmarkWithCodegen { --- End diff -- Done --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22522: [SPARK-25510][TEST] Create new trait replace Benc...
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22522#discussion_r219672131 --- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/benchmark/RunBenchmarkWithCodegen.scala --- @@ -0,0 +1,52 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + *http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.spark.sql.execution.benchmark + +import org.apache.spark.benchmark.Benchmark +import org.apache.spark.sql.SparkSession + +/** + * Common base trait for micro benchmarks that are supposed to run standalone (i.e. not together + * with other benchmarks). + */ +private[benchmark] trait RunBenchmarkWithCodegen { --- End diff -- I'd remove `private[benchmark]` to be consistent with other benchmark classes. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22522: [SPARK-25510][TEST] Create new trait replace Benc...
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/22522#discussion_r219672121 --- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/benchmark/RunBenchmarkWithCodegen.scala --- @@ -0,0 +1,52 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + *http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.spark.sql.execution.benchmark + +import org.apache.spark.benchmark.Benchmark +import org.apache.spark.sql.SparkSession + +/** + * Common base trait for micro benchmarks that are supposed to run standalone (i.e. not together + * with other benchmarks). + */ +private[benchmark] trait RunBenchmarkWithCodegen { --- End diff -- shall this extend `BenchmarkBase`? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #22522: [SPARK-25510][TEST] Create new trait replace Benc...
GitHub user wangyum opened a pull request: https://github.com/apache/spark/pull/22522 [SPARK-25510][TEST] Create new trait replace BenchmarkWithCodegen ## What changes were proposed in this pull request? We need create a new trait to replace `BenchmarkWithCodegen` as `BenchmarkWithCodegen` extends from `SparkFunSuite`. For example. when doing `AggregateBenchmark` refactor. Before this change, it should be: ```scala object AggregateBenchmark extends BenchmarkBase { lazy val sparkSession = SparkSession.builder .master("local[1]") .appName(this.getClass.getSimpleName) .config("spark.sql.shuffle.partitions", 1) .config("spark.sql.autoBroadcastJoinThreshold", 1) .getOrCreate() /** Runs function `f` with whole stage codegen on and off. */ def runBenchmark(name: String, cardinality: Long)(f: => Unit): Unit = { val benchmark = new Benchmark(name, cardinality, output = output) benchmark.addCase(s"$name wholestage off", numIters = 2) { iter => sparkSession.conf.set("spark.sql.codegen.wholeStage", value = false) f } benchmark.addCase(s"$name wholestage on", numIters = 5) { iter => sparkSession.conf.set("spark.sql.codegen.wholeStage", value = true) f } benchmark.run() } override def benchmark(): Unit = { runBenchmark("aggregate without grouping") { val N = 500L << 22 runBenchmark("agg w/o group", N) { sparkSession.range(N).selectExpr("sum(id)").collect() } } ... ``` After this change: ```scala object AggregateBenchmark extends BenchmarkBase with RunBenchmarkWithCodegen { override def benchmark(): Unit = { runBenchmark("aggregate without grouping") { val N = 500L << 22 runBenchmark("agg w/o group", N) { sparkSession.range(N).selectExpr("sum(id)").collect() } } ... ``` All affect benchmarks: ``` AggregateBenchmark BenchmarkWideTable JoinBenchmark MiscBenchmark ObjectHashAggregateExecBenchmark SortBenchmark UnsafeArrayDataBenchmark ``` ## How was this patch tested? manual tests You can merge this pull request into a Git repository by running: $ git pull https://github.com/wangyum/spark SPARK-25510 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/spark/pull/22522.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #22522 commit 275cc6c5f8f106eb339c7ed01734e279a223705e Author: Yuming Wang Date: 2018-09-21T17:36:57Z Create new BenchmarkWithCodegen trait doesn't extends SparkFunSuite --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org