Skip to content

Conversation

@HyukjinKwon
Copy link
Member

@HyukjinKwon HyukjinKwon commented Mar 30, 2021

What changes were proposed in this pull request?

This PR proposes to add a script that detects and runs all benchmarks.

Why are the changes needed?

To run the benchmarks easily. This is actually for SPARK-34821.

Does this PR introduce any user-facing change?

No, dev-only.

How was this patch tested?

Manually tested with the command below after building Spark:

SPARK_GENERATE_BENCHMARK_FILES=1 bin/spark-submit --class \
     org.apache.spark.benchmark.Benchmarks --jars \
     "`find . -name "*3.2.0-SNAPSHOT-tests.jar" | paste -sd ',' -`" \
     ./core/target/scala-2.12/spark-core_2.12-3.2.0-SNAPSHOT-tests.jar

This is ongoing work. I will double check with working on SPARK-34821 and updating the results.

@HyukjinKwon
Copy link
Member Author

Thanks @srowen. I just fixed some style nits additionally.

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.

Thank you, @HyukjinKwon . This looks helpful.

Please note that some benchmarks need extra cares.
For example, ExternalAppendOnlyUnsafeRowArrayBenchmark requires spark.memory.debugFill=false option additionally. You may want to keep exclude-list for some benchmarks like that.

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.

Since this will affect all benchmark results, it would be great if we can see the actual generated results to verify that they are reasonable.

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.

I realized that this PR is designed to use spark-submit only. +1.

Copy link
Member

@MaxGekk MaxGekk left a comment

Choose a reason for hiding this comment

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

I don't understand the case when you need to run all detected benchmarks. Especially, when the sequential run will take days.

I thought you was going to give to others opportunity of running some specific benchmarks in user's PR/branch.

@SparkQA
Copy link

SparkQA commented Mar 30, 2021

Kubernetes integration test unable to build dist.

exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/41312/

@SparkQA
Copy link

SparkQA commented Mar 30, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/41314/

@SparkQA
Copy link

SparkQA commented Mar 30, 2021

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/41314/

@SparkQA
Copy link

SparkQA commented Mar 30, 2021

Test build #136731 has finished for PR 32005 at commit f43f7d5.

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

@SparkQA
Copy link

SparkQA commented Mar 30, 2021

Test build #136732 has finished for PR 32005 at commit f257aab.

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

@HyukjinKwon
Copy link
Member Author

HyukjinKwon commented Mar 30, 2021

Max, this will be used to run a specific benchmark, wild card, and all benchmarks. We should update all the benchmark results.

GA build can run up to 72 hours fwiw.

@maropu
Copy link
Member

maropu commented Mar 30, 2021

It looks fine, too.

@github-actions github-actions bot added the CORE label Mar 30, 2021
@HyukjinKwon
Copy link
Member Author

Thanks guys. Let me merge this in first and proceed (it won't break or affect anything in our CI anyway). I am working on SPARK-34821 now. Let's see how it goes!

@HyukjinKwon
Copy link
Member Author

Merged to master.

@HyukjinKwon HyukjinKwon deleted the SPARK-34907 branch January 4, 2022 00:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants