You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by an...@apache.org on 2015/10/08 20:27:51 UTC
spark git commit: [SPARK-7527] [CORE] Fix createNullValue to return
the correct null values and REPL mode detection
Repository: spark
Updated Branches:
refs/heads/branch-1.4 e7c4346d0 -> e2ff49198
[SPARK-7527] [CORE] Fix createNullValue to return the correct null values and REPL mode detection
The root cause of SPARK-7527 is `createNullValue` returns an incompatible value `Byte(0)` for `char` and `boolean`.
This PR fixes it and corrects the class name of the main class, and also adds an unit test to demonstrate it.
Author: zsxwing <zs...@gmail.com>
Closes #6735 from zsxwing/SPARK-7527 and squashes the following commits:
bbdb271 [zsxwing] Use pattern match in createNullValue
b0a0e7e [zsxwing] Remove the noisy in the test output
903e269 [zsxwing] Remove the code for Utils.isInInterpreter == false
5f92dc1 [zsxwing] Fix createNullValue to return the correct null values and REPL mode detection
Project: http://git-wip-us.apache.org/repos/asf/spark/repo
Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/e2ff4919
Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/e2ff4919
Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/e2ff4919
Branch: refs/heads/branch-1.4
Commit: e2ff4919834fac5fa92c4fbb7e24078e04b81262
Parents: e7c4346
Author: zsxwing <zs...@gmail.com>
Authored: Wed Jun 10 13:22:52 2015 -0700
Committer: Andrew Or <an...@databricks.com>
Committed: Thu Oct 8 11:27:24 2015 -0700
----------------------------------------------------------------------
.../org/apache/spark/util/ClosureCleaner.scala | 40 ++++++++----------
.../scala/org/apache/spark/util/Utils.scala | 9 +---
.../apache/spark/util/ClosureCleanerSuite.scala | 44 ++++++++++++++++++++
3 files changed, 64 insertions(+), 29 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/spark/blob/e2ff4919/core/src/main/scala/org/apache/spark/util/ClosureCleaner.scala
----------------------------------------------------------------------
diff --git a/core/src/main/scala/org/apache/spark/util/ClosureCleaner.scala b/core/src/main/scala/org/apache/spark/util/ClosureCleaner.scala
index 6f2966b..305de4c 100644
--- a/core/src/main/scala/org/apache/spark/util/ClosureCleaner.scala
+++ b/core/src/main/scala/org/apache/spark/util/ClosureCleaner.scala
@@ -109,7 +109,14 @@ private[spark] object ClosureCleaner extends Logging {
private def createNullValue(cls: Class[_]): AnyRef = {
if (cls.isPrimitive) {
- new java.lang.Byte(0: Byte) // Should be convertible to any primitive type
+ cls match {
+ case java.lang.Boolean.TYPE => new java.lang.Boolean(false)
+ case java.lang.Character.TYPE => new java.lang.Character('\0')
+ case java.lang.Void.TYPE =>
+ // This should not happen because `Foo(void x) {}` does not compile.
+ throw new IllegalStateException("Unexpected void parameter in constructor")
+ case _ => new java.lang.Byte(0: Byte)
+ }
} else {
null
}
@@ -319,28 +326,17 @@ private[spark] object ClosureCleaner extends Logging {
private def instantiateClass(
cls: Class[_],
enclosingObject: AnyRef): AnyRef = {
- if (!Utils.isInInterpreter) {
- // This is a bona fide closure class, whose constructor has no effects
- // other than to set its fields, so use its constructor
- val cons = cls.getConstructors()(0)
- val params = cons.getParameterTypes.map(createNullValue).toArray
- if (enclosingObject != null) {
- params(0) = enclosingObject // First param is always enclosing object
- }
- return cons.newInstance(params: _*).asInstanceOf[AnyRef]
- } else {
- // Use reflection to instantiate object without calling constructor
- val rf = sun.reflect.ReflectionFactory.getReflectionFactory()
- val parentCtor = classOf[java.lang.Object].getDeclaredConstructor()
- val newCtor = rf.newConstructorForSerialization(cls, parentCtor)
- val obj = newCtor.newInstance().asInstanceOf[AnyRef]
- if (enclosingObject != null) {
- val field = cls.getDeclaredField("$outer")
- field.setAccessible(true)
- field.set(obj, enclosingObject)
- }
- obj
+ // Use reflection to instantiate object without calling constructor
+ val rf = sun.reflect.ReflectionFactory.getReflectionFactory()
+ val parentCtor = classOf[java.lang.Object].getDeclaredConstructor()
+ val newCtor = rf.newConstructorForSerialization(cls, parentCtor)
+ val obj = newCtor.newInstance().asInstanceOf[AnyRef]
+ if (enclosingObject != null) {
+ val field = cls.getDeclaredField("$outer")
+ field.setAccessible(true)
+ field.set(obj, enclosingObject)
}
+ obj
}
}
http://git-wip-us.apache.org/repos/asf/spark/blob/e2ff4919/core/src/main/scala/org/apache/spark/util/Utils.scala
----------------------------------------------------------------------
diff --git a/core/src/main/scala/org/apache/spark/util/Utils.scala b/core/src/main/scala/org/apache/spark/util/Utils.scala
index 615944e..6abc3ac 100644
--- a/core/src/main/scala/org/apache/spark/util/Utils.scala
+++ b/core/src/main/scala/org/apache/spark/util/Utils.scala
@@ -1685,15 +1685,10 @@ private[spark] object Utils extends Logging {
lazy val isInInterpreter: Boolean = {
try {
- val interpClass = classForName("spark.repl.Main")
+ val interpClass = classForName("org.apache.spark.repl.Main")
interpClass.getMethod("interp").invoke(null) != null
} catch {
- // Returning true seems to be a mistake.
- // Currently changing it to false causes tests failures in Streaming.
- // For a more detailed discussion, please, refer to
- // https://github.com/apache/spark/pull/5835#issuecomment-101042271 and subsequent comments.
- // Addressing this changed is tracked as https://issues.apache.org/jira/browse/SPARK-7527
- case _: ClassNotFoundException => true
+ case _: ClassNotFoundException => false
}
}
http://git-wip-us.apache.org/repos/asf/spark/blob/e2ff4919/core/src/test/scala/org/apache/spark/util/ClosureCleanerSuite.scala
----------------------------------------------------------------------
diff --git a/core/src/test/scala/org/apache/spark/util/ClosureCleanerSuite.scala b/core/src/test/scala/org/apache/spark/util/ClosureCleanerSuite.scala
index 70cd27b..1053c6c 100644
--- a/core/src/test/scala/org/apache/spark/util/ClosureCleanerSuite.scala
+++ b/core/src/test/scala/org/apache/spark/util/ClosureCleanerSuite.scala
@@ -121,6 +121,10 @@ class ClosureCleanerSuite extends SparkFunSuite {
expectCorrectException { TestUserClosuresActuallyCleaned.testSubmitJob(sc) }
}
}
+
+ test("createNullValue") {
+ new TestCreateNullValue().run()
+ }
}
// A non-serializable class we create in closures to make sure that we aren't
@@ -350,3 +354,43 @@ private object TestUserClosuresActuallyCleaned {
)
}
}
+
+class TestCreateNullValue {
+
+ var x = 5
+
+ def getX: Int = x
+
+ def run(): Unit = {
+ val bo: Boolean = true
+ val c: Char = '1'
+ val b: Byte = 1
+ val s: Short = 1
+ val i: Int = 1
+ val l: Long = 1
+ val f: Float = 1
+ val d: Double = 1
+
+ // Bring in all primitive types into the closure such that they become
+ // parameters of the closure constructor. This allows us to test whether
+ // null values are created correctly for each type.
+ val nestedClosure = () => {
+ if (s.toString == "123") { // Don't really output them to avoid noisy
+ println(bo)
+ println(c)
+ println(b)
+ println(s)
+ println(i)
+ println(l)
+ println(f)
+ println(d)
+ }
+
+ val closure = () => {
+ println(getX)
+ }
+ ClosureCleaner.clean(closure)
+ }
+ nestedClosure()
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org