Possibly inefficient code gen of instanceof patterns

Remi Forax forax at univ-mlv.fr
Thu Jan 28 14:25:24 UTC 2021


----- 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