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.redes...@oracle.com <mailto:claes.redes...@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





Reply via email to