A small patch to use the StringConcatFactory instead of String.format() to implement toString() of a data class

[B. Blaser]

Brian,

On 13 November 2017 at 21:38, Brian Goetz <brian.goetz at oracle.com> wrote:
> Thanks!  We are refining a combo test to validate these sorts of changes.
>
> Performance tweaks are low priority for now, and we’ll need to chose based on realistic Performance measurements. I am sure the version I wrote is much slower than optimal so I doubt well be keeping that one. But if anybody wants to work on a performance testing harness for code gen options here that would be great!
>

I won't take too much time speaking of generated code optimization as
it doesn't seem to be the main priority right now (and not fully
related to this thread)...

But when recently correcting a jasm bug related to indy static
arguments [1], I observed (which is also mentioned in the javadoc [2])
that in a number of situations 'samMethodType' is identical to
'instantiatedMethodType' when using 'LambdaMetafactory.metafactory()'
as bootstrap method, causing the duplication of the two corresponding
static arguments, for example:

import java.lang.invoke.*;

interface F {
    void m();
}

class B {
    void b() {
        System.out.println("success!");
    }

    public static void main(String[] args) {
        F f = new B()::b;
        f.m();
    }
}

Using 'javap', we can see the following bootstrap method along with
the corresponding static arguments:

BootstrapMethods:
  0: #27 REF_invokeStatic
java/lang/invoke/LambdaMetafactory.metafactory:(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodHandle;Ljava/lang/invoke/MethodType;)Ljava/lang/invoke/CallSite;
    Method arguments:
      #28 ()V
      #29 REF_invokeVirtual B.b:()V
      #28 ()V

I think it should be possible to have a simplified bootstrap method
signature for such cases to reduce the number of static arguments
which will probably avoid some 'ldc' (JVMS9 §5.4.3.6) and save space
in the class file:

public class Boot {
    public static CallSite bsm(MethodHandles.Lookup l, String s,
MethodType t, MethodHandle h, MethodType sam)
            throws Exception {
        return LambdaMetafactory.metafactory(l, s, t, sam, h, sam);
    }
}

BootstrapMethods:
  0: #40 REF_invokeStatic
Boot.bsm:(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodHandle;Ljava/lang/invoke/MethodType;)Ljava/lang/invoke/CallSite;
    Method arguments:
      #29 REF_invokeVirtual C.c:()V
      #54 ()V

I observed that with JDK9 but I'm not sure this is still the case with amber/10.

Regards,
Bernard


[1] https://bugs.openjdk.java.net/browse/CODETOOLS-7901507
[2] https://docs.oracle.com/javase/9/docs/api/java/lang/invoke/LambdaMetafactory.html