svn merge -c 1442386 from trunk for HADOOP-9252. In StringUtils, humanReadableInt(..) has a race condition and the synchronization of limitDecimalTo2(double) can be avoided.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/branch-2@1442387 13f79535-47bb-0310-9956-ffa450edef68

hadoop-common-project/hadoop-common/CHANGES.txt

@@ -280,6 +280,9 @@ Release 2.0.3-alpha - Unreleased
     HADOOP-9124. SortedMapWritable violates contract of Map interface for
     equals() and hashCode(). (Surenkumar Nihalani via tomwhite)
 
+    HADOOP-9252. In StringUtils, humanReadableInt(..) has a race condition and
+    the synchronization of limitDecimalTo2(double) can be avoided.  (szetszwo)
+
 Release 2.0.2-alpha - 2012-09-07 
 
   INCOMPATIBLE CHANGES

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/FsUsage.java

@@ -48,7 +48,7 @@ class FsUsage extends FsCommand {
   
   protected String formatSize(long size) {
     return humanReadable
-        ? StringUtils.humanReadableInt(size)
+        ? StringUtils.TraditionalBinaryPrefix.long2String(size, "", 1)
         : String.valueOf(size);
   }
 

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Ls.java

@@ -67,7 +67,7 @@ class Ls extends FsCommand {
   protected boolean humanReadable = false;
   protected String formatSize(long size) {
     return humanReadable
-      ? StringUtils.humanReadableInt(size)
+      ? StringUtils.TraditionalBinaryPrefix.long2String(size, "", 1)
       : String.valueOf(size);
   }
 

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/StringUtils.java

@@ -23,8 +23,6 @@ import java.io.StringWriter;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.text.DateFormat;
-import java.text.DecimalFormat;
-import java.text.NumberFormat;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -34,12 +32,13 @@ import java.util.List;
 import java.util.Locale;
 import java.util.StringTokenizer;
 
-import com.google.common.net.InetAddresses;
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.classification.InterfaceStability;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.net.NetUtils;
 
+import com.google.common.net.InetAddresses;
+
 /**
  * General string utils
  */
@@ -52,13 +51,6 @@ public class StringUtils {
    */
   public static final int SHUTDOWN_HOOK_PRIORITY = 0;
 
-  private static final DecimalFormat decimalFormat;
-  static {
-          NumberFormat numberFormat = NumberFormat.getNumberInstance(Locale.ENGLISH);
-          decimalFormat = (DecimalFormat) numberFormat;
-          decimalFormat.applyPattern("#.##");
-  }
-
   /**
    * Make a string representation of the exception.
    * @param e The exception to stringify
@@ -87,50 +79,33 @@ public class StringUtils {
     }
     return fullHostname;
   }
-
-  private static DecimalFormat oneDecimal = new DecimalFormat("0.0");
   
   /**
    * Given an integer, return a string that is in an approximate, but human 
    * readable format. 
-   * It uses the bases 'k', 'm', and 'g' for 1024, 1024**2, and 1024**3.
    * @param number the number to format
    * @return a human readable form of the integer
+   *
+   * @deprecated use {@link TraditionalBinaryPrefix#long2String(long, String, int)}.
    */
+  @Deprecated
   public static String humanReadableInt(long number) {
-    long absNumber = Math.abs(number);
-    double result = number;
-    String suffix = "";
-    if (absNumber < 1024) {
-      // since no division has occurred, don't format with a decimal point
-      return String.valueOf(number);
-    } else if (absNumber < 1024 * 1024) {
-      result = number / 1024.0;
-      suffix = "k";
-    } else if (absNumber < 1024 * 1024 * 1024) {
-      result = number / (1024.0 * 1024);
-      suffix = "m";
-    } else {
-      result = number / (1024.0 * 1024 * 1024);
-      suffix = "g";
-    }
-    return oneDecimal.format(result) + suffix;
+    return TraditionalBinaryPrefix.long2String(number, "", 1);
   }
-  
+
+  /** The same as String.format(Locale.ENGLISH, format, objects). */
+  public static String format(final String format, final Object... objects) {
+    return String.format(Locale.ENGLISH, format, objects);
+  }
+
   /**
    * Format a percentage for presentation to the user.
-   * @param done the percentage to format (0.0 to 1.0)
-   * @param digits the number of digits past the decimal point
+   * @param fraction the percentage as a fraction, e.g. 0.1 = 10%
+   * @param decimalPlaces the number of decimal places
    * @return a string representation of the percentage
    */
-  public static String formatPercent(double done, int digits) {
-    DecimalFormat percentFormat = new DecimalFormat("0.00%");
-    double scale = Math.pow(10.0, digits+2);
-    double rounded = Math.floor(done * scale);
-    percentFormat.setDecimalSeparatorAlwaysShown(false);
-    percentFormat.setMinimumFractionDigits(digits);
-    percentFormat.setMaximumFractionDigits(digits);
-    return percentFormat.format(rounded / scale);
+  public static String formatPercent(double fraction, int decimalPlaces) {
+    return format("%." + decimalPlaces + "f%%", fraction*100);
   }
   
   /**
@@ -165,7 +140,7 @@ public class StringUtils {
     }
     StringBuilder s = new StringBuilder(); 
     for(int i = start; i < end; i++) {
-      s.append(String.format("%02x", bytes[i]));
+      s.append(format("%02x", bytes[i]));
     }
     return s.toString();
   }
@@ -630,18 +605,22 @@ public class StringUtils {
    * TraditionalBinaryPrefix symbol are case insensitive. 
    */
   public static enum TraditionalBinaryPrefix {
-    KILO(1024),
-    MEGA(KILO.value << 10),
-    GIGA(MEGA.value << 10),
-    TERA(GIGA.value << 10),
-    PETA(TERA.value << 10),
-    EXA(PETA.value << 10);
+    KILO(10),
+    MEGA(KILO.bitShift + 10),
+    GIGA(MEGA.bitShift + 10),
+    TERA(GIGA.bitShift + 10),
+    PETA(TERA.bitShift + 10),
+    EXA (PETA.bitShift + 10);
 
     public final long value;
     public final char symbol;
+    public final int bitShift;
+    public final long bitMask;
 
-    TraditionalBinaryPrefix(long value) {
-      this.value = value;
+    private TraditionalBinaryPrefix(int bitShift) {
+      this.bitShift = bitShift;
+      this.value = 1L << bitShift;
+      this.bitMask = this.value - 1L;
       this.symbol = toString().charAt(0);
     }
 
@@ -692,8 +671,58 @@ public class StringUtils {
         return num * prefix;
       }
     }
+
+    /**
+     * Convert a long integer to a string with traditional binary prefix.
+     * 
+     * @param n the value to be converted
+     * @param unit The unit, e.g. "B" for bytes.
+     * @param decimalPlaces The number of decimal places.
+     * @return a string with traditional binary prefix.
+     */
+    public static String long2String(long n, String unit, int decimalPlaces) {
+      if (unit == null) {
+        unit = "";
+      }
+      //take care a special case
+      if (n == Long.MIN_VALUE) {
+        return "-8 " + EXA.symbol + unit;
+      }
+
+      final StringBuilder b = new StringBuilder();
+      //take care negative numbers
+      if (n < 0) {
+        b.append('-');
+        n = -n;
+      }
+      if (n < KILO.value) {
+        //no prefix
+        b.append(n);
+        return (unit.isEmpty()? b: b.append(" ").append(unit)).toString();
+      } else {
+        //find traditional binary prefix
+        int i = 0;
+        for(; i < values().length && n >= values()[i].value; i++);
+        TraditionalBinaryPrefix prefix = values()[i - 1];
+
+        if ((n & prefix.bitMask) == 0) {
+          //exact division
+          b.append(n >> prefix.bitShift);
+        } else {
+          final String  format = "%." + decimalPlaces + "f";
+          String s = format(format, n/(double)prefix.value);
+          //check a special rounding up case
+          if (s.startsWith("1024")) {
+            prefix = values()[i];
+            s = format(format, n/(double)prefix.value);
+          }
+          b.append(s);
+        }
+        return b.append(' ').append(prefix.symbol).append(unit).toString();
+      }
+    }
   }
-  
+
     /**
      * Escapes HTML Special characters present in the string.
      * @param string
@@ -731,32 +760,16 @@ public class StringUtils {
     }
 
   /**
-   * Return an abbreviated English-language desc of the byte length
+   * @return a byte description of the given long interger value.
    */
   public static String byteDesc(long len) {
-    double val = 0.0;
-    String ending = "";
-    if (len < 1024 * 1024) {
-      val = (1.0 * len) / 1024;
-      ending = " KB";
-    } else if (len < 1024 * 1024 * 1024) {
-      val = (1.0 * len) / (1024 * 1024);
-      ending = " MB";
-    } else if (len < 1024L * 1024 * 1024 * 1024) {
-      val = (1.0 * len) / (1024 * 1024 * 1024);
-      ending = " GB";
-    } else if (len < 1024L * 1024 * 1024 * 1024 * 1024) {
-      val = (1.0 * len) / (1024L * 1024 * 1024 * 1024);
-      ending = " TB";
-    } else {
-      val = (1.0 * len) / (1024L * 1024 * 1024 * 1024 * 1024);
-      ending = " PB";
-    }
-    return limitDecimalTo2(val) + ending;
-  }
-
-  public static synchronized String limitDecimalTo2(double d) {
-    return decimalFormat.format(d);
+    return TraditionalBinaryPrefix.long2String(len, "B", 2);
+  }
+
+  /** @deprecated use StringUtils.format("%.2f", d). */
+  @Deprecated
+  public static String limitDecimalTo2(double d) {
+    return format("%.2f", d);
   }
   
   /**

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestStringUtils.java

@@ -18,6 +18,8 @@
 
 package org.apache.hadoop.util;
 
+import static org.apache.hadoop.util.StringUtils.TraditionalBinaryPrefix.long2String;
+import static org.apache.hadoop.util.StringUtils.TraditionalBinaryPrefix.string2long;
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.fail;
@@ -26,6 +28,7 @@ import java.util.ArrayList;
 import java.util.List;
 
 import org.apache.hadoop.test.UnitTestcaseTimeLimit;
+import org.apache.hadoop.util.StringUtils.TraditionalBinaryPrefix;
 import org.junit.Test;
 
 public class TestStringUtils extends UnitTestcaseTimeLimit {
@@ -134,45 +137,34 @@ public class TestStringUtils extends UnitTestcaseTimeLimit {
   
   @Test
   public void testTraditionalBinaryPrefix() throws Exception {
+    //test string2long(..)
     String[] symbol = {"k", "m", "g", "t", "p", "e"};
     long m = 1024;
     for(String s : symbol) {
-      assertEquals(0, StringUtils.TraditionalBinaryPrefix.string2long(0 + s));
-      assertEquals(m, StringUtils.TraditionalBinaryPrefix.string2long(1 + s));
+      assertEquals(0, string2long(0 + s));
+      assertEquals(m, string2long(1 + s));
       m *= 1024;
     }
     
-    assertEquals(0L, StringUtils.TraditionalBinaryPrefix.string2long("0"));
-    assertEquals(1024L, StringUtils.TraditionalBinaryPrefix.string2long("1k"));
-    assertEquals(-1024L, StringUtils.TraditionalBinaryPrefix.string2long("-1k"));
-    assertEquals(1259520L,
-        StringUtils.TraditionalBinaryPrefix.string2long("1230K"));
-    assertEquals(-1259520L,
-        StringUtils.TraditionalBinaryPrefix.string2long("-1230K"));
-    assertEquals(104857600L,
-        StringUtils.TraditionalBinaryPrefix.string2long("100m"));
-    assertEquals(-104857600L,
-        StringUtils.TraditionalBinaryPrefix.string2long("-100M"));
-    assertEquals(956703965184L,
-        StringUtils.TraditionalBinaryPrefix.string2long("891g"));
-    assertEquals(-956703965184L,
-        StringUtils.TraditionalBinaryPrefix.string2long("-891G"));
-    assertEquals(501377302265856L,
-        StringUtils.TraditionalBinaryPrefix.string2long("456t"));
-    assertEquals(-501377302265856L,
-        StringUtils.TraditionalBinaryPrefix.string2long("-456T"));
-    assertEquals(11258999068426240L,
-        StringUtils.TraditionalBinaryPrefix.string2long("10p"));
-    assertEquals(-11258999068426240L,
-        StringUtils.TraditionalBinaryPrefix.string2long("-10P"));
-    assertEquals(1152921504606846976L,
-        StringUtils.TraditionalBinaryPrefix.string2long("1e"));
-    assertEquals(-1152921504606846976L,
-        StringUtils.TraditionalBinaryPrefix.string2long("-1E"));
+    assertEquals(0L, string2long("0"));
+    assertEquals(1024L, string2long("1k"));
+    assertEquals(-1024L, string2long("-1k"));
+    assertEquals(1259520L, string2long("1230K"));
+    assertEquals(-1259520L, string2long("-1230K"));
+    assertEquals(104857600L, string2long("100m"));
+    assertEquals(-104857600L, string2long("-100M"));
+    assertEquals(956703965184L, string2long("891g"));
+    assertEquals(-956703965184L, string2long("-891G"));
+    assertEquals(501377302265856L, string2long("456t"));
+    assertEquals(-501377302265856L, string2long("-456T"));
+    assertEquals(11258999068426240L, string2long("10p"));
+    assertEquals(-11258999068426240L, string2long("-10P"));
+    assertEquals(1152921504606846976L, string2long("1e"));
+    assertEquals(-1152921504606846976L, string2long("-1E"));
 
     String tooLargeNumStr = "10e";
     try {
-      StringUtils.TraditionalBinaryPrefix.string2long(tooLargeNumStr);
+      string2long(tooLargeNumStr);
       fail("Test passed for a number " + tooLargeNumStr + " too large");
     } catch (IllegalArgumentException e) {
       assertEquals(tooLargeNumStr + " does not fit in a Long", e.getMessage());
@@ -180,7 +172,7 @@ public class TestStringUtils extends UnitTestcaseTimeLimit {
 
     String tooSmallNumStr = "-10e";
     try {
-      StringUtils.TraditionalBinaryPrefix.string2long(tooSmallNumStr);
+      string2long(tooSmallNumStr);
       fail("Test passed for a number " + tooSmallNumStr + " too small");
     } catch (IllegalArgumentException e) {
       assertEquals(tooSmallNumStr + " does not fit in a Long", e.getMessage());
@@ -189,7 +181,7 @@ public class TestStringUtils extends UnitTestcaseTimeLimit {
     String invalidFormatNumStr = "10kb";
     char invalidPrefix = 'b';
     try {
-      StringUtils.TraditionalBinaryPrefix.string2long(invalidFormatNumStr);
+      string2long(invalidFormatNumStr);
       fail("Test passed for a number " + invalidFormatNumStr
           + " has invalid format");
     } catch (IllegalArgumentException e) {
@@ -199,6 +191,74 @@ public class TestStringUtils extends UnitTestcaseTimeLimit {
           e.getMessage());
     }
 
+    //test long2string(..)
+    assertEquals("0", long2String(0, null, 2));
+    for(int decimalPlace = 0; decimalPlace < 2; decimalPlace++) {
+      for(int n = 1; n < TraditionalBinaryPrefix.KILO.value; n++) {
+        assertEquals(n + "", long2String(n, null, decimalPlace));
+        assertEquals(-n + "", long2String(-n, null, decimalPlace));
+      }
+      assertEquals("1 K", long2String(1L << 10, null, decimalPlace));
+      assertEquals("-1 K", long2String(-1L << 10, null, decimalPlace));
+    }
+
+    assertEquals("8.00 E", long2String(Long.MAX_VALUE, null, 2));
+    assertEquals("8.00 E", long2String(Long.MAX_VALUE - 1, null, 2));
+    assertEquals("-8 E", long2String(Long.MIN_VALUE, null, 2));
+    assertEquals("-8.00 E", long2String(Long.MIN_VALUE + 1, null, 2));
+
+    final String[] zeros = {" ", ".0 ", ".00 "};
+    for(int decimalPlace = 0; decimalPlace < zeros.length; decimalPlace++) {
+      final String trailingZeros = zeros[decimalPlace]; 
+
+      for(int e = 11; e < Long.SIZE - 1; e++) {
+        final TraditionalBinaryPrefix p
+            = TraditionalBinaryPrefix.values()[e/10 - 1]; 
+  
+        { // n = 2^e
+          final long n = 1L << e;
+          final String expected = (n/p.value) + " " + p.symbol;
+          assertEquals("n=" + n, expected, long2String(n, null, 2));
+        }
+  
+        { // n = 2^e + 1
+          final long n = (1L << e) + 1;
+          final String expected = (n/p.value) + trailingZeros + p.symbol;
+          assertEquals("n=" + n, expected, long2String(n, null, decimalPlace));
+        }
+  
+        { // n = 2^e - 1
+          final long n = (1L << e) - 1;
+          final String expected = ((n+1)/p.value) + trailingZeros + p.symbol;
+          assertEquals("n=" + n, expected, long2String(n, null, decimalPlace));
+        }
+      }
+    }
+
+    assertEquals("1.50 K", long2String(3L << 9, null, 2));
+    assertEquals("1.5 K", long2String(3L << 9, null, 1));
+    assertEquals("1.50 M", long2String(3L << 19, null, 2));
+    assertEquals("2 M", long2String(3L << 19, null, 0));
+    assertEquals("3 G", long2String(3L << 30, null, 2));
+
+    // test byteDesc(..)
+    assertEquals("0 B", StringUtils.byteDesc(0));
+    assertEquals("-100 B", StringUtils.byteDesc(-100));
+    assertEquals("1 KB", StringUtils.byteDesc(1024));
+    assertEquals("1.50 KB", StringUtils.byteDesc(3L << 9));
+    assertEquals("1.50 MB", StringUtils.byteDesc(3L << 19));
+    assertEquals("3 GB", StringUtils.byteDesc(3L << 30));
+    
+    // test formatPercent(..)
+    assertEquals("10%", StringUtils.formatPercent(0.1, 0));
+    assertEquals("10.0%", StringUtils.formatPercent(0.1, 1));
+    assertEquals("10.00%", StringUtils.formatPercent(0.1, 2));
+
+    assertEquals("1%", StringUtils.formatPercent(0.00543, 0));
+    assertEquals("0.5%", StringUtils.formatPercent(0.00543, 1));
+    assertEquals("0.54%", StringUtils.formatPercent(0.00543, 2));
+    assertEquals("0.543%", StringUtils.formatPercent(0.00543, 3));
+    assertEquals("0.5430%", StringUtils.formatPercent(0.00543, 4));
   }
 
   @Test
@@ -313,10 +373,9 @@ public class TestStringUtils extends UnitTestcaseTimeLimit {
         }
         long et = System.nanoTime();
         if (outer > 3) {
-          System.out.println(
-            (useOurs ? "StringUtils impl" : "Java impl") +
-            " #" + outer + ":" +
-            (et - st)/1000000 + "ms");
+          System.out.println( (useOurs ? "StringUtils impl" : "Java impl")
+              + " #" + outer + ":" + (et - st)/1000000 + "ms, components="
+              + components  );
         }
       }
     }