chia7712 commented on code in PR #17441:
URL: https://github.com/apache/kafka/pull/17441#discussion_r1799093306
##########
clients/src/main/java/org/apache/kafka/common/utils/ByteBufferUnmapper.java:
##########
@@ -87,40 +82,13 @@ public static void unmap(String resourceDescription,
ByteBuffer buffer) throws I
private static MethodHandle lookupUnmapMethodHandle() {
final MethodHandles.Lookup lookup = lookup();
try {
- if (Java.IS_JAVA9_COMPATIBLE)
- return unmapJava9(lookup);
- else
- return unmapJava7Or8(lookup);
+ return unmapJava9(lookup);
} catch (ReflectiveOperationException | RuntimeException e1) {
throw new UnsupportedOperationException("Unmapping is not
supported on this platform, because internal " +
"Java APIs are not compatible with this Kafka version", e1);
}
}
- 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(void.class));
- MethodHandle nonNullTest = lookup.findStatic(ByteBufferUnmapper.class,
"nonNull",
- methodType(boolean.class,
Object.class)).asType(methodType(boolean.class, cleanerClass));
- MethodHandle noop = dropArguments(constant(Void.class,
null).asType(methodType(void.class)), 0, cleanerClass);
- return filterReturnValue(directBufferCleanerMethod,
guardWithTest(nonNullTest, cleanMethod, noop))
- .asType(methodType(void.class, ByteBuffer.class));
- }
-
private static MethodHandle unmapJava9(MethodHandles.Lookup lookup) throws
ReflectiveOperationException {
Review Comment:
Could you please inline this method?
##########
clients/src/main/java/org/apache/kafka/common/utils/Crc32C.java:
##########
@@ -34,14 +34,7 @@
*/
public final class Crc32C {
- private static final ChecksumFactory CHECKSUM_FACTORY;
-
- static {
- if (Java.IS_JAVA9_COMPATIBLE)
- CHECKSUM_FACTORY = new Java9ChecksumFactory();
- else
- CHECKSUM_FACTORY = new PureJavaChecksumFactory();
- }
+ private static final ChecksumFactory CHECKSUM_FACTORY = new
Java9ChecksumFactory();
Review Comment:
Please inline `Java9ChecksumFactory`
##########
clients/src/main/java/org/apache/kafka/common/utils/Java.java:
##########
@@ -36,10 +36,6 @@ static Version parseVersion(String versionString) {
return new Version(majorVersion, minorVersion);
}
- // Having these as static final provides the best opportunity for compiler
optimization
- public static final boolean IS_JAVA9_COMPATIBLE =
VERSION.isJava9Compatible();
Review Comment:
Please remove `VERSION` also
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
