RFR(L): 8143211: provide bytecode intrinsics for loop and try/finally executors

[Vladimir Ivanov]

Nice work, Michael!

Some comments follow:

===

src/java.base/share/classes/java/lang/invoke/InvokerBytecodeGenerator.java:

+    private final List<Class<?>> loopClauseTypes;
+    /** This list contains the actual local state types added by loop 
clauses, without {@code void} entries. */
+    private final List<Class<?>> loopLocalStateTypes;
+    /** The first slot after the arguments, to contain the first loop 
state element. */
+    private final int firstLoopStateSlot;

You assume there's at most a single loop intrinsic in a compiled LF 
instance.

It's the case now, but it's not necessarily the case in the future. 
There's no such limitation for other intrinsics and I'd prefer to keep 
it that way.

Why not iterate over the LF to compute localsMap & localClasses? You can 
restore the information in emitLoop() from the intrinsic (localStateDesc).

===

src/java.base/share/classes/java/lang/invoke/MethodHandleImpl.java:

+     * The LambdaForm shape for the loop combinator is as follows 
(assuming one reference parameter passed in
...
+     *  tryFinally=Lambda(a0:L,a1:L)=>{
...
+     *    t9:L=MethodHandleImpl.loop(t2:L,t3:L,t4:L,t5:L,t6:L);  // 
call the loop executor

Wrong LambdaForm name in the example (tryFinally vs loop).

===

1058     private static BoundMethodHandle 
makeBoundHandle(BoundMethodHandle.SpeciesData data, Object... args) {
1059         try {
1060             return (BoundMethodHandle) 
data.constructor().invokeWithArguments(args);
1061         } catch (Throwable ex) {
1062             throw uncaughtException(ex);
1063         }
1064     }

It can be used in makeGuardWithTest as well.

I like the helper method, but it can cause bootstrapping issues (due to 
MH.invokeWithArguments which depends on j.l.i machinery) if used 
unwisely. I think that's the main reason why invokeBasic was used in the 
first place.

===

+    static final class IntrinsicMethodHandle extends 
DelegatingMethodHandle {
...
+        private final String localStateDesc;

Maybe subclass IntrinsicMH instead?

LoopIntrinsic extends IntrinsicMethodHandle {
   private final String localStateDesc;

   LoopIntrinsic(MethodHandle target, String localStateDesc) {
     super(target,Intrinsic.LOOP);
     this.localStateDesc = localStateDesc;
   }
...
   String getLocalStateDesc() {
     return localStateDesc;
   }
}

static MethodHandle makeLoopIntrinsic(MethodHandle target, String 
localStateDesc) {
   return new LoopIntrinsic(target, localStateDesc);
}


Also, you can consider getting rid of IMH and moving intrinsic 
information to LambdaForm.NamedFunction (akin to 
LambdaForm.Name.constraint). I was responsible for adding IMH, but 
LambdaForm.NameFunction looks like a better place now.

===

src/java.base/share/classes/java/lang/invoke/MethodTypeForm.java:

+    Map<String, SoftReference<LambdaForm>> loopLambdaForms;

What about purging of stale entries from the cache?

Have you considered reusing LambdaFormEditor? E.g., put a root LF (w/ 
empty local state?) in MTF slot and fork new LFs (w/ non-empty local 
state) from it?

Best regards,
Vladimir Ivanov

On 6/16/16 4:17 PM, Michael Haupt wrote:
> Dear all,
>
> please review this change.
> RFE: https://bugs.openjdk.java.net/browse/JDK-8143211
> Webrev: http://cr.openjdk.java.net/~mhaupt/8143211/webrev.00/
>
> The change puts the tryFinally and loop method handle combinator implementations, which were introduced as part of the JEP 274 effort, on a LambdaForm basis, which executes in bytecode generating (default) and LambdaForm interpretation (-Djava.lang.invoke.MethodHandle.COMPILE_THRESHOLD=-1) modes. It also changes the output formatting of LambdaForms, introducing a (hopefully) more readable format.
>
> Thanks,
>
> Michael
>