RFR [9]: 8050142: Optimize java.util.Formatter

[Xueming Shen]

Shouldn't this one be at the same class (FormatSpecifier?) as the "localizedMagnitudeExp"?
the webrev shows it is one level up.
Btw, when we are here, maybe we can just remove the FormatSpecifier.getZero(), it appears
we have a static version already at Formatter level.

On 09/24/2014 10:41 AM, Xueming Shen wrote:
> Looks good.
>
> On 09/23/2014 02:07 PM, Claes Redestad wrote:
>> How about:
>>
>>     // Specialized localization of exponents, where the source value can only
>>     // contain characters '0' through '9', starting at index offset, and no
>>     // group separators is added for any locale.
>>     private void localizedMagnitudeExp(StringBuilder sb, char[] value,
>>             final int offset, Locale l) {
>>         char zero = getZero(l);
>>
>>         int len = value.length;
>>         for (int j = offset; j < len; j++) {
>>             char c = value[j];
>>             sb.append((char) ((c - '0') + zero));
>>         }
>>     }
>>
>> Webrev including this and the fixes for Conversion.SCIENTIFIC:
>> http://cr.openjdk.java.net/~redestad/8050142/webrev.09/
>>
>> /Claes
>>
>> On 2014-09-23 22:12, Xueming Shen wrote:
>>> I don''t think an exponent should ever have a "dot', it always a signed integer. I think we can
>>> just remove the dead code, maybe put some wording to explain why no group, no dot here.
>>>
>>> -Sherman
>>>
>>> On 09/23/2014 12:42 PM, Claes Redestad wrote:
>>>> Ouch... but wait... the char[] returned from sun.misc.FormattedFloatingDecimal.getExponent() can never contain a '.', so we'll never find a dot here. Remove the dead code or fix the logic?
>>>>
>>>> /Claes
>>>>
>>>> On 2014-09-23 21:30, Xueming Shen wrote:
>>>>> Also the logic in the loop of localizedMagnitudeExp() does not look correct.
>>>>> Shouldn't it be
>>>>>
>>>>> char c= value[j];
>>>>> if (c == '.') {
>>>>>    append l10n dot...
>>>>> } else {
>>>>> sb.append(c - '0' + zero);
>>>>> }
>>>>>
>>>>> it appears the 'else" is missing? or I read it wrong?
>>>>>
>>>>> -Sherman
>>>>>
>>>>> On 09/23/2014 12:27 PM, Claes Redestad wrote:
>>>>>> On 2014-09-23 21:14, Xueming Shen wrote:
>>>>>>> On 09/22/2014 12:43 PM, Claes Redestad wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> Sherman pointed out that there was a path that could actually take a minor performance hit from this patch,
>>>>>>>> which would be unacceptable. This version takes the minimal approach to addressing this by adding back a
>>>>>>>> method operating on a char[], simplified for the specific usage case (the exponent part of a %g double formatting):
>>>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~redestad/8050142/webrev.07/
>>>>>>>>
>>>>>>>> This latest patch passes using the extended test coverage of java.util.Formatter I've proposed in 8058887, see
>>>>>>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-September/028849.html
>>>>>>>>
>>>>>>>
>>>>>>> Hi Claes,
>>>>>>>
>>>>>>> Shouldn't we also keep the exp as char[] as well in "c == Conversion.SCIENTIFIC" case?
>>>>>>> It appears the usage of exp is the same as the one in "GENERAL", in which we are keeping
>>>>>>> the simple char[] for exp.
>>>>>>
>>>>>> You're right, of course. I'll update the webrev.
>>>>>>
>>>>>> /Claes
>>>>>
>>>>
>>>
>>
>