From 07a86205f9efc43ea1ec5edb97c21c32abe7fb8a Mon Sep 17 00:00:00 2001
From: Josh Rosen <joshrosen@databricks.com>
Date: Thu, 30 Apr 2015 15:21:00 -0700
Subject: [PATCH] [SPARK-7288] Suppress compiler warnings due to use of
sun.misc.Unsafe; add facade in front of Unsafe; remove use of
Unsafe.setMemory
This patch suppresses compiler warnings due to our use of `sun.misc.Unsafe` (introduced in #5725). These warnings can only be suppressed via the `-XDignore.symbol.file` javac flag; the `SuppressWarnings` annotation won't work for these.
In order to restrict uses of this compiler flag to the `unsafe` module, I placed a facade in front of `Unsafe` so that other modules won't call it directly. This facade also will also help us to avoid accidental usage of deprecated Unsafe methods or methods that aren't supported in Java 6.
I also removed an unnecessary use of `Unsafe.setMemory`, which isn't present in certain versions of Java 6, and excluded the new `unsafe` module from Javadoc.
Author: Josh Rosen <joshrosen@databricks.com>
Closes #5814 from JoshRosen/unsafe-compiler-warnings-fixes and squashes the following commits:
9e8c483 [Josh Rosen] Exclude new unsafe module from Javadoc
ba75ecf [Josh Rosen] Only apply -XDignore.symbol.file flag in unsafe project.
7403345 [Josh Rosen] Put facade in front of Unsafe.
50230c0 [Josh Rosen] Remove usage of Unsafe.setMemory
96d41c9 [Josh Rosen] Use -XDignore.symbol.file to suppress warnings about sun.misc.Unsafe usage
---
project/SparkBuild.scala | 11 +++
unsafe/pom.xml | 24 +++++
.../spark/unsafe/PlatformDependent.java | 93 +++++++++++++++++--
.../spark/unsafe/map/BytesToBytesMap.java | 9 +-
.../spark/unsafe/memory/MemoryBlock.java | 8 --
.../spark/unsafe/bitset/BitSetSuite.java | 2 +-
.../map/AbstractBytesToBytesMapSuite.java | 2 +-
7 files changed, 125 insertions(+), 24 deletions(-)
diff --git a/project/SparkBuild.scala b/project/SparkBuild.scala
index 4bfa8cede7..a93aa12ce1 100644
--- a/project/SparkBuild.scala
+++ b/project/SparkBuild.scala
@@ -163,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))
@@ -216,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
}
@@ -424,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")))
diff --git a/unsafe/pom.xml b/unsafe/pom.xml
index 8901d77591..5b0733206b 100644
--- a/unsafe/pom.xml
+++ b/unsafe/pom.xml
@@ -65,5 +65,29 @@
<build>
<outputDirectory>target/scala-${scala.binary.version}/classes</outputDirectory>
<testOutputDirectory>target/scala-${scala.binary.version}/test-classes</testOutputDirectory>
+ <pluginManagement>
+ <plugins>
+ <plugin>
+ <groupId>net.alchim31.maven</groupId>
+ <artifactId>scala-maven-plugin</artifactId>
+ <configuration>
+ <javacArgs>
+ <!-- This option is needed to suppress warnings from sun.misc.Unsafe usage -->
+ <javacArg>-XDignore.symbol.file</javacArg>
+ </javacArgs>
+ </configuration>
+ </plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <configuration>
+ <compilerArgs>
+ <!-- This option is needed to suppress warnings from sun.misc.Unsafe usage -->
+ <arg>-XDignore.symbol.file</arg>
+ </compilerArgs>
+ </configuration>
+ </plugin>
+ </plugins>
+ </pluginManagement>
</build>
</project>
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 91b2f9aa43..24b2892098 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 @@ import sun.misc.Unsafe;
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 @@ public final class PlatformDependent {
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 @@ public final class PlatformDependent {
* 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 85b64c0833..4e5ebc402b 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 final class BytesToBytesMap {
// 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(
@@ -429,7 +429,7 @@ public final class BytesToBytesMap {
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 final class BytesToBytesMap {
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<MemoryBlock> dataPagesIterator = dataPages.iterator();
@@ -535,7 +535,6 @@ public final class BytesToBytesMap {
// 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 0beb743e56..3dc82d8c2e 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 class MemoryBlock extends MemoryLocation {
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 4bf132fd40..e3a824e29b 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
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 9038cf567f..7a5c0622d1 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 abstract class AbstractBytesToBytesMapSuite {
private static byte[] getByteArray(MemoryLocation loc, int size) {
final byte[] arr = new byte[size];
- PlatformDependent.UNSAFE.copyMemory(
+ PlatformDependent.copyMemory(
loc.getBaseObject(),
loc.getBaseOffset(),
arr,
--
GitLab