HADOOP-3046. Fix the raw comparators for Text and BytesWritables

to use the provided length rather than recompute it.


git-svn-id: https://svn.apache.org/repos/asf/hadoop/core/trunk@641454 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -361,6 +361,9 @@ Trunk (unreleased changes)
     HADOOP-2983. [HOD] Fixes the problem - local_fqdn() returns None when 
     gethostbyname_ex doesnt return any FQDNs. (Craig Macdonald via ddas)
 
+    HADOOP-3046. Fix the raw comparators for Text and BytesWritables
+    to use the provided length rather than recompute it. (omalley)
+
 Release 0.16.2 - Unreleased
 
   BUG FIXES

src/java/org/apache/hadoop/io/BytesWritable.java

@@ -29,6 +29,7 @@ import java.io.DataOutput;
  * buffer. The sort order is the same as memcmp.
  */
 public class BytesWritable implements WritableComparable {
+  private static final int LENGTH_BYTES = 4;
   private int size;
   private byte[] bytes;
   
@@ -194,9 +195,8 @@ public class BytesWritable implements WritableComparable {
      */
     public int compare(byte[] b1, int s1, int l1,
                        byte[] b2, int s2, int l2) {
-      int size1 = readInt(b1, s1);
-      int size2 = readInt(b2, s2);
-      return compareBytes(b1, s1+4, size1, b2, s2+4, size2);
+      return compareBytes(b1, s1+LENGTH_BYTES, l1-LENGTH_BYTES, 
+                          b2, s2+LENGTH_BYTES, l2-LENGTH_BYTES);
     }
   }
   

src/java/org/apache/hadoop/io/Text.java

@@ -306,15 +306,9 @@ public class Text implements WritableComparable {
 
     public int compare(byte[] b1, int s1, int l1,
                        byte[] b2, int s2, int l2) {
-      try {
-        int n1 = readVInt(b1, s1);
-        int n2 = readVInt(b2, s2);
-        return compareBytes(b1, s1+WritableUtils.getVIntSize(n1), n1, 
-                            b2, s2+WritableUtils.getVIntSize(n2), n2);
-      }catch(IOException e) {
-        LOG.warn(e);
-        throw new RuntimeException(e);
-      }
+      int n1 = WritableUtils.decodeVIntSize(b1[s1]);
+      int n2 = WritableUtils.decodeVIntSize(b2[s2]);
+      return compareBytes(b1, s1+n1, l1-n1, b2, s2+n2, l2-n2);
     }
   }
 

src/java/org/apache/hadoop/io/WritableUtils.java

@@ -321,19 +321,18 @@ public final class WritableUtils  {
    * @return deserialized long from stream.
    */
   public static long readVLong(DataInput stream) throws IOException {
-    int len = stream.readByte();
-    if (len >= -112) {
-      return len;
+    byte firstByte = stream.readByte();
+    int len = decodeVIntSize(firstByte);
+    if (len == 1) {
+      return firstByte;
     }
-    boolean isNegative = (len < -120);
-    len = isNegative ? -(len + 120) : -(len + 112);
     long i = 0;
-    for (int idx = 0; idx < len; idx++) {
+    for (int idx = 0; idx < len-1; idx++) {
       byte b = stream.readByte();
       i = i << 8;
       i = i | (b & 0xFF);
     }
-    return (isNegative ? (i ^ -1L) : i);
+    return (isNegativeVInt(firstByte) ? (i ^ -1L) : i);
   }
 
   /**
@@ -345,7 +344,29 @@ public final class WritableUtils  {
   public static int readVInt(DataInput stream) throws IOException {
     return (int) readVLong(stream);
   }
-  
+ 
+  /**
+   * Given the first byte of a vint/vlong, determine the sign
+   * @param value the first byte
+   * @return is the value negative
+   */
+  public static boolean isNegativeVInt(byte value) {
+    return value < -120 || (value >= -112 && value < 0);
+  }
+
+  /**
+   * Parse the first byte of a vint/vlong to determine the number of bytes
+   * @param value the first byte of the vint/vlong
+   * @return the total number of bytes (1 to 9)
+   */
+  public static int decodeVIntSize(byte value) {
+    if (value >= -112) {
+      return 1;
+    } else if (value < -120) {
+      return -119 - value;
+    }
+    return -111 - value;
+  }
 
   /**
    * Get the encoded length if an integer is stored in a variable-length format
@@ -356,21 +377,13 @@ public final class WritableUtils  {
       return 1;
     }
       
-    int len = -112;
     if (i < 0) {
       i ^= -1L; // take one's complement'
-      len = -120;
-    }
-      
-    long tmp = i;
-    while (tmp != 0) {
-      tmp = tmp >> 8;
-      len--;
     }
-      
-    len = (len < -120) ? -(len + 120) : -(len + 112);
-      
-    return len+1;
+    // find the number of bytes with non-leading zeros
+    int dataBits = Long.SIZE - Long.numberOfLeadingZeros(i);
+    // find the number of data bytes + length byte
+    return (dataBits + 7) / 8 + 1;
   }
   /**
    * Read an Enum value from DataInput, Enums are read and written 

src/test/org/apache/hadoop/io/TestWritableUtils.java

@@ -0,0 +1,65 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.io;
+
+import java.io.IOException;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+
+import junit.framework.TestCase;
+
+public class TestWritableUtils extends TestCase {
+  private static final Log LOG = LogFactory.getLog(TestWritableUtils.class);
+
+  public static void testValue(int val, int vintlen) throws IOException {
+    DataOutputBuffer buf = new DataOutputBuffer();
+    DataInputBuffer inbuf = new DataInputBuffer();
+    WritableUtils.writeVInt(buf, val);
+    if (LOG.isDebugEnabled()) {
+      LOG.debug("Value = " + val);
+      BytesWritable printer = new BytesWritable();
+      printer.set(buf.getData(), 0, buf.getLength());
+      LOG.debug("Buffer = " + printer);
+    }
+    inbuf.reset(buf.getData(), 0, buf.getLength());
+    assertEquals(val, WritableUtils.readVInt(inbuf));
+    assertEquals(vintlen, buf.getLength());
+    assertEquals(vintlen, WritableUtils.getVIntSize(val));
+    assertEquals(vintlen, WritableUtils.decodeVIntSize(buf.getData()[0]));
+  }
+
+  public static void testVInt() throws Exception {
+    testValue(12, 1);
+    testValue(127, 1);
+    testValue(-112, 1);
+    testValue(-113, 2);
+    testValue(-128, 2);
+    testValue(128, 2);
+    testValue(-129, 2);
+    testValue(255, 2);
+    testValue(-256, 2);
+    testValue(256, 3);
+    testValue(-257, 3);
+    testValue(65535, 3);
+    testValue(-65536, 3);
+    testValue(65536, 4);
+    testValue(-65537, 4);
+  }
+}