You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by yucai <gi...@git.apache.org> on 2018/09/20 13:56:39 UTC

[GitHub] spark pull request #22495: [SPARK-25486][TEST] Refactor SortBenchmark to use...

GitHub user yucai opened a pull request:

    https://github.com/apache/spark/pull/22495

    [SPARK-25486][TEST] Refactor SortBenchmark to use main method

    ## What changes were proposed in this pull request?
    
    Refactor SortBenchmark to use main method.
    Generate benchmark result:
    ```
    SPARK_GENERATE_BENCHMARK_FILES=1 build/sbt "sql/test:runMain org.apache.spark.sql.execution.benchmark.SortBenchmark"
    ```
    
    ## How was this patch tested?
    
    manual tests


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/yucai/spark SPARK-25486

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/22495.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 #22495
    
----
commit c3c3d8bb5f7c0fc476c3960f12e53133efd3e2f2
Author: yucai <yy...@...>
Date:   2018-09-20T13:52:40Z

    Refactor SortBenchmark

----


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #22495: [SPARK-25486][TEST] Refactor SortBenchmark to use main m...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/22495
  
    **[Test build #96361 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96361/testReport)** for PR 22495 at commit [`c3c3d8b`](https://github.com/apache/spark/commit/c3c3d8bb5f7c0fc476c3960f12e53133efd3e2f2).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #22495: [SPARK-25486][TEST] Refactor SortBenchmark to use main m...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/22495
  
    **[Test build #96444 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96444/testReport)** for PR 22495 at commit [`be2d1c0`](https://github.com/apache/spark/commit/be2d1c0e1b224386b2d3a5c43b6f2b1638604607).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #22495: [SPARK-25486][TEST] Refactor SortBenchmark to use main m...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/22495
  
    Can one of the admins verify this patch?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #22495: [SPARK-25486][TEST] Refactor SortBenchmark to use main m...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/22495
  
    **[Test build #96444 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96444/testReport)** for PR 22495 at commit [`be2d1c0`](https://github.com/apache/spark/commit/be2d1c0e1b224386b2d3a5c43b6f2b1638604607).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #22495: [SPARK-25486][TEST] Refactor SortBenchmark to use...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:

    https://github.com/apache/spark/pull/22495#discussion_r219725464
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/benchmark/SortBenchmark.scala ---
    @@ -28,12 +28,15 @@ import org.apache.spark.util.random.XORShiftRandom
     
     /**
      * Benchmark to measure performance for aggregate primitives.
    - * To run this:
    - *  build/sbt "sql/test-only *benchmark.SortBenchmark"
    - *
    - * Benchmarks in this file are skipped in normal builds.
    + * {{{
    + *   To run this benchmark:
    + *   1. without sbt: bin/spark-submit --class <this class> <spark sql test jar>
    + *   2. build/sbt "sql/test:runMain <this class>"
    + *   3. generate result: SPARK_GENERATE_BENCHMARK_FILES=1 build/sbt "sql/test:runMain <this class>"
    + *      Results will be written to "benchmarks/<this class>-results.txt".
    + * }}}
      */
    -class SortBenchmark extends BenchmarkWithCodegen {
    +object SortBenchmark extends BenchmarkBase {
    --- End diff --
    
    @yucai . `BenchmarkWithCodegen` is different from `BenchmarkBase`. Can we keep `BenchmarkWithCodegen`?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #22495: [SPARK-25486][TEST] Refactor SortBenchmark to use main m...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/22495
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96361/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #22495: [SPARK-25486][TEST] Refactor SortBenchmark to use main m...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/22495
  
    **[Test build #96361 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96361/testReport)** for PR 22495 at commit [`c3c3d8b`](https://github.com/apache/spark/commit/c3c3d8bb5f7c0fc476c3960f12e53133efd3e2f2).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #22495: [SPARK-25486][TEST] Refactor SortBenchmark to use main m...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/22495
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96444/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #22495: [SPARK-25486][TEST] Refactor SortBenchmark to use main m...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/22495
  
    Can one of the admins verify this patch?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #22495: [SPARK-25486][TEST] Refactor SortBenchmark to use...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/22495


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #22495: [SPARK-25486][TEST] Refactor SortBenchmark to use main m...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/22495
  
    Can one of the admins verify this patch?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #22495: [SPARK-25486][TEST] Refactor SortBenchmark to use main m...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/22495
  
    **[Test build #96441 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96441/testReport)** for PR 22495 at commit [`3943a7f`](https://github.com/apache/spark/commit/3943a7f7b9cfa8f389c765ef4870323c4b40ab05).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #22495: [SPARK-25486][TEST] Refactor SortBenchmark to use main m...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/22495
  
    **[Test build #96419 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96419/testReport)** for PR 22495 at commit [`bbdc202`](https://github.com/apache/spark/commit/bbdc2029c992803e75d04be9e5b2507d6df1779f).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #22495: [SPARK-25486][TEST] Refactor SortBenchmark to use...

Posted by yucai <gi...@git.apache.org>.
Github user yucai commented on a diff in the pull request:

    https://github.com/apache/spark/pull/22495#discussion_r219731873
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/benchmark/SortBenchmark.scala ---
    @@ -28,12 +28,15 @@ import org.apache.spark.util.random.XORShiftRandom
     
     /**
      * Benchmark to measure performance for aggregate primitives.
    - * To run this:
    - *  build/sbt "sql/test-only *benchmark.SortBenchmark"
    - *
    - * Benchmarks in this file are skipped in normal builds.
    + * {{{
    + *   To run this benchmark:
    + *   1. without sbt: bin/spark-submit --class <this class> <spark sql test jar>
    + *   2. build/sbt "sql/test:runMain <this class>"
    + *   3. generate result: SPARK_GENERATE_BENCHMARK_FILES=1 build/sbt "sql/test:runMain <this class>"
    + *      Results will be written to "benchmarks/<this class>-results.txt".
    + * }}}
      */
    -class SortBenchmark extends BenchmarkWithCodegen {
    +object SortBenchmark extends BenchmarkBase {
    --- End diff --
    
    @dongjoon-hyun `SortBenchmark` does not use any function provided in `BenchmarkWithCodegen`, so I remove it.
    Another option is like #22484 did, make `BenchmarkWithCodegen` extend `BenchmarkBase`, and then `SortBenchmark` can extend `BenchmarkWithCodegen`.
    Do you prefer the 2nd way?
    
    BTW, congratulations! :)


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #22495: [SPARK-25486][TEST] Refactor SortBenchmark to use main m...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:

    https://github.com/apache/spark/pull/22495
  
    Merged to `master`.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #22495: [SPARK-25486][TEST] Refactor SortBenchmark to use main m...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/22495
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #22495: [SPARK-25486][TEST] Refactor SortBenchmark to use main m...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/22495
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96441/
    Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #22495: [SPARK-25486][TEST] Refactor SortBenchmark to use main m...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/22495
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #22495: [SPARK-25486][TEST] Refactor SortBenchmark to use main m...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/22495
  
    Merged build finished. Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #22495: [SPARK-25486][TEST] Refactor SortBenchmark to use main m...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/22495
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96419/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #22495: [SPARK-25486][TEST] Refactor SortBenchmark to use...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:

    https://github.com/apache/spark/pull/22495#discussion_r220299180
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/benchmark/SortBenchmark.scala ---
    @@ -28,12 +28,15 @@ import org.apache.spark.util.random.XORShiftRandom
     
     /**
      * Benchmark to measure performance for aggregate primitives.
    - * To run this:
    - *  build/sbt "sql/test-only *benchmark.SortBenchmark"
    - *
    - * Benchmarks in this file are skipped in normal builds.
    + * {{{
    + *   To run this benchmark:
    + *   1. without sbt: bin/spark-submit --class <this class> <spark sql test jar>
    + *   2. build/sbt "sql/test:runMain <this class>"
    + *   3. generate result: SPARK_GENERATE_BENCHMARK_FILES=1 build/sbt "sql/test:runMain <this class>"
    + *      Results will be written to "benchmarks/<this class>-results.txt".
    + * }}}
      */
    -class SortBenchmark extends BenchmarkWithCodegen {
    +object SortBenchmark extends BenchmarkBase {
    --- End diff --
    
    I got it. +1 for the current one. Thanks, @yucai . 


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #22495: [SPARK-25486][TEST] Refactor SortBenchmark to use main m...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/22495
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #22495: [SPARK-25486][TEST] Refactor SortBenchmark to use main m...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/22495
  
    **[Test build #96419 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96419/testReport)** for PR 22495 at commit [`bbdc202`](https://github.com/apache/spark/commit/bbdc2029c992803e75d04be9e5b2507d6df1779f).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #22495: [SPARK-25486][TEST] Refactor SortBenchmark to use main m...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/22495
  
    **[Test build #96441 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96441/testReport)** for PR 22495 at commit [`3943a7f`](https://github.com/apache/spark/commit/3943a7f7b9cfa8f389c765ef4870323c4b40ab05).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org