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

[Paul Sandoz]

It may be better to expose a new enum value in Flags and set the bit based on the _intrinsic_id, rather than JVMCI checking the range.

Thus the Flags enum value for sig-poly remains constant even if the _intrinsic_id values do not (IIUC they will change when new intrinsics are added), thus less needs to be exposed to JVMCI.

e.g. something like below (not tested)?

Paul.

diff -r 881b3acaed84 src/share/vm/oops/method.cpp
--- a/src/share/vm/oops/method.cpp	Thu Aug 18 21:33:36 2016 +0000
+++ b/src/share/vm/oops/method.cpp	Fri Aug 19 17:25:22 2016 -0700
@@ -85,6 +85,7 @@
   set_constMethod(xconst);
   set_access_flags(access_flags);
   set_intrinsic_id(vmIntrinsics::_none);
+  set_signature_polymorphic(false);
   set_jfr_towrite(false);
   set_force_inline(false);
   set_hidden(false);
@@ -1451,6 +1452,8 @@
     id = MethodHandles::signature_polymorphic_name_id(method_holder(), name());
     if (is_static() != MethodHandles::is_signature_polymorphic_static(id))
       id = vmIntrinsics::_none;
+    else
+      set_signature_polymorphic(true);
     break;
   }

diff -r 881b3acaed84 src/share/vm/oops/method.hpp
--- a/src/share/vm/oops/method.hpp	Thu Aug 18 21:33:36 2016 +0000
+++ b/src/share/vm/oops/method.hpp	Fri Aug 19 17:25:22 2016 -0700
@@ -82,7 +82,8 @@
     _has_injected_profile  = 1 << 5,
     _running_emcp          = 1 << 6,
     _intrinsic_candidate   = 1 << 7,
-    _reserved_stack_access = 1 << 8
+    _reserved_stack_access = 1 << 8,
+    _signature_polymorphic = 1 << 9
   };
   mutable u2 _flags;

@@ -867,6 +868,13 @@
     _flags = x ? (_flags | _reserved_stack_access) : (_flags & ~_reserved_stack_access);
   }

+  bool signature_polymorphic() {
+    return (_flags & _signature_polymorphic) != 0;
+  }
+  void set_signature_polymorphic(bool x) {
+    _flags = x ? (_flags | _signature_polymorphic) : (_flags & ~_signature_polymorphic);
+  }
+
   ConstMethod::MethodType method_type() const {
       return _constMethod->method_type();
   }

> On 19 Aug 2016, at 17:05, John Rose <john.r.rose at oracle.com> wrote:
> 
> On Aug 19, 2016, at 2:32 PM, Doug Simon <doug.simon at oracle.com> wrote:
>> 
>>> 
>>> On 19 Aug 2016, at 20:14, John Rose <john.r.rose at oracle.com> wrote:
>>> 
>>> 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.
>> 
>> Most of the laziness is to make HSRJMI objects as light as possible. They have a reference to a Method* and query it with Unsafe (as you observe).
> 
> Yes.  It's good pattern:  lazy, minimal state and setup, no extra copies of stuff.
> 
> (It's not absolute, though, since the HotSpotResolvedJavaMethodImpl has a few key eager initializations.  The HotSpot's own internal CI makes similar choices.)
> 
>>>>> 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.
>> 
>> That would definitely be the best option from my perspective. Fast, cheap and shifts all the sig-poly logic to the VM. Assuming I can exercise this option, how best to proceed? The changes to Method and the class file parser should obviously be done in a seperate RFE. Is that something you or someone in your team could undertake? I’d like to integrate 8164214 without waiting for the Method::_signature_polymorphic bit to be available since it’s blocking a few other issues and efforts.
> 
> In principle, yes,  But let's see if we can use the other option, the intrinsic_id.
> 
>> 
>>> 
>>> 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.
>> 
>> I’m not sure I fully understand this proposal. Are you saying it might be possible to fully encode the sig-poly bit(s) into intrinsic_id (as opposed to just using intrinsic_id as a guard for a VM call)? And would this encoding be guaranteed never to change? If not, then it could require a change on the Java side at which point I think we’re back to being no better off than the sig-poly holder test as the guard for a VM call.
> 
> There is a range of intrinsic ID's which apply only to s-p methods.  And every s-p method has such an IID.
> Thus, I think you can load the IID (in Java) and range-check it.  You might need a couple of new config parameters,
> for the bounds of the range.  That's something you can do on the side without a new RFE, right?
> Actually, good-enough constants are already there:  Config.vmIntrinsicInvokeBasic, etc.
> See the comment here:
>   http://hg.openjdk.java.net/jdk9/jdk9/hotspot/file/a4faaf753e03/src/share/vm/classfile/vmSymbols.hpp#l1396
> 
> I don't know why that wouldn't work…
> 
>>> 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.
>> 
>> Yes, a cheap test of a bit (not sure I understand what a fraction of a bit looks like!) would be ideal. Seems like a justifiable use of one of the 7 spare bits in Method::_flags.
> 
> Since s-p methods are disjoint from other intrinsics, it follows that both kinds of methods can share the IID coding space.  The relatively small fraction of IID codes used by s-p methods means that the other methods get about 7.9 bits, and the s-p methods use the remaining 0.1 bits of the 8-bit IID field.  :-)
> 
> — John
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 841 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20160819/5af51e5c/signature.asc>