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







Reply via email to