[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 apache#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 <[email protected]>

Closes apache#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

Author: JoshRosen

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")))

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>

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);
   }
 }

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(
@@ -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<MemoryBlock> 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;
     }

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.
    */

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

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,