From 412062c0bae1fc6eb8225022ab3a9f299a15f021 Mon Sep 17 00:00:00 2001 From: Tsz-wo Sze Date: Mon, 4 Feb 2013 21:43:27 +0000 Subject: [PATCH] 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/CHANGES.txt | 3 + .../org/apache/hadoop/fs/shell/FsUsage.java | 2 +- .../java/org/apache/hadoop/fs/shell/Ls.java | 2 +- .../org/apache/hadoop/util/StringUtils.java | 159 ++++++++++-------- .../apache/hadoop/util/TestStringUtils.java | 131 +++++++++++---- 5 files changed, 186 insertions(+), 111 deletions(-) diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index fff598b16ed..106fce474ef 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/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 diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/FsUsage.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/FsUsage.java index 0d73f76e666..a99494573fd 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/FsUsage.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/FsUsage.java @@ -48,7 +48,7 @@ public static void registerCommands(CommandFactory factory) { protected String formatSize(long size) { return humanReadable - ? StringUtils.humanReadableInt(size) + ? StringUtils.TraditionalBinaryPrefix.long2String(size, "", 1) : String.valueOf(size); } diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Ls.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Ls.java index 289adea8d34..76192e73b4a 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Ls.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/Ls.java @@ -67,7 +67,7 @@ public static void registerCommands(CommandFactory factory) { protected boolean humanReadable = false; protected String formatSize(long size) { return humanReadable - ? StringUtils.humanReadableInt(size) + ? StringUtils.TraditionalBinaryPrefix.long2String(size, "", 1) : String.valueOf(size); } diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/StringUtils.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/StringUtils.java index 36bc2262678..3d674169a28 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/StringUtils.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/StringUtils.java @@ -23,8 +23,6 @@ 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.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 static String simpleHostname(String fullHostname) { } 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 static String byteToHexString(byte[] bytes, int start, int end) { } 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 void run() { * 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 static long string2long(String s) { 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 static String escapeHTML(String string) { } /** - * 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; + return TraditionalBinaryPrefix.long2String(len, "B", 2); } - public static synchronized String limitDecimalTo2(double d) { - return decimalFormat.format(d); + /** @deprecated use StringUtils.format("%.2f", d). */ + @Deprecated + public static String limitDecimalTo2(double d) { + return format("%.2f", d); } /** diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestStringUtils.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestStringUtils.java index d07afcc5f0a..eca5ede14d6 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestStringUtils.java +++ b/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.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 void testUnescapeString() throws Exception { @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 void testTraditionalBinaryPrefix() throws Exception { 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 void testTraditionalBinaryPrefix() throws Exception { 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 void testTraditionalBinaryPrefix() throws Exception { 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 static void main(String []args) { } 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 ); } } }