[SPARK-7718] [SQL] Speed up partitioning by avoiding closure cleaning by andrewor14 · Pull Request #6256 · apache/spark

andrewor14 · 2015-05-19T06:04:10Z

According to @yhuai we spent 6-7 seconds cleaning closures in a partitioning job that takes 12 seconds. Since we provide these closures in Spark we know for sure they are serializable, so we can bypass the cleaning.

andrewor14 · 2015-05-19T06:04:16Z

Also cc/ @pwendell

AmplabJenkins · 2015-05-19T06:07:10Z

Merged build triggered.

AmplabJenkins · 2015-05-19T06:07:15Z

Merged build started.

SparkQA · 2015-05-19T06:07:37Z

Test build #33060 has started for PR 6256 at commit e92dff2.

andrewor14 · 2015-05-19T06:07:55Z

@yhuai can you try this and see if you can notice the speedup? (Does the closure get serialized correctly?)

liancheng · 2015-05-19T06:19:01Z

@andrewor14 I can do a local microbenchmark for this.

AmplabJenkins · 2015-05-19T06:32:11Z

Merged build triggered.

AmplabJenkins · 2015-05-19T06:32:21Z

Merged build started.

SparkQA · 2015-05-19T06:34:25Z

Test build #33061 has started for PR 6256 at commit f7fe143.

SparkQA · 2015-05-19T07:59:29Z

Test build #33060 has finished for PR 6256 at commit e92dff2.

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

AmplabJenkins · 2015-05-19T07:59:33Z

Merged build finished. Test PASSed.

AmplabJenkins · 2015-05-19T07:59:34Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/33060/
Test PASSed.

SparkQA · 2015-05-19T08:30:32Z

Test build #33061 has finished for PR 6256 at commit f7fe143.

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

AmplabJenkins · 2015-05-19T08:30:36Z

Merged build finished. Test PASSed.

AmplabJenkins · 2015-05-19T08:30:36Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/33061/
Test PASSed.

liancheng · 2015-05-19T09:04:31Z

Used the same micro benchmark configuration and code in #6225. Removing the clean() call does help, and leads to a ~6% performance gain.

(Not able to upload jvisualvm screenshots right now, will fill them in later.)

andrewor14 · 2015-05-19T16:47:36Z

Hm, 6% seems much lower than what I expected. Maybe it's not super worth it.

yhuai · 2015-05-19T17:02:19Z

@andrewor14 Cheng's test did not include my pr (#6252). I just tested this one and #6252. The compilation time is down from 12.6s to 8.3s. My test table has 5000 partitions.

AmplabJenkins · 2015-05-19T19:17:13Z

Merged build triggered.

AmplabJenkins · 2015-05-19T19:17:18Z

Merged build started.

SparkQA · 2015-05-19T19:19:11Z

Test build #33092 has started for PR 6256 at commit 523f042.

andrewor14 · 2015-05-19T19:20:52Z

I see. From @liancheng's PR description at #6225, the 6% speed up is from the 53s, which is roughly 3.2s. This is more or less consistent with what @yhuai observed after applying this patch to his PR, where the speed up is roughly 4.3s.

In other words the real speed up is about 34%!

andrewor14 · 2015-05-19T19:22:10Z

After some more offline discussion with @yhuai we decided to remove the unnecessary calls to Utils.getCallSite as well, which collectively took up to another 4s in his 12.6s. If we skip all of these calls for internal RDDs we may notice another significant speedup.

SparkQA · 2015-05-19T21:44:03Z

Test build #33092 has finished for PR 6256 at commit 523f042.

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

AmplabJenkins · 2015-05-19T21:44:08Z

Merged build finished. Test PASSed.

AmplabJenkins · 2015-05-19T21:44:09Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/33092/
Test PASSed.

…peed-up Conflicts: sql/core/src/main/scala/org/apache/spark/sql/sources/DataSourceStrategy.scala

AmplabJenkins · 2015-05-20T18:42:11Z

Merged build triggered.

AmplabJenkins · 2015-05-20T18:42:18Z

Merged build started.

SparkQA · 2015-05-20T18:44:25Z

Test build #33159 has started for PR 6256 at commit 10f7e3e.

SparkQA · 2015-05-20T18:46:07Z

Test build #33159 has finished for PR 6256 at commit 10f7e3e.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2015-05-20T18:46:09Z

Merged build finished. Test FAILed.

AmplabJenkins · 2015-05-20T18:46:09Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/33159/
Test FAILed.

AmplabJenkins · 2015-05-20T18:52:17Z

Merged build triggered.

AmplabJenkins · 2015-05-20T18:52:24Z

Merged build started.

SparkQA · 2015-05-20T18:54:33Z

Test build #33161 has started for PR 6256 at commit a82b451.

SparkQA · 2015-05-20T21:21:27Z

Test build #33161 has finished for PR 6256 at commit a82b451.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class MultilabelMetrics(JavaModelWrapper):

AmplabJenkins · 2015-05-20T21:21:32Z

Merged build finished. Test PASSed.

AmplabJenkins · 2015-05-20T21:21:32Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/33161/
Test PASSed.

yhuai · 2015-05-21T21:32:57Z

LGTM. I am going to merge it to master and branch 1.4.

According to yhuai we spent 6-7 seconds cleaning closures in a partitioning job that takes 12 seconds. Since we provide these closures in Spark we know for sure they are serializable, so we can bypass the cleaning. Author: Andrew Or <andrew@databricks.com> Closes#6256 from andrewor14/sql-partition-speed-up and squashes the following commits: a82b451 [Andrew Or] Fix style 10f7e3e [Andrew Or] Avoid getting call sites and cleaning closures 17e2943 [Andrew Or] Merge branch 'master' of github.com:apache/spark into sql-partition-speed-up 523f042 [Andrew Or] Skip unnecessary Utils.getCallSites too f7fe143 [Andrew Or] Avoid unnecessary closure cleaning (cherry picked from commit 5287eec) Signed-off-by: Yin Huai <yhuai@databricks.com>

According to yhuai we spent 6-7 seconds cleaning closures in a partitioning job that takes 12 seconds. Since we provide these closures in Spark we know for sure they are serializable, so we can bypass the cleaning. Author: Andrew Or <andrew@databricks.com> Closesapache#6256 from andrewor14/sql-partition-speed-up and squashes the following commits: a82b451 [Andrew Or] Fix style 10f7e3e [Andrew Or] Avoid getting call sites and cleaning closures 17e2943 [Andrew Or] Merge branch 'master' of github.com:apache/spark into sql-partition-speed-up 523f042 [Andrew Or] Skip unnecessary Utils.getCallSites too f7fe143 [Andrew Or] Avoid unnecessary closure cleaning

andrewor14 mentioned this pull request May 19, 2015
[SPARK-7713] [SQL] Use shared broadcast hadoop conf for partitioned table scan. #6252
Closed

Avoid unnecessary closure cleaning
f7fe143

andrewor14 force-pushed the sql-partition-speed-up branch from e92dff2 to f7fe143Compare May 19, 2015 06:31

Skip unnecessary Utils.getCallSites too
523f042

Andrew Or added 2 commits May 20, 2015 11:34

Merge branch 'master' of github.com:apache/spark into sql-partition-s…
17e2943
…peed-up Conflicts: sql/core/src/main/scala/org/apache/spark/sql/sources/DataSourceStrategy.scala

Avoid getting call sites and cleaning closures
10f7e3e

Fix style
a82b451

asfgit closed this in 5287eecMay 21, 2015

andrewor14 deleted the sql-partition-speed-up branch May 21, 2015 22:02

Uh oh!

Conversation

andrewor14 commented May 19, 2015

Uh oh!

andrewor14 commented May 19, 2015

Uh oh!

AmplabJenkins commented May 19, 2015

Uh oh!

AmplabJenkins commented May 19, 2015

Uh oh!

SparkQA commented May 19, 2015

Uh oh!

andrewor14 commented May 19, 2015

Uh oh!

liancheng commented May 19, 2015

Uh oh!

AmplabJenkins commented May 19, 2015

Uh oh!

AmplabJenkins commented May 19, 2015

Uh oh!

SparkQA commented May 19, 2015

Uh oh!

SparkQA commented May 19, 2015

Uh oh!

AmplabJenkins commented May 19, 2015

Uh oh!

AmplabJenkins commented May 19, 2015

Uh oh!

SparkQA commented May 19, 2015

Uh oh!

AmplabJenkins commented May 19, 2015

Uh oh!

AmplabJenkins commented May 19, 2015

Uh oh!

liancheng commented May 19, 2015

Uh oh!

andrewor14 commented May 19, 2015

Uh oh!

yhuai commented May 19, 2015

Uh oh!

AmplabJenkins commented May 19, 2015

Uh oh!

AmplabJenkins commented May 19, 2015

Uh oh!

SparkQA commented May 19, 2015

Uh oh!

andrewor14 commented May 19, 2015

Uh oh!

andrewor14 commented May 19, 2015

Uh oh!

SparkQA commented May 19, 2015

Uh oh!

AmplabJenkins commented May 19, 2015

Uh oh!

AmplabJenkins commented May 19, 2015

Uh oh!

AmplabJenkins commented May 20, 2015

Uh oh!

AmplabJenkins commented May 20, 2015

Uh oh!

SparkQA commented May 20, 2015

Uh oh!

SparkQA commented May 20, 2015

Uh oh!

AmplabJenkins commented May 20, 2015

Uh oh!

AmplabJenkins commented May 20, 2015

Uh oh!

AmplabJenkins commented May 20, 2015

Uh oh!

AmplabJenkins commented May 20, 2015

Uh oh!

SparkQA commented May 20, 2015

Uh oh!

SparkQA commented May 20, 2015

Uh oh!

AmplabJenkins commented May 20, 2015

Uh oh!

AmplabJenkins commented May 20, 2015

Uh oh!