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

[Claes Redestad]

Sorry about that; maybe I should've renamed the field c to conv instead, 
but I think I need to be conservative now and will revert the name changes.

New webrev: http://cr.openjdk.java.net/~redestad/8050142/webrev.3

Changing usage of given locale or any semantic change is really 
out-of-scope here. There seems to be a few ancient bugs hanging around 
which maybe someone should take a look at, e.g., 
https://bugs.openjdk.java.net/browse/JDK-5063466

/Claes

On 2014-07-14 20:05, Jason Mehrens wrote:
> Claes,
>
>
> Maybe change (or not):
>
>
> -throw new UnknownFormatConversionException(String.valueOf(c));
>
> +throw new UnknownFormatConversionException(String.valueOf(conv));
>
>
>
> I haven't examined it too deeply but it seems odd that some of those print methods don't use the given locale when converting case.  That is probably not a change for this issue.
>
>
> Jason
>
>
>
>
> ----------------------------------------
>> Date: Mon, 14 Jul 2014 17:40:41 +0200
>> From: claes.redestad at oracle.com
>> To: core-libs-dev at openjdk.java.net
>> Subject: Re: RFR [9]: 8050142: Optimize java.util.Formatter
>>
>> Hi,
>>
>> final(?) webrev: http://cr.openjdk.java.net/~redestad/8050142/webrev.2
>>
>> Thanks in advance for reviews. I also need a volunteer to sponsor
>> this. :-)
>>
>> /Claes
>>
>> On 07/14/2014 04:21 PM, Claes Redestad wrote:
>>> Yes, might be worth addressing just for correctness/readability.
>>>
>>> /Claes
>>>
>>> On 07/14/2014 03:02 PM, Ivan Gerasimov wrote:
>>>> A very minor one:
>>>> 2704 if (Character.isUpperCase(conv))
>>>> 2705 f.add(Flags.UPPERCASE);
>>>> 2706 c = Character.toLowerCase(conv);
>>>>
>>>> maybe
>>>>
>>>> 2704 if (Character.isUpperCase(conv)) {
>>>> 2705 f.add(Flags.UPPERCASE);
>>>> 2706 c = Character.toLowerCase(conv);
>>>> }
>>>>
>>>> Sincerely yours,
>>>> Ivan
>>>>
>>>>
>>>> On 14.07.2014 16:23, Claes Redestad wrote:
>>>>> Hi again,
>>>>>
>>>>> updated webrev: http://cr.openjdk.java.net/~redestad/8050142/webrev.1
>>>>>
>>>>> changes:
>>>>> - specify capacity on line 2931 as suggested by Andrej Golovnin
>>>>> - exp.append("0") -> exp.append('0') on line 3781
>>>>> - merged append+justify into appendJustified as suggested by Peter
>>>>> Levart
>>>>> - replaced the reoccuring pattern of appending a number of zeros
>>>>> into a call to trailingZeros
>>>>>
>>>>> performance difference seemingly at noise levels in micros, but
>>>>> bonus to readability and Formatter*.class-files are now a total of
>>>>> 246 bytes smaller
>>>>>
>>>>> /Claes
>>>>>
>>>>> On 2014-07-14 13:29, Claes Redestad wrote:
>>>>>> Hi Peter,
>>>>>>
>>>>>> On 2014-07-14 13:25, Peter Levart wrote:
>>>>>>> On 07/14/2014 12:07 PM, Claes Redestad wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> please review this patch which optimizes away some allocations
>>>>>>>> from java.util.Formatter and achieve 1.1-1.3x speedups of micros
>>>>>>>> targetting String.format. See bug for more details.
>>>>>>>>
>>>>>>>> webrev: http://cr.openjdk.java.net/~redestad/8050142/webrev.0
>>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8050142
>>>>>>>>
>>>>>>>> Testing: JPRT, jtreg (java/lang/String, java/util/Formatter),
>>>>>>>> SPECjbb2013 and microbenchmarks
>>>>>>>>
>>>>>>>> Thanks!
>>>>>>>>
>>>>>>>> /Claes
>>>>>>> Hi Claes,
>>>>>>>
>>>>>>> Since justify() result is always appended to the resulting
>>>>>>> Appendable, you could merge the functionality and eliminate
>>>>>>> constructing intermediary StringBuilder altogether:
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~plevart/jdk9-dev/Formatter/webrev.01/
>>>>>>>
>>>>>> Looks good, especially eliminating the need for two different
>>>>>> append methods. I'll update based on this and other suggestions.
>>>>>>
>>>>>> /Claes
>>>>>>> Regards, Peter
>>>>>
>>>>>
>>