You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by zsxwing <gi...@git.apache.org> on 2016/09/12 21:58:13 UTC
[GitHub] spark pull request #15065: [SPARK-17463][Core]Add necessary memory barrier f...
GitHub user zsxwing opened a pull request:
https://github.com/apache/spark/pull/15065
[SPARK-17463][Core]Add necessary memory barrier for accumulators
## What changes were proposed in this pull request?
Added `volatile` for fields that will be read in the heartbeat thread. Without them, the worse case is the user cannot see any metric updates until a task finishes.
Unfortunately, there will be a performance regression comparing to Spark 2.0.0. Note: in Spark 1.6, accumulators have a volatile value, so there should not be any performance regression comparing to 1.6.
To reduce the performance lost caused by `volatile`, there are two alternative ways:
1. Use `AtomicXXX.lazySet` to update a metric value. The eventually set should be enough for metrics. There are some comparison numbers in http://psy-lob-saw.blogspot.com/2012/12/atomiclazyset-is-performance-win-for.html . However, this will increase the metric object size a lot.
2. Use `AtomicLongFieldUpdater` to avoid the overhead of AtomicLong. See: http://normanmaurer.me/blog/2013/10/28/Lesser-known-concurrent-classes-Part-1/
. The problem of this approach is we need to rewrite codes using Java because we cannot add static codes in a class in Scala. The following codes will fail with `java.lang.IllegalAccessException: Class Foo$ can not access a member of class Foo with modifiers "private volatile"`:
```Scala
import java.util.concurrent.atomic.AtomicLongFieldUpdater
class Foo {
@volatile var x: Long = 0
}
object Foo extends App {
classOf[Foo].getDeclaredField("x").setAccessible(true) // newUpdater doesn't use this
val field = AtomicLongFieldUpdater.newUpdater(classOf[Foo], "x")
}
```
## How was this patch tested?
Jenkins tests
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zsxwing/spark fix-accmu
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/15065.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 #15065
----
commit 4c6bb0b92b55ca5368b2e2752704fc197f9be4ad
Author: Shixiong Zhu <sh...@databricks.com>
Date: 2016-09-12T20:51:53Z
Add necessary memory barrier for accumulators
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #15065: [SPARK-17463][Core]Add memory barriers for accumu...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing closed the pull request at:
https://github.com/apache/spark/pull/15065
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15065: [SPARK-17463][Core]Add necessary memory barrier for accu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15065
**[Test build #65282 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/65282/consoleFull)** for PR 15065 at commit [`4c6bb0b`](https://github.com/apache/spark/commit/4c6bb0b92b55ca5368b2e2752704fc197f9be4ad).
* This patch **fails PySpark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15065: [SPARK-17463][Core]Add memory barriers for accumulators
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15065
**[Test build #65286 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/65286/consoleFull)** for PR 15065 at commit [`c6fb896`](https://github.com/apache/spark/commit/c6fb896b9e694d99eec91d5ebc367cd936eb90fa).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15065: [SPARK-17463][Core]Add necessary memory barrier for accu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15065
**[Test build #65282 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/65282/consoleFull)** for PR 15065 at commit [`4c6bb0b`](https://github.com/apache/spark/commit/4c6bb0b92b55ca5368b2e2752704fc197f9be4ad).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15065: [SPARK-17463][Core]Add necessary memory barrier for accu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/15065
**[Test build #65286 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/65286/consoleFull)** for PR 15065 at commit [`c6fb896`](https://github.com/apache/spark/commit/c6fb896b9e694d99eec91d5ebc367cd936eb90fa).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15065: [SPARK-17463][Core]Add memory barriers for accumulators
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15065
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/65286/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15065: [SPARK-17463][Core]Add necessary memory barrier for accu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15065
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/65282/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15065: [SPARK-17463][Core]Add memory barriers for accumulators
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the issue:
https://github.com/apache/spark/pull/15065
I'm going to close this one for now since I found there was a huge regression for `MiscBenchmark."filter & aggregate without group"`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15065: [SPARK-17463][Core]Add necessary memory barrier for accu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15065
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15065: [SPARK-17463][Core]Add memory barriers for accumulators
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15065
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org