[GitHub] spark pull request #22522: [SPARK-25510][TEST] Create new trait replace Benc...

2018-10-01 Thread wangyum
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...

2018-09-28 Thread wangyum
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...

2018-09-22 Thread wangyum
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...

2018-09-22 Thread wangyum
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...

2018-09-22 Thread cloud-fan
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...

2018-09-22 Thread cloud-fan
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...

2018-09-21 Thread wangyum
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