RFR: 8164214: [JVMCI] include VarHandle in signature polymorphic method test

[John Rose]

On Aug 17, 2016, at 1:46 PM, Doug Simon <doug.simon at oracle.com> wrote:
> 
>> 
>> On 17 Aug 2016, at 22:13, John Rose <john.r.rose at oracle.com> wrote:
>> 
>> On Aug 17, 2016, at 1:00 PM, Paul Sandoz <paul.sandoz at oracle.com> wrote:
>>> 
>>> 
>>>> On 17 Aug 2016, at 06:08, Doug Simon <doug.simon at oracle.com> wrote:
>>>> 
>>>> Please review this change that should have been part of 8163962. The problem is that java.lang.invoke.VarHandle needs to be treated the same as java.lang.invoke.MethodHandle when resolving invoke bytecodes, especially when the resolution should be eager (e.g., as may be required for ahead of time compilation).
>>>> 
>>>> I removed ResolvedJavaMethod.isSignaturePolymorphic(...) as well since a) it was broken and b) we should rely as much as possible on VM code to do the test for whether a method is signature polymorphic to be more future proof.
>> 
>> Ideally, the CI should call MethodHandles::is_signature_polymorphic_name in the VM, as resolveInvokeHandleInPool does, or at least assert an equivalence with that call.  Is there a pre-existing attribute-fetching routine for methods that could deliver the extra bit?
> 
> I’m a little confused - what CI code are you referring to? In my mind, resolveInvokeHandleInPool is part of JVMCI. Also, I’m not aware of any attribute bit denoting a signature polymorphic method.

I'll tell you the way it looks to me, with a disclaimer that you (and Igor/Vladimir/Chris) know far more about how the JVMCI works.

The main attribute-fetching hook for methods is HotSpotResolvedJavaMethodImpl / CompilerToVM::get_jvmci_method, which does as much as possible lazily; the lazy logic tries to use Unsafe peek/poke methods on the metaspace method instead of expensive transitions into the JVM.

Meanwhile, the checks for sig-poly in the JVM are centralized in MethodHandles::is_method_handle_invoke_name and various related inline methods (in methodHandles.hpp).  The tricky dance is more complicated than just fetching a mode bit.  (But perhaps we should add the mode bit.)

You don't want to make a JVM call for a large number of ResolvedJavaMethods.  So you are looking to replicate the sig-poly logic somewhere in Java.

But that perpetuates the code duplication (moves it to a lower level in the webrev, IIUC).  It is perhaps defensible because the CI code is tightly coupled to HotSpot, and/or JVM spec talks about sig-poly methods.  But it gets fragile as soon as we are move the spec (as we are in 9 and will probably do more).  And our private APIs between JVM and JDK might have off-label sig-poly methods and classes, not mandated by the JVM spec.  So code duplication = fragile even in this case.

I see three ways to kill the code duplication:  0. Put in a query method that punches into the JVM (too slow), 1. Have an into-the-JVM query method, but gate it on an easy-to-determine precondition rarely true (intrinsic_id comes to mind, but that doesn't work for VarHandles), 2. Pull a flag eagerly from the JVM when creating a HSRJM, on the assumption that you are already visiting the JVM when you are creating a HSRJM that might be sig-poly (which may be false?).

I see there are no eagerly-pulled flags in HSRJMI, which makes me doubt option 2.

> 
>> 
>> The move of the "magic names" MethodHandle and VarHandle down into the hotspot-specific CI code is good, but it would be best to pull the sig-poly bit straight from the VM.
> 
> It’s a little tricky since we cannot deal with Symbol* values directly in the Java part of JVMCI and all VM based sig-poly tests are based on such values. One thing we could do is ask the VM for the set of sig-poly holders. However, I don’t see this centralized anywhere in the VM currently.

I think a reasonable high road to take would be to model the sig-poly query on the caller-sensitive query.  That means putting a new flag in Method::_flags.  I support this, if you wish to make that cut.  The class file parser would have to (a) detect when a sig-poly-bearing class is being loaded (this is a cheap tax), and then (b) more carefully sift the methods and mark the sig-poly ones.

Another reasonable high road would be to choose a special value to store in the Method::_intrinsic_id field for sig-poly methods that don't already have their own special intrinsic_id.  Again, the value would be set up (cheaply) at class load time, and the JVMCI could probe for that.

Paul, does any of this sound reasonable?

The sig-poly condition, like the caller-sensitive condition, is very rare, but probably needs to be queried on many methods.  Therefore there is a tradeoff between compact representation (ideally a fraction of a bit, as in intrinsic_id) and fast access.

Finally, I'm not against the lower-road approach, of moving the duplicated query logic down-stack, but I want us to consider the best option.

HTH

— John
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20160819/20148500/attachment.html>