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

[Brian Goetz]

I don’t think it’s worth the added complexity.  The shared arg is interned so you’re talking 2 more bytes in the bootstrap method attribute per call site.  

Sent from my iPad

> On Nov 14, 2017, at 7:34 PM, B. Blaser <bsrbnd at gmail.com> wrote:
> 
> 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