RFR [9]: 8050142: Optimize java.util.Formatter (original) (raw)

Claes Redestad claes.redestad at oracle.com
Mon Jul 14 22:41:12 UTC 2014

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

More information about the core-libs-dev mailing list