[jdk18] RFR: 8279515: C1: No inlining through invokedynamic and invokestatic call sites when resolved class is not linked

[Vladimir Ivanov]

On Wed, 5 Jan 2022 15:30:27 GMT, Vladimir Ivanov <vlivanov at openjdk.org> wrote:

> [JDK-8267806](https://bugs.openjdk.java.net/browse/JDK-8267806) broke inlining at `invokedynamic` and `invokestatic` call sites in some circumstances. Though the enhancement was intended to relax the inlining checks, it introduced one new check to ensure that resolved klass (`REFC`) is linked. It turned out the check is too strong: resolution of the symbolic class reference at a call site does not trigger class linkage.  
> 
> For `invokestatic` consider the following example:
> 
> class A { static m() { ... } }
> class B extends A {}
> 
> invokestatic "B"::"m" => A::m
> 
> 
> Method resolution loads class `B` along the way, but neither initialises nor links it. Invocation does trigger class initialisation, but only for class `A`. 
> 
> For `invokedynamic` it's a bit more complicated. Syntactically, there's no symbolic class reference at the call site, but CI assigns an artificial one (`java.lang.invoke.MethodHandle`, see `ciBytecodeStream::get_declared_method_holder()`). The problem is `MethodHandle` class doesn't have to be loaded by the class loader of the accessing class, so `ciEnv::get_klass_by_name()` can produce unloaded CI mirror (`ciKlass`).
> 
> For example, indy call site linkage of indified string concatenation (`StringConcatFactory.makeConcatWithConstants()`) doesn't get `MethodHandle` class recorded in the context class loader [1]  while `LambdaMetafactory.metafactory()` introduces a class loader constraint for it [2] which reveals the class to the subsequent class lookup.  ​
> 
> I decided to make `ciBytecodeStream::get_declared_method_holder()` predictable and return well-known VM class right away without doing the lookup.
> 
> Proposed fix relaxes the constraint on resolved class (`REFC`) and requires it to be loaded.
> 
> Also, while working on the fix, I spotted that the constraint for `invokestatic` is also too strong (requires resolved class to be initialized while the specification mandates resolved method holder to be initialized instead):
> 
>     if ((code == Bytecodes::_invokestatic && callee_holder->is_initialized()) || // invokestatic involves an initialization barrier on resolved klass
> 
> 
> I fixed it as well (`s/callee_holder/klass/`).
> 
> Testing: hs-tier1 - hs-tier4
> 
> [1]
> 
> [0.160s][debug][class,resolve           ] Test1 java.lang.invoke.StringConcatFactory Test.java:3
> [0.160s][info ][class,loader,constraints] adding new constraint for name: java/lang/invoke/MethodHandles$Lookup, loader[0]: 'app', loader[1]: 'bootstrap'
> [0.160s][info ][class,loader,constraints] adding new constraint for name: java/lang/invoke/MethodType, loader[0]: 'app', loader[1]: 'bootstrap'
> [0.160s][info ][class,loader,constraints] adding new constraint for name: java/lang/invoke/CallSite, loader[0]: 'app', loader[1]: 'bootstrap'
> 
> 
> [2] 
> 
> [0.130s][debug][class,resolve          ] Test java.lang.invoke.LambdaMetafactory Test.java:21
> [0.130s][info ][class,loader,constraints] adding new constraint for name: java/lang/invoke/MethodHandles$Lookup, loader[0]: 'app', loader[1]: 'bootstrap'
> [0.130s][info ][class,loader,constraints] adding new constraint for name: java/lang/invoke/MethodType, loader[0]: 'app', loader[1]: 'bootstrap'
> [0.130s][info ][class,loader,constraints] adding new constraint for name: java/lang/invoke/MethodHandle, loader[0]: 'app', loader[1]: 'bootstrap'
> [0.130s][info ][class,loader,constraints] adding new constraint for name: java/lang/invoke/CallSite, loader[0]: 'app', loader[1]: 'bootstrap'

Thanks for the review, Vladimir and Dean.

-------------

PR: https://git.openjdk.java.net/jdk18/pull/80