Possibly inefficient code gen of instanceof patterns
Jan Lahoda
jan.lahoda at oracle.com
Thu Jan 28 15:11:02 UTC 2021
Hi,
I've filled:
https://bugs.openjdk.java.net/browse/JDK-8260593
Thanks,
Jan
On 28. 01. 21 15:25, Remi Forax wrote:
> ----- Mail original -----
>> De: "Claes Redestad" <claes.redestad at oracle.com>
>> À: "compiler-dev" <compiler-dev at openjdk.java.net>
>> Envoyé: Jeudi 28 Janvier 2021 14:58:47
>> Objet: Possibly inefficient code gen of instanceof patterns
>> Hi,
>>
>> as an experiment, I changed some code to use instanceof pattern
>> matching to see if there's any effect on the generated code.
>>
>> Consider j.l.i.BootstrapMethodInvoker.invoke, which I changed from:
>>
>> private static Object invoke(MethodHandle bootstrapMethod, Lookup
>> caller,
>> String name, Object type) throws
>> Throwable {
>> if (type instanceof Class) {
>> return bootstrapMethod.invoke(caller, name, (Class)type);
>> } else {
>> return bootstrapMethod.invoke(caller, name, (MethodType)type);
>> }
>> }
>>
>> .. to:
>>
>> private static Object invoke(MethodHandle bootstrapMethod, Lookup
>> caller,
>> String name, Object type) throws
>> Throwable {
>> if (type instanceof Class c) {
>> return bootstrapMethod.invoke(caller, name, c);
>> } else {
>> return bootstrapMethod.invoke(caller, name, (MethodType)type);
>> }
>> }
> It should be "type instanceof Class<?> c" BTW.
>
>> This seem to grow the bytecode size ever so slightly. Looking at the
>> javap output it seems some of that might be redundant and possibly
>> fixable:
>>
>> private static java.lang.Object invoke(java.lang.invoke.MethodHandle,
>> java.lang.invoke.MethodHandles$Lookup, java.lang.String,
>> java.lang.Object) throws java.lang.Throwable;
>> descriptor:
>> (Ljava/lang/invoke/MethodHandle;Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/Object;)Ljava/lang/Object;
>> flags: (0x000a) ACC_PRIVATE, ACC_STATIC
>> Code:
>> stack=4, locals=6, args_size=4
>> 0: aload_3
>> 1: astore 5
>> 3: aload 5
>> 5: instanceof #46 // class java/lang/Class
>> 8: ifeq 27
>> 11: aload 5
>> 13: checkcast #46 // class java/lang/Class
>> 16: astore 4
>> 18: aload_0
>> 19: aload_1
>> 20: aload_2
>> 21: aload 4
>> 23: invokevirtual #169 // Method
>> java/lang/invoke/MethodHandle.invoke:(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/Class;)Ljava/lang/Object;
>> 26: areturn
>> 27: aload_0
>> 28: aload_1
>> 29: aload_2
>> 30: aload_3
>> 31: checkcast #61 // class
>> java/lang/invoke/MethodType
>> 34: invokevirtual #172 // Method
>> java/lang/invoke/MethodHandle.invoke:(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;)Ljava/lang/Object;
>> 37: areturn
>> LineNumberTable:
>> line 226: 0
>> line 227: 18
>> line 229: 27
>> LocalVariableTable:
>> Start Length Slot Name Signature
>> 18 9 4 c Ljava/lang/Class;
>> 0 38 0 bootstrapMethod
>> Ljava/lang/invoke/MethodHandle;
>> 0 38 1 caller
>> Ljava/lang/invoke/MethodHandles$Lookup;
>> 0 38 2 name Ljava/lang/String;
>> 0 38 3 type Ljava/lang/Object;
>> StackMapTable: number_of_entries = 1
>> frame_type = 27 /* same */
>> Exceptions:
>> throws java.lang.Throwable
>>
>> - The storing and loading of the type argument into slot 5 seem
>> completely redundant: couldn't we load slot 3 in each place that
>> now loads slot 5?
> yes, it looks like a bug.
>
>> - Storing c into a new slot 4 also seem unfortunate, but is more
>> understandable (I guess this is a limitation of the LVT not allowing
>> overlapping a new variable with a new type into the same slot?)
> The new syntax declares a supplementary local variable so it has to be visible in the bytecode for debuggers.
>
>> - It's also unfortunate that the checkcast now bubbles up so that the
>> result has to be stored and loaded again, but I guess pushing the
>> checkcast down to just before the invokevirtual as in the other
>> branch would go against some specification.
> Modern javac (i don't know the previous version, before java 1.4) never had such "optimization".
>
> As a grey beard, when the enhanced for was introduced, the for(:), we had the very similar discussions, because the enhanced for introduces a supplementary variable and usually a cast because of the erasure of generics, so the bytecode is a little more verbose. In the end, yes, it inflates the generated bytecodes at bit because it creates a new local variable but the java code is more readable because it creates a new local variable :)
>
>> My gut feeling - without knowing much about javac - is that at least
>> the use of a temporary slot (5) could be optimized away here.
> I agree.
>
>> WDYT?
>>
>> For reference the baseline method without the pattern matching
>> compiles to this:
>>
>> private static java.lang.Object invoke(java.lang.invoke.MethodHandle,
>> java.lang.invoke.MethodHandles$Lookup, java.lang.String,
>> java.lang.Object) throws java.lang.Throwable;
>> descriptor:
>> (Ljava/lang/invoke/MethodHandle;Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/Object;)Ljava/lang/Object;
>> flags: (0x000a) ACC_PRIVATE, ACC_STATIC
>> Code:
>> stack=4, locals=4, args_size=4
>> 0: aload_3
>> 1: instanceof #46 // class java/lang/Class
>> 4: ifeq 18
>> 7: aload_0
>> 8: aload_1
>> 9: aload_2
>> 10: aload_3
>> 11: checkcast #46 // class java/lang/Class
>> 14: invokevirtual #169 // Method
>> java/lang/invoke/MethodHandle.invoke:(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/Class;)Ljava/lang/Object;
>> 17: areturn
>> 18: aload_0
>> 19: aload_1
>> 20: aload_2
>> 21: aload_3
>> 22: checkcast #61 // class
>> java/lang/invoke/MethodType
>> 25: invokevirtual #172 // Method
>> java/lang/invoke/MethodHandle.invoke:(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;)Ljava/lang/Object;
>> 28: areturn
>> LineNumberTable:
>> line 226: 0
>> line 227: 7
>> line 229: 18
>> LocalVariableTable:
>> Start Length Slot Name Signature
>> 0 29 0 bootstrapMethod
>> Ljava/lang/invoke/MethodHandle;
>> 0 29 1 caller
>> Ljava/lang/invoke/MethodHandles$Lookup;
>> 0 29 2 name Ljava/lang/String;
>> 0 29 3 type Ljava/lang/Object;
>> StackMapTable: number_of_entries = 1
>> frame_type = 18 /* same */
>> Exceptions:
>> throws java.lang.Throwable
>>
>> Thanks!
>>
>> /Claes
> Rémi
More information about the compiler-dev
mailing list