Skip to content

Commit

Permalink
Cleanup unreachable Java 8 code flows (#17559)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Akshat-Jain
Akshat-Jain authored Dec 13, 2024
1 parent 24e5d8a commit a26e4c0
Show file tree
Hide file tree
Showing 9 changed files with 18 additions and 198 deletions.
11 changes: 1 addition & 10 deletions indexing-service/src/main/java/org/apache/druid/indexing/common/task/HadoopTask.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,16 +173,7 @@ public static ClassLoader buildClassLoader(final List<String> hadoopDependencyCo
localClassLoaderURLs.addAll(Arrays.asList(hadoopLoader.getURLs()));
}

ClassLoader parent = null;
if (JvmUtils.isIsJava9Compatible()) {
try {
// See also https://docs.oracle.com/en/java/javase/11/migrate/index.html#JSMIG-GUID-A868D0B9-026F-4D46-B979-901834343F9E
parent = (ClassLoader) ClassLoader.class.getMethod("getPlatformClassLoader").invoke(null);
}
catch (IllegalAccessException | InvocationTargetException | NoSuchMethodException e) {
throw new RuntimeException(e);
}
}
ClassLoader parent = ClassLoader.getPlatformClassLoader();
final ClassLoader classLoader = new URLClassLoader(
localClassLoaderURLs.toArray(new URL[0]),
parent
Expand Down
4 changes: 1 addition & 3 deletions indexing-service/src/main/java/org/apache/druid/indexing/overlord/ForkingTaskRunner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -244,9 +244,7 @@ public TaskStatus call()

command.add(config.getJavaCommand());

if (JvmUtils.majorVersion() >= 11) {
command.addAll(STRONG_ENCAPSULATION_PROPERTIES);
}
command.addAll(STRONG_ENCAPSULATION_PROPERTIES);

command.add("-cp");
command.add(taskClasspath);
Expand Down
10 changes: 2 additions & 8 deletions indexing-service/src/test/java/org/apache/druid/indexing/common/task/HadoopTaskTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import org.apache.druid.java.util.common.StringUtils;
import org.apache.druid.java.util.common.granularity.Granularity;
import org.apache.druid.timeline.DataSegment;
import org.apache.druid.utils.JvmUtils;
import org.easymock.EasyMock;
import org.joda.time.Interval;
import org.junit.Assert;
Expand Down Expand Up @@ -131,13 +130,8 @@ public TaskStatus runTask(TaskToolbox toolbox)

public static void assertClassLoaderIsSingular(ClassLoader classLoader)
{
if (JvmUtils.isIsJava9Compatible()) {
// See also https://docs.oracle.com/en/java/javase/11/migrate/index.html#JSMIG-GUID-A868D0B9-026F-4D46-B979-901834343F9E
Assert.assertEquals("PlatformClassLoader", classLoader.getParent().getClass().getSimpleName());
} else {
// This is a check against the current HadoopTask which creates a single URLClassLoader with null parent
Assert.assertNull(classLoader.getParent());
}
// See also https://docs.oracle.com/en/java/javase/11/migrate/index.html#JSMIG-GUID-A868D0B9-026F-4D46-B979-901834343F9E
Assert.assertEquals("PlatformClassLoader", classLoader.getParent().getClass().getSimpleName());
Assert.assertFalse(classLoader.getClass().getSimpleName().equals("ApplicationClassLoader"));
Assert.assertTrue(classLoader instanceof URLClassLoader);

Expand Down
3 changes: 3 additions & 0 deletions pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1666,6 +1666,9 @@
<ignore>sun.misc.Unsafe</ignore>
<!-- ignore java reflection polymorphic api signatures -->
<ignore>java.lang.invoke.MethodHandle</ignore>
<!-- For ignoring java.lang.ClassLoader#getPlatformClassLoader since that's present in java 9+.
Need to be added since animal sniffer is using JDK8 signature currently. -->
<ignore>java.lang.ClassLoader</ignore>
<!--
For the following java.nio.* classes, we get errors like: "Undefined reference: java.nio.ByteBuffer java.nio.ByteBuffer.clear()"
GitHub issue: https://github.com/mojohaus/animal-sniffer/issues/4
Expand Down
59 changes: 6 additions & 53 deletions processing/src/main/java/org/apache/druid/java/util/common/ByteBufferUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,14 @@
package org.apache.druid.java.util.common;

import org.apache.druid.collections.ResourceHolder;
import org.apache.druid.utils.JvmUtils;

import javax.annotation.Nullable;
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;
import java.lang.reflect.Method;
import java.nio.ByteBuffer;
import java.nio.MappedByteBuffer;
import java.util.Comparator;
import java.util.Objects;
import java.util.concurrent.atomic.AtomicBoolean;

/**
Expand Down Expand Up @@ -88,63 +85,19 @@ private static MethodHandle lookupUnmapMethodHandle()
{
final MethodHandles.Lookup lookup = MethodHandles.lookup();
try {
if (JvmUtils.isIsJava9Compatible()) {
return unmapJava9(lookup);
} else {
return unmapJava7Or8(lookup);
}
MethodHandle unmapper = lookup.findVirtual(
UnsafeUtils.theUnsafeClass(),
"invokeCleaner",
MethodType.methodType(void.class, ByteBuffer.class)
);
return unmapper.bindTo(UnsafeUtils.theUnsafe());
}
catch (ReflectiveOperationException | RuntimeException e1) {
throw new UnsupportedOperationException("Unmapping is not supported on this platform, because internal " +
"Java APIs are not compatible with this Druid version", e1);
}
}

/**
* NB: while Druid no longer support Java 7, this method would still work with that version as well.
*/
private static MethodHandle unmapJava7Or8(MethodHandles.Lookup lookup) throws ReflectiveOperationException
{
// "Compile" a MethodHandle that is roughly equivalent to the following lambda:
//
// (ByteBuffer buffer) -> {
// sun.misc.Cleaner cleaner = ((java.nio.DirectByteBuffer) byteBuffer).cleaner();
// if (nonNull(cleaner))
// cleaner.clean();
// else
// noop(cleaner); // the noop is needed because MethodHandles#guardWithTest always needs both if and else
// }
//
Class<?> directBufferClass = Class.forName("java.nio.DirectByteBuffer");
Method m = directBufferClass.getMethod("cleaner");
m.setAccessible(true);
MethodHandle directBufferCleanerMethod = lookup.unreflect(m);
Class<?> cleanerClass = directBufferCleanerMethod.type().returnType();
MethodHandle cleanMethod = lookup.findVirtual(cleanerClass, "clean", MethodType.methodType(void.class));
MethodHandle nonNullTest = lookup.findStatic(Objects.class, "nonNull",
MethodType.methodType(boolean.class, Object.class)
).asType(MethodType.methodType(boolean.class, cleanerClass));
MethodHandle noop = MethodHandles.dropArguments(MethodHandles.constant(
Void.class,
null
).asType(MethodType.methodType(void.class)), 0, cleanerClass);
MethodHandle unmapper = MethodHandles.filterReturnValue(
directBufferCleanerMethod,
MethodHandles.guardWithTest(nonNullTest, cleanMethod, noop)
).asType(MethodType.methodType(void.class, ByteBuffer.class));
return unmapper;
}

private static MethodHandle unmapJava9(MethodHandles.Lookup lookup) throws ReflectiveOperationException
{
MethodHandle unmapper = lookup.findVirtual(
UnsafeUtils.theUnsafeClass(),
"invokeCleaner",
MethodType.methodType(void.class, ByteBuffer.class)
);
return unmapper.bindTo(UnsafeUtils.theUnsafe());
}

/**
* Same as {@link ByteBuffer#allocateDirect(int)}, but returns a closeable {@link ResourceHolder} that
* frees the buffer upon close.
Expand Down
23 changes: 2 additions & 21 deletions processing/src/main/java/org/apache/druid/java/util/common/Cleaners.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@

package org.apache.druid.java.util.common;

import org.apache.druid.utils.JvmUtils;

import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;
Expand Down Expand Up @@ -62,19 +60,15 @@ private static Cleaner takeMeToTheCleaners()
{
final MethodHandles.Lookup lookup = MethodHandles.lookup();
try {
if (JvmUtils.isIsJava9Compatible()) {
return lookupCleanerJava9(lookup);
} else {
return lookupCleanerJava8(lookup);
}
return lookupCleaner(lookup);
}
catch (ReflectiveOperationException | RuntimeException e) {
throw new UnsupportedOperationException("Cleaning is not support on this platform, because internal " +
"Java APIs are not compatible with this Druid version", e);
}
}

private static Cleaner lookupCleanerJava9(MethodHandles.Lookup lookup) throws ReflectiveOperationException
private static Cleaner lookupCleaner(MethodHandles.Lookup lookup) throws ReflectiveOperationException
{
Class<?> cleaner = Class.forName("java.lang.ref.Cleaner");
Class<?> cleanable = Class.forName("java.lang.ref.Cleaner$Cleanable");
Expand All @@ -100,19 +94,6 @@ private static Cleaner lookupCleanerJava9(MethodHandles.Lookup lookup) throws Re
return new CleanerImpl(register, clean);
}

private static Cleaner lookupCleanerJava8(MethodHandles.Lookup lookup) throws ReflectiveOperationException
{
Class<?> cleaner = Class.forName("sun.misc.Cleaner");
MethodHandle register = lookup.findStatic(
cleaner,
"create",
MethodType.methodType(cleaner, Object.class, Runnable.class)
);

MethodHandle clean = lookup.findVirtual(cleaner, "clean", MethodType.methodType(void.class));
return new CleanerImpl(register, clean);
}

public static Cleanable register(Object object, Runnable runnable)
{
if (CLEANER == null) {
Expand Down
99 changes: 2 additions & 97 deletions processing/src/main/java/org/apache/druid/java/util/common/DefineClassUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,9 @@

package org.apache.druid.java.util.common;

import org.apache.druid.utils.JvmUtils;

import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;
import java.security.ProtectionDomain;

/**
* This utility class provides a thin runtime abstraction to pick between
Expand All @@ -44,11 +41,7 @@ public class DefineClassUtils
Exception exception = null;
try {
MethodHandles.Lookup lookup = MethodHandles.lookup();
if (JvmUtils.isIsJava9Compatible()) {
defineClass = defineClassJava9(lookup);
} else {
defineClass = defineClassJava8(lookup);
}
defineClass = getMethodHandle(lookup);
}
catch (ReflectiveOperationException | RuntimeException e) {
exception = e;
Expand All @@ -70,9 +63,8 @@ public class DefineClassUtils
* return targetClassLookup.defineClass(byteCode);
* }
*/
private static MethodHandle defineClassJava9(MethodHandles.Lookup lookup) throws ReflectiveOperationException
private static MethodHandle getMethodHandle(MethodHandles.Lookup lookup) throws ReflectiveOperationException
{

// this is getting meta
MethodHandle defineClass = lookup.unreflect(MethodHandles.Lookup.class.getMethod("defineClass", byte[].class));
MethodHandle privateLookupIn = lookup.findStatic(
Expand All @@ -94,93 +86,6 @@ private static MethodHandle defineClassJava9(MethodHandles.Lookup lookup) throws
return defineClass;
}

/**
* "Compile" a MethodHandle that is equivalent to:
*
* Class<?> defineClass(Class targetClass, byte[] byteCode, String className) {
* return Unsafe.defineClass(
* className,
* byteCode,
* 0,
* byteCode.length,
* targetClass.getClassLoader(),
* targetClass.getProtectionDomain()
* );
* }
*/
private static MethodHandle defineClassJava8(MethodHandles.Lookup lookup) throws ReflectiveOperationException
{
MethodHandle defineClass = lookup.findVirtual(
UnsafeUtils.theUnsafeClass(),
"defineClass",
MethodType.methodType(
Class.class,
String.class,
byte[].class,
int.class,
int.class,
ClassLoader.class,
ProtectionDomain.class
)
).bindTo(UnsafeUtils.theUnsafe());

MethodHandle getProtectionDomain = lookup.unreflect(Class.class.getMethod("getProtectionDomain"));
MethodHandle getClassLoader = lookup.unreflect(Class.class.getMethod("getClassLoader"));

// apply getProtectionDomain and getClassLoader to the targetClass, modifying the methodHandle as follows:
// defineClass = (String className, byte[] byteCode, int offset, int length, Class class1, Class class2) ->
// defineClass(className, byteCode, offset, length, class1.getClassLoader(), class2.getProtectionDomain())
defineClass = MethodHandles.filterArguments(defineClass, 5, getProtectionDomain);
defineClass = MethodHandles.filterArguments(defineClass, 4, getClassLoader);

// duplicate the last argument to apply the methods above to the same class, modifying the methodHandle as follows:
// defineClass = (String className, byte[] byteCode, int offset, int length, Class targetClass) ->
// defineClass(className, byteCode, offset, length, targetClass, targetClass)
defineClass = MethodHandles.permuteArguments(
defineClass,
MethodType.methodType(Class.class, String.class, byte[].class, int.class, int.class, Class.class),
0, 1, 2, 3, 4, 4
);

// set offset argument to 0, modifying the methodHandle as follows:
// defineClass = (String className, byte[] byteCode, int length, Class targetClass) ->
// defineClass(className, byteCode, 0, length, targetClass)
defineClass = MethodHandles.insertArguments(defineClass, 2, (int) 0);

// JDK8 does not implement MethodHandles.arrayLength, so we have to roll our own
MethodHandle arrayLength = lookup.findStatic(
lookup.lookupClass(),
"getArrayLength",
MethodType.methodType(int.class, byte[].class)
);

// apply arrayLength to the length argument, modifying the methodHandle as follows:
// defineClass = (String className, byte[] byteCode1, byte[] byteCode2, Class targetClass) ->
// defineClass(className, byteCode1, byteCode2.length, targetClass)
defineClass = MethodHandles.filterArguments(defineClass, 2, arrayLength);

// duplicate the byteCode argument and reorder to match JDK9 signature, modifying the methodHandle as follows:
// defineClass = (Class targetClass, byte[] byteCode, String className) ->
// defineClass(className, byteCode, byteCode, targetClass)
defineClass = MethodHandles.permuteArguments(
defineClass,
MethodType.methodType(Class.class, Class.class, byte[].class, String.class),
2, 1, 1, 0
);

return defineClass;
}

/**
* This method is referenced in Java 8 using method handle, therefore it is not actually unused, and shouldn't be
* removed (till Java 8 is supported)
*/
@SuppressWarnings("unused") // method is referenced and used in defineClassJava8
static int getArrayLength(byte[] bytes)
{
return bytes.length;
}

public static Class defineClass(
Class<?> targetClass,
byte[] byteCode,
Expand Down
5 changes: 0 additions & 5 deletions processing/src/main/java/org/apache/druid/utils/JvmUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,6 @@ public static int majorVersion()
return MAJOR_VERSION;
}

public static boolean isIsJava9Compatible()
{
return MAJOR_VERSION >= 9;
}

public static RuntimeInfo getRuntimeInfo()
{
return RUNTIME_INFO;
Expand Down
2 changes: 1 addition & 1 deletion processing/src/main/java/org/apache/druid/utils/RuntimeInfo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public long getFreeHeapSizeBytes()
public long getDirectMemorySizeBytes()
{
try {
Class<?> vmClass = Class.forName(JvmUtils.majorVersion() >= 9 ? "jdk.internal.misc.VM" : "sun.misc.VM");
Class<?> vmClass = Class.forName("jdk.internal.misc.VM");
Object maxDirectMemoryObj = vmClass.getMethod("maxDirectMemory").invoke(null);

if (maxDirectMemoryObj == null || !(maxDirectMemoryObj instanceof Number)) {
Expand Down

0 comments on commit a26e4c0

Please sign in to comment.