This is an automated email from the ASF dual-hosted git repository.
kgyrtkirk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/master by this push:
new a26e4c0e061 Cleanup unreachable Java 8 code flows (#17559)
a26e4c0e061 is described below
commit a26e4c0e06107f290e573f51ef46f374cea8f826
Author: Akshat Jain <[email protected]>
AuthorDate: Fri Dec 13 19:54:21 2024 +0530
Cleanup unreachable Java 8 code flows (#17559)
---
.../druid/indexing/common/task/HadoopTask.java | 11 +--
.../druid/indexing/overlord/ForkingTaskRunner.java | 4 +-
.../druid/indexing/common/task/HadoopTaskTest.java | 10 +--
pom.xml | 3 +
.../druid/java/util/common/ByteBufferUtils.java | 59 ++-----------
.../apache/druid/java/util/common/Cleaners.java | 23 +----
.../druid/java/util/common/DefineClassUtils.java | 99 +---------------------
.../main/java/org/apache/druid/utils/JvmUtils.java | 5 --
.../java/org/apache/druid/utils/RuntimeInfo.java | 2 +-
9 files changed, 18 insertions(+), 198 deletions(-)
diff --git
a/indexing-service/src/main/java/org/apache/druid/indexing/common/task/HadoopTask.java
b/indexing-service/src/main/java/org/apache/druid/indexing/common/task/HadoopTask.java
index 201024540e1..a15b77b90d1 100644
---
a/indexing-service/src/main/java/org/apache/druid/indexing/common/task/HadoopTask.java
+++
b/indexing-service/src/main/java/org/apache/druid/indexing/common/task/HadoopTask.java
@@ -173,16 +173,7 @@ public abstract class HadoopTask extends
AbstractBatchIndexTask
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
diff --git
a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/ForkingTaskRunner.java
b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/ForkingTaskRunner.java
index 7e4dd6c39c2..1a34c76e33f 100644
---
a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/ForkingTaskRunner.java
+++
b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/ForkingTaskRunner.java
@@ -244,9 +244,7 @@ public class ForkingTaskRunner
command.add(config.getJavaCommand());
- if (JvmUtils.majorVersion() >= 11) {
- command.addAll(STRONG_ENCAPSULATION_PROPERTIES);
- }
+ command.addAll(STRONG_ENCAPSULATION_PROPERTIES);
command.add("-cp");
command.add(taskClasspath);
diff --git
a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/HadoopTaskTest.java
b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/HadoopTaskTest.java
index 130258e85d4..741208dd0db 100644
---
a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/HadoopTaskTest.java
+++
b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/HadoopTaskTest.java
@@ -30,7 +30,6 @@ import
org.apache.druid.indexing.common.config.TaskConfigBuilder;
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;
@@ -131,13 +130,8 @@ public class HadoopTaskTest
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);
diff --git a/pom.xml b/pom.xml
index 3ee7600d543..3f80196334a 100644
--- a/pom.xml
+++ b/pom.xml
@@ -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
diff --git
a/processing/src/main/java/org/apache/druid/java/util/common/ByteBufferUtils.java
b/processing/src/main/java/org/apache/druid/java/util/common/ByteBufferUtils.java
index fb67c323834..696f0b629c9 100644
---
a/processing/src/main/java/org/apache/druid/java/util/common/ByteBufferUtils.java
+++
b/processing/src/main/java/org/apache/druid/java/util/common/ByteBufferUtils.java
@@ -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;
/**
@@ -88,11 +85,12 @@ public class ByteBufferUtils
{
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 " +
@@ -100,51 +98,6 @@ public class ByteBufferUtils
}
}
- /**
- * 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.
diff --git
a/processing/src/main/java/org/apache/druid/java/util/common/Cleaners.java
b/processing/src/main/java/org/apache/druid/java/util/common/Cleaners.java
index 2de6a3e140d..2fa4ebdfafb 100644
--- a/processing/src/main/java/org/apache/druid/java/util/common/Cleaners.java
+++ b/processing/src/main/java/org/apache/druid/java/util/common/Cleaners.java
@@ -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;
@@ -62,11 +60,7 @@ public class Cleaners
{
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 " +
@@ -74,7 +68,7 @@ public class Cleaners
}
}
- 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");
@@ -100,19 +94,6 @@ public class Cleaners
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) {
diff --git
a/processing/src/main/java/org/apache/druid/java/util/common/DefineClassUtils.java
b/processing/src/main/java/org/apache/druid/java/util/common/DefineClassUtils.java
index d79c517ae3d..5e3bd6626f7 100644
---
a/processing/src/main/java/org/apache/druid/java/util/common/DefineClassUtils.java
+++
b/processing/src/main/java/org/apache/druid/java/util/common/DefineClassUtils.java
@@ -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
@@ -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;
@@ -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(
@@ -94,93 +86,6 @@ public class DefineClassUtils
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,
diff --git a/processing/src/main/java/org/apache/druid/utils/JvmUtils.java
b/processing/src/main/java/org/apache/druid/utils/JvmUtils.java
index 493ac901420..696a15aaee2 100644
--- a/processing/src/main/java/org/apache/druid/utils/JvmUtils.java
+++ b/processing/src/main/java/org/apache/druid/utils/JvmUtils.java
@@ -71,11 +71,6 @@ public class JvmUtils
return MAJOR_VERSION;
}
- public static boolean isIsJava9Compatible()
- {
- return MAJOR_VERSION >= 9;
- }
-
public static RuntimeInfo getRuntimeInfo()
{
return RUNTIME_INFO;
diff --git a/processing/src/main/java/org/apache/druid/utils/RuntimeInfo.java
b/processing/src/main/java/org/apache/druid/utils/RuntimeInfo.java
index 86571eff945..115787cfd66 100644
--- a/processing/src/main/java/org/apache/druid/utils/RuntimeInfo.java
+++ b/processing/src/main/java/org/apache/druid/utils/RuntimeInfo.java
@@ -50,7 +50,7 @@ public class RuntimeInfo
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)) {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
