From 96d41c94ceee04bc645d5dff452c4398ea70286a Mon Sep 17 00:00:00 2001 From: Josh Rosen Date: Thu, 30 Apr 2015 11:18:03 -0700 Subject: [PATCH 1/5] Use -XDignore.symbol.file to suppress warnings about sun.misc.Unsafe usage --- pom.xml | 6 ++++++ project/SparkBuild.scala | 4 +++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index c85c5feeaf383..ec9c3656dbafc 100644 --- a/pom.xml +++ b/pom.xml @@ -1162,6 +1162,8 @@ ${java.version} -target ${java.version} + + -XDignore.symbol.file @@ -1184,6 +1186,10 @@ UTF-8 1024m true + + + -XDignore.symbol.file + diff --git a/project/SparkBuild.scala b/project/SparkBuild.scala index b7dbcd9bc562a..cc3f3d59ad83f 100644 --- a/project/SparkBuild.scala +++ b/project/SparkBuild.scala @@ -140,7 +140,9 @@ object SparkBuild extends PomBuild { javacOptions in (Compile, doc) ++= { val Array(major, minor, _) = System.getProperty("java.version").split("\\.", 3) if (major.toInt >= 1 && minor.toInt >= 8) Seq("-Xdoclint:all", "-Xdoclint:-missing") else Seq.empty - } + }, + // This option is needed to suppress warnings from sun.misc.Unsafe usage + javacOptions in Compile += "-XDignore.symbol.file" ) def enable(settings: Seq[Setting[_]])(projectRef: ProjectRef) = { From 50230c0d9f5cbd143c7183201973a8462962c6e9 Mon Sep 17 00:00:00 2001 From: Josh Rosen Date: Thu, 30 Apr 2015 11:32:05 -0700 Subject: [PATCH 2/5] Remove usage of Unsafe.setMemory --- .../java/org/apache/spark/unsafe/map/BytesToBytesMap.java | 5 ++--- .../java/org/apache/spark/unsafe/memory/MemoryBlock.java | 8 -------- .../java/org/apache/spark/unsafe/bitset/BitSetSuite.java | 2 +- 3 files changed, 3 insertions(+), 12 deletions(-) diff --git a/unsafe/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java b/unsafe/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java index 85b64c0833803..e1ffa0dbb037b 100644 --- a/unsafe/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java +++ b/unsafe/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java @@ -429,7 +429,7 @@ public void putNewKey( private void allocate(int capacity) { capacity = Math.max((int) Math.min(Integer.MAX_VALUE, nextPowerOf2(capacity)), 64); longArray = new LongArray(memoryManager.allocate(capacity * 8 * 2)); - bitset = new BitSet(memoryManager.allocate(capacity / 8).zero()); + bitset = new BitSet(MemoryBlock.fromLongArray(new long[capacity / 64])); this.growthThreshold = (int) (capacity * loadFactor); this.mask = capacity - 1; @@ -447,7 +447,7 @@ public void free() { longArray = null; } if (bitset != null) { - memoryManager.free(bitset.memoryBlock()); + // The bitset's heap memory isn't managed by a memory manager, so no need to free it here. bitset = null; } Iterator dataPagesIterator = dataPages.iterator(); @@ -535,7 +535,6 @@ private void growAndRehash() { // Deallocate the old data structures. memoryManager.free(oldLongArray.memoryBlock()); - memoryManager.free(oldBitSet.memoryBlock()); if (enablePerfMetrics) { timeSpentResizingNs += System.nanoTime() - resizeStartTime; } diff --git a/unsafe/src/main/java/org/apache/spark/unsafe/memory/MemoryBlock.java b/unsafe/src/main/java/org/apache/spark/unsafe/memory/MemoryBlock.java index 0beb743e5644e..3dc82d8c2eb39 100644 --- a/unsafe/src/main/java/org/apache/spark/unsafe/memory/MemoryBlock.java +++ b/unsafe/src/main/java/org/apache/spark/unsafe/memory/MemoryBlock.java @@ -46,14 +46,6 @@ public long size() { return length; } - /** - * Clear the contents of this memory block. Returns `this` to facilitate chaining. - */ - public MemoryBlock zero() { - PlatformDependent.UNSAFE.setMemory(obj, offset, length, (byte) 0); - return this; - } - /** * Creates a memory block pointing to the memory used by the long array. */ diff --git a/unsafe/src/test/java/org/apache/spark/unsafe/bitset/BitSetSuite.java b/unsafe/src/test/java/org/apache/spark/unsafe/bitset/BitSetSuite.java index 4bf132fd4053e..e3a824e29b768 100644 --- a/unsafe/src/test/java/org/apache/spark/unsafe/bitset/BitSetSuite.java +++ b/unsafe/src/test/java/org/apache/spark/unsafe/bitset/BitSetSuite.java @@ -27,7 +27,7 @@ public class BitSetSuite { private static BitSet createBitSet(int capacity) { assert capacity % 64 == 0; - return new BitSet(MemoryBlock.fromLongArray(new long[capacity / 64]).zero()); + return new BitSet(MemoryBlock.fromLongArray(new long[capacity / 64])); } @Test From 7403345975b1ea8f541a1732753f63c972722b79 Mon Sep 17 00:00:00 2001 From: Josh Rosen Date: Thu, 30 Apr 2015 12:40:06 -0700 Subject: [PATCH 3/5] Put facade in front of Unsafe. --- .../spark/unsafe/PlatformDependent.java | 93 +++++++++++++++++-- .../spark/unsafe/map/BytesToBytesMap.java | 4 +- .../map/AbstractBytesToBytesMapSuite.java | 2 +- 3 files changed, 87 insertions(+), 12 deletions(-) diff --git a/unsafe/src/main/java/org/apache/spark/unsafe/PlatformDependent.java b/unsafe/src/main/java/org/apache/spark/unsafe/PlatformDependent.java index 91b2f9aa43921..24b2892098059 100644 --- a/unsafe/src/main/java/org/apache/spark/unsafe/PlatformDependent.java +++ b/unsafe/src/main/java/org/apache/spark/unsafe/PlatformDependent.java @@ -23,7 +23,82 @@ public final class PlatformDependent { - public static final Unsafe UNSAFE; + /** + * Facade in front of {@link sun.misc.Unsafe}, used to avoid directly exposing Unsafe outside of + * this package. This also lets us aovid accidental use of deprecated methods or methods that + * aren't present in Java 6. + */ + public static final class UNSAFE { + + private UNSAFE() { } + + public static int getInt(Object object, long offset) { + return _UNSAFE.getInt(object, offset); + } + + public static void putInt(Object object, long offset, int value) { + _UNSAFE.putInt(object, offset, value); + } + + public static boolean getBoolean(Object object, long offset) { + return _UNSAFE.getBoolean(object, offset); + } + + public static void putBoolean(Object object, long offset, boolean value) { + _UNSAFE.putBoolean(object, offset, value); + } + + public static byte getByte(Object object, long offset) { + return _UNSAFE.getByte(object, offset); + } + + public static void putByte(Object object, long offset, byte value) { + _UNSAFE.putByte(object, offset, value); + } + + public static short getShort(Object object, long offset) { + return _UNSAFE.getShort(object, offset); + } + + public static void putShort(Object object, long offset, short value) { + _UNSAFE.putShort(object, offset, value); + } + + public static long getLong(Object object, long offset) { + return _UNSAFE.getLong(object, offset); + } + + public static void putLong(Object object, long offset, long value) { + _UNSAFE.putLong(object, offset, value); + } + + public static float getFloat(Object object, long offset) { + return _UNSAFE.getFloat(object, offset); + } + + public static void putFloat(Object object, long offset, float value) { + _UNSAFE.putFloat(object, offset, value); + } + + public static double getDouble(Object object, long offset) { + return _UNSAFE.getDouble(object, offset); + } + + public static void putDouble(Object object, long offset, double value) { + _UNSAFE.putDouble(object, offset, value); + } + + public static long allocateMemory(long size) { + return _UNSAFE.allocateMemory(size); + } + + public static void freeMemory(long address) { + _UNSAFE.freeMemory(address); + } + + } + + private static final Unsafe _UNSAFE; public static final int BYTE_ARRAY_OFFSET; @@ -48,13 +123,13 @@ public final class PlatformDependent { } catch (Throwable cause) { unsafe = null; } - UNSAFE = unsafe; + _UNSAFE = unsafe; - if (UNSAFE != null) { - BYTE_ARRAY_OFFSET = UNSAFE.arrayBaseOffset(byte[].class); - INT_ARRAY_OFFSET = UNSAFE.arrayBaseOffset(int[].class); - LONG_ARRAY_OFFSET = UNSAFE.arrayBaseOffset(long[].class); - DOUBLE_ARRAY_OFFSET = UNSAFE.arrayBaseOffset(double[].class); + if (_UNSAFE != null) { + BYTE_ARRAY_OFFSET = _UNSAFE.arrayBaseOffset(byte[].class); + INT_ARRAY_OFFSET = _UNSAFE.arrayBaseOffset(int[].class); + LONG_ARRAY_OFFSET = _UNSAFE.arrayBaseOffset(long[].class); + DOUBLE_ARRAY_OFFSET = _UNSAFE.arrayBaseOffset(double[].class); } else { BYTE_ARRAY_OFFSET = 0; INT_ARRAY_OFFSET = 0; @@ -71,7 +146,7 @@ static public void copyMemory( long length) { while (length > 0) { long size = Math.min(length, UNSAFE_COPY_THRESHOLD); - UNSAFE.copyMemory(src, srcOffset, dst, dstOffset, size); + _UNSAFE.copyMemory(src, srcOffset, dst, dstOffset, size); length -= size; srcOffset += size; dstOffset += size; @@ -82,6 +157,6 @@ static public void copyMemory( * Raises an exception bypassing compiler checks for checked exceptions. */ public static void throwException(Throwable t) { - UNSAFE.throwException(t); + _UNSAFE.throwException(t); } } diff --git a/unsafe/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java b/unsafe/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java index e1ffa0dbb037b..4e5ebc402be35 100644 --- a/unsafe/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java +++ b/unsafe/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java @@ -401,11 +401,11 @@ public void putNewKey( // Copy the key PlatformDependent.UNSAFE.putLong(pageBaseObject, keySizeOffsetInPage, keyLengthBytes); - PlatformDependent.UNSAFE.copyMemory( + PlatformDependent.copyMemory( keyBaseObject, keyBaseOffset, pageBaseObject, keyDataOffsetInPage, keyLengthBytes); // Copy the value PlatformDependent.UNSAFE.putLong(pageBaseObject, valueSizeOffsetInPage, valueLengthBytes); - PlatformDependent.UNSAFE.copyMemory( + PlatformDependent.copyMemory( valueBaseObject, valueBaseOffset, pageBaseObject, valueDataOffsetInPage, valueLengthBytes); final long storedKeyAddress = memoryManager.encodePageNumberAndOffset( diff --git a/unsafe/src/test/java/org/apache/spark/unsafe/map/AbstractBytesToBytesMapSuite.java b/unsafe/src/test/java/org/apache/spark/unsafe/map/AbstractBytesToBytesMapSuite.java index 9038cf567f1e2..7a5c0622d1ffb 100644 --- a/unsafe/src/test/java/org/apache/spark/unsafe/map/AbstractBytesToBytesMapSuite.java +++ b/unsafe/src/test/java/org/apache/spark/unsafe/map/AbstractBytesToBytesMapSuite.java @@ -57,7 +57,7 @@ public void tearDown() { private static byte[] getByteArray(MemoryLocation loc, int size) { final byte[] arr = new byte[size]; - PlatformDependent.UNSAFE.copyMemory( + PlatformDependent.copyMemory( loc.getBaseObject(), loc.getBaseOffset(), arr, From ba75ecfe329684c5809c4ccb263f9da0b7bd231e Mon Sep 17 00:00:00 2001 From: Josh Rosen Date: Thu, 30 Apr 2015 12:52:36 -0700 Subject: [PATCH 4/5] Only apply -XDignore.symbol.file flag in unsafe project. --- pom.xml | 6 ------ project/SparkBuild.scala | 14 +++++++++++--- unsafe/pom.xml | 24 ++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/pom.xml b/pom.xml index ec9c3656dbafc..c85c5feeaf383 100644 --- a/pom.xml +++ b/pom.xml @@ -1162,8 +1162,6 @@ ${java.version} -target ${java.version} - - -XDignore.symbol.file @@ -1186,10 +1184,6 @@ UTF-8 1024m true - - - -XDignore.symbol.file - diff --git a/project/SparkBuild.scala b/project/SparkBuild.scala index cc3f3d59ad83f..4bd48603787b5 100644 --- a/project/SparkBuild.scala +++ b/project/SparkBuild.scala @@ -140,9 +140,7 @@ object SparkBuild extends PomBuild { javacOptions in (Compile, doc) ++= { val Array(major, minor, _) = System.getProperty("java.version").split("\\.", 3) if (major.toInt >= 1 && minor.toInt >= 8) Seq("-Xdoclint:all", "-Xdoclint:-missing") else Seq.empty - }, - // This option is needed to suppress warnings from sun.misc.Unsafe usage - javacOptions in Compile += "-XDignore.symbol.file" + } ) def enable(settings: Seq[Setting[_]])(projectRef: ProjectRef) = { @@ -165,6 +163,9 @@ object SparkBuild extends PomBuild { x => enable(MimaBuild.mimaSettings(sparkHome, x))(x) } + /* Unsafe settings */ + enable(Unsafe.settings)(unsafe) + /* Enable Assembly for all assembly projects */ assemblyProjects.foreach(enable(Assembly.settings)) @@ -218,6 +219,13 @@ object SparkBuild extends PomBuild { } +object Unsafe { + lazy val settings = Seq( + // This option is needed to suppress warnings from sun.misc.Unsafe usage + javacOptions in Compile += "-XDignore.symbol.file" + ) +} + object Flume { lazy val settings = sbtavro.SbtAvro.avroSettings } diff --git a/unsafe/pom.xml b/unsafe/pom.xml index 8901d77591932..5b0733206b2bc 100644 --- a/unsafe/pom.xml +++ b/unsafe/pom.xml @@ -65,5 +65,29 @@ target/scala-${scala.binary.version}/classes target/scala-${scala.binary.version}/test-classes + + + + net.alchim31.maven + scala-maven-plugin + + + + -XDignore.symbol.file + + + + + org.apache.maven.plugins + maven-compiler-plugin + + + + -XDignore.symbol.file + + + + + From 9e8c4834aed1692081026a28aae96e822c0161db Mon Sep 17 00:00:00 2001 From: Josh Rosen Date: Thu, 30 Apr 2015 13:13:28 -0700 Subject: [PATCH 5/5] Exclude new unsafe module from Javadoc --- project/SparkBuild.scala | 1 + 1 file changed, 1 insertion(+) diff --git a/project/SparkBuild.scala b/project/SparkBuild.scala index 4bd48603787b5..f0c53f6d1dd91 100644 --- a/project/SparkBuild.scala +++ b/project/SparkBuild.scala @@ -434,6 +434,7 @@ object Unidoc { .map(_.filterNot(_.getCanonicalPath.contains("org/apache/spark/network"))) .map(_.filterNot(_.getCanonicalPath.contains("org/apache/spark/shuffle"))) .map(_.filterNot(_.getCanonicalPath.contains("org/apache/spark/executor"))) + .map(_.filterNot(_.getCanonicalPath.contains("org/apache/spark/unsafe"))) .map(_.filterNot(_.getCanonicalPath.contains("python"))) .map(_.filterNot(_.getCanonicalPath.contains("org/apache/spark/util/collection"))) .map(_.filterNot(_.getCanonicalPath.contains("org/apache/spark/sql/catalyst")))