Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 3 commits into from

Conversation

yucai
Copy link
Contributor

@yucai yucai commented Sep 20, 2018

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

@SparkQA
Copy link

SparkQA commented Sep 20, 2018

Test build #96361 has finished for PR 22495 at commit c3c3d8b.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Sep 21, 2018

Test build #96419 has finished for PR 22495 at commit bbdc202.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Sep 21, 2018

Test build #96441 has finished for PR 22495 at commit 3943a7f.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Sep 21, 2018

Test build #96444 has finished for PR 22495 at commit be2d1c0.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

*/
class SortBenchmark extends BenchmarkWithCodegen {
object SortBenchmark extends BenchmarkBase {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yucai . BenchmarkWithCodegen is different from BenchmarkBase. Can we keep BenchmarkWithCodegen?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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! :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got it. +1 for the current one. Thanks, @yucai .

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, LGTM.

@dongjoon-hyun
Copy link
Member

Merged to master.

@asfgit asfgit closed this in 04db035 Sep 25, 2018
daspalrahul pushed a commit to daspalrahul/spark that referenced this pull request Sep 29, 2018
## 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

Closes apache#22495 from yucai/SPARK-25486.

Authored-by: yucai <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants