[GitHub] [kafka] ijuma commented on a diff in pull request #13312: KAFKA-14766: Improve performance of VarInt encoding and decoding

["ijuma (via GitHub)" <gi...@apache.org>]

ijuma commented on code in PR #13312:
URL: https://github.com/apache/kafka/pull/13312#discussion_r1120217103


##########
clients/src/main/java/org/apache/kafka/common/utils/ByteUtils.java:
##########
@@ -292,29 +415,66 @@ public static double readDouble(ByteBuffer buffer) {
      * @param buffer The output to write to
      */
     public static void writeUnsignedVarint(int value, ByteBuffer buffer) {

Review Comment:
   What does netty do for this? Similar to what we're doing here or something else?



##########
clients/src/main/java/org/apache/kafka/common/utils/ByteUtils.java:
##########
@@ -150,17 +151,32 @@ public static void writeUnsignedIntLE(byte[] buffer, int offset, int value) {
      * @throws IllegalArgumentException if variable-length value does not terminate after 5 bytes have been read
      */
     public static int readUnsignedVarint(ByteBuffer buffer) {

Review Comment:
   Shall we have a comment indicating that we borrowed the implementation from Netty with a link to the relevant file?



##########
clients/src/main/java/org/apache/kafka/common/utils/ByteUtils.java:
##########
@@ -227,17 +259,62 @@ public static int readVarint(DataInput in) throws IOException {
      * @throws IOException              if {@link DataInput} throws {@link IOException}
      */
     public static long readVarlong(DataInput in) throws IOException {
-        long value = 0L;
-        int i = 0;
-        long b;
-        while (((b = in.readByte()) & 0x80) != 0) {
-            value |= (b & 0x7f) << i;
-            i += 7;
-            if (i > 63)
-                throw illegalVarlongException(value);
+        long raw = readUnsignedVarlong(in);
+        return (raw >>> 1) ^ -(raw & 1);
+    }
+
+    private static long readUnsignedVarlong(DataInput in) throws IOException {
+        byte tmp = in.readByte();

Review Comment:
   I saw a link to the Netty implementation for varint32, is there one for varint64 too?



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org