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

[Claes Redestad]

Hi Andrej,

  while the change seems reasonable, I can't find any usage of %a/%A in 
the JDK and haven't even covered it in my micros. :-) Also, it could be 
further improved with 8041972 to turn:

3533                             String exp = res.substring(idx + 1);
3534                             int iexp = Integer.parseInt(exp) -54;

3533                             int exp = Integer.parseInt(res, 10, idx 
+ 1);

  I've planned to file a follow-on RFE to incorporate improvements made 
possible by 8041972 (like the one above); maybe we should roll this 
suggestion into that follow-up and add some micros to measure that we're 
actually improving?

/Claes

On 07/14/2014 03:09 PM, Andrej Golovnin wrote:
> Hi Claes,
>
> it looks good. And I have one minor improvement. Take look at the 
> following lines of the method FormatSpecifier#hexDouble(double, int):
>
> 3532                             // Get exponent and append at the end.
> 3533                             String exp = res.substring(idx + 1);
> 3534                             int iexp = Integer.parseInt(exp) -54;
> 3535                             return res.substring(0, idx) + "p"
> 3536                                 + Integer.toString(iexp);
>
> The lines 3535-3536 are translated by the compiler to:
>
> new StringBuilder().append(res.substring(0, 
> idx)).append("p").append(Integer.toString(iexp)).toString();
>
> If we rewrite it to use StringBuilder, we can avoid creation of 
> intermediate String objects, e.g.:
>
> 3535                             return new 
> StringBuilder().append(res, 0, idx).append('p')
> 3536 .append(iexp).toString();
>
> But I'm not sure how common is the usage of #hexDouble-method. Maybe 
> this change is not worth it.
>
> Best regards,
> Andrej Golovnin
>
>
>
> On Mon, Jul 14, 2014 at 2:23 PM, Claes Redestad 
> <claes.redestad at oracle.com <mailto:claes.redestad at oracle.com>> wrote:
>
>     Hi again,
>
>      updated webrev:
>     http://cr.openjdk.java.net/~redestad/8050142/webrev.1
>     <http://cr.openjdk.java.net/%7Eredestad/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
>                 <http://cr.openjdk.java.net/%7Eredestad/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/
>             <http://cr.openjdk.java.net/%7Eplevart/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
>
>
>
>