Skip to content

Cleanup unreachable Java 8 code flows #17559

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kgyrtkirk merged 3 commits into from
Dec 13, 2024
Merged

Cleanup unreachable Java 8 code flows #17559

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0e387ff
Cleanup unreachable Java 8 code flows
Akshat-Jain Dec 12, 2024
5c2145d
Address review comment
Akshat-Jain Dec 12, 2024
d0475ed
Ignore java.lang.ClassLoader for animal-sniffer check
Akshat-Jain Dec 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 1 addition & 10 deletions indexing-service/src/main/java/org/apache/druid/indexing/common/task/HadoopTask.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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