Skip to content

Commit

Permalink
[SPARK-7288] Suppress compiler warnings due to use of sun.misc.Unsafe…
Browse files Browse the repository at this point in the history
…; 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
  • Loading branch information
JoshRosen authored and jeanlyn committed Jun 12, 2015
1 parent 2183a40 commit d779d2d
Show file tree
Hide file tree
Showing 7 changed files with 125 additions and 24 deletions.
11 changes: 11 additions & 0 deletions project/SparkBuild.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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")))
Expand Down
24 changes: 24 additions & 0 deletions unsafe/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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;
Expand All @@ -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();
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit d779d2d

Please sign in to comment.