[
https://issues.apache.org/jira/browse/HADOOP-9252?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13569845#comment-13569845
]
Chris Nauroth commented on HADOOP-9252:
---------------------------------------
Thanks, Nicholas. It sounds like a good plan. I agree with you that the new
format is more easily readable. I think we can find the broken tests, change
them to accept either the old format or the new format, and then commit these
changes. For example, the failure I found in {{TestContainersMonitor}} is due
to a regex match on the old format. We can update that regex to match either
the old format or the new format. That way, other sub-projects shouldn't see
an impact from this change.
Can we add more tests to {{TestStringUtils}} around boundary conditions, such
as the minimum and maximum values of a long? Here is what I am seeing for some
boundary cases before and after the changes.
Before:
{code}
scala> println(StringUtils.humanReadableInt(Long.MaxValue))
8589934592.0g
scala> println(StringUtils.humanReadableInt(Long.MaxValue - 1))
8589934592.0g
scala> println(StringUtils.humanReadableInt(Long.MinValue))
-9223372036854775808
scala> println(StringUtils.humanReadableInt(Long.MinValue + 1))
-8589934592.0g
{code}
After:
{code}
scala> println(StringUtils.humanReadableInt(Long.MaxValue))
8.0 E
scala> println(StringUtils.humanReadableInt(Long.MaxValue - 1))
8.0 E
scala> println(StringUtils.humanReadableInt(Long.MinValue))
--9223372036854775808
scala> println(StringUtils.humanReadableInt(Long.MinValue + 1))
-8.0 E
{code}
Notice that there is an odd behavior on {{Long#MIN_VALUE}}: 2 negative signs.
That's because of this part of the code:
{code}
+ if (n < 0) {
+ b.append('-');
+ n = -n;
+ }
{code}
{{Long#MIN_VALUE}} is a really irritating case, because the data type cannot
represent the positive conversion. It would be 1 greater than
{{Long#MAX_VALUE}}, so negating it or taking its absolute value overflows back
to the same value:
{code}
scala> println(Long.MinValue)
-9223372036854775808
scala> println(-Long.MinValue)
-9223372036854775808
scala> println(-(-Long.MinValue))
-9223372036854775808
scala> println(Math.abs(Long.MinValue))
-9223372036854775808
{code}
Maybe we need to special-case this to prevent 2 negative signs. It looks like
the old version of the code wasn't able to get the units right either.
> StringUtils.limitDecimalTo2(..) is unnecessarily synchronized
> -------------------------------------------------------------
>
> Key: HADOOP-9252
> URL: https://issues.apache.org/jira/browse/HADOOP-9252
> Project: Hadoop Common
> Issue Type: Improvement
> Components: util
> Reporter: Tsz Wo (Nicholas), SZE
> Assignee: Tsz Wo (Nicholas), SZE
> Priority: Minor
> Attachments: c9252_20130127.patch, c9252_20130128.patch
>
>
> limitDecimalTo2(double) currently uses decimalFormat, which is a static
> field, so that it is synchronized. Synchronization is unnecessary since it
> can simply uses String.format(..).
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
