divijvaidya commented on code in PR #12163:
URL: https://github.com/apache/kafka/pull/12163#discussion_r873603513
##########
clients/src/main/java/org/apache/kafka/common/utils/Checksums.java:
##########
@@ -40,11 +55,41 @@ public static void update(Checksum checksum, ByteBuffer
buffer, int length) {
public static void update(Checksum checksum, ByteBuffer buffer, int
offset, int length) {
if (buffer.hasArray()) {
checksum.update(buffer.array(), buffer.position() +
buffer.arrayOffset() + offset, length);
- } else {
- int start = buffer.position() + offset;
- for (int i = start; i < start + length; i++)
- checksum.update(buffer.get(i));
+ return;
+ }
+ if (BYTE_BUFFER_UPDATE != null && buffer.isDirect()) {
+ final int oldPosition = buffer.position();
Review Comment:
alternatively, use buffer.mark() inside try and buffer.reset() in the
finally.
##########
clients/src/main/java/org/apache/kafka/common/utils/Checksums.java:
##########
@@ -30,6 +33,18 @@
*/
public final class Checksums {
+ private static final MethodHandle BYTE_BUFFER_UPDATE;
+
+ static {
+ MethodHandle byteBufferUpdate = null;
+ try {
+ byteBufferUpdate =
MethodHandles.publicLookup().findVirtual(Checksum.class, "update",
MethodType.methodType(void.class, ByteBuffer.class));
+ } catch (Throwable silent) {
Review Comment:
I understand that you are keeping it here for backward compatibility with
older versions (8) of JDK which don't have this API. But we should ideally want
to throw an error when we remove support for JDK 8 from Kafka.
Could we do one of the following two options here:
1. (preferred) Use
[isJava9Compatible](https://docs.gradle.org/current/javadoc/org/gradle/api/JavaVersion.html#isJava9Compatible--)
to decide whether we want to propagate the exception here or fail silently OR
2. add a TODO here so that we can remember to propagate exception when
support for JDK 8 is removed?
Also, a comment explaining why are we consuming the exception would be nice.
##########
clients/src/main/java/org/apache/kafka/common/utils/Checksums.java:
##########
@@ -40,11 +55,41 @@ public static void update(Checksum checksum, ByteBuffer
buffer, int length) {
public static void update(Checksum checksum, ByteBuffer buffer, int
offset, int length) {
Review Comment:
A java doc here which clarifies that this method leaves the buffer as it was
received would be nice addition here.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
