RFR: 8188102: [JVMCI] Convert special JVMCI oops in nmethod to jweak values

[Kim Barrett]

[added hotspot-gc-dev to cc list]

> On Oct 27, 2017, at 5:05 PM, Doug Simon <doug.simon at oracle.com> wrote:
> 
> Please review this change that converts the JVMCI-specific object references in nmethod from oops to weak values. This removes GC API extensions added purely for these fields (e.g. so that G1 can insert it into the right remembered set, and when unloading an nmethod, to go and remove the nmethod from that remembered set).
> 
> Testing: I've run the Graal unit tests (mx unittest --verbose --gc-after-test -Xlog:class+unload=trace) which trigger a lot of nmethod unloading.
> 
> https://bugs.openjdk.java.net/browse/JDK-8188102
> http://cr.openjdk.java.net/~dnsimon/8188102/

I didn't look at the .java, .py, or project files.

------------------------------------------------------------------------------ 
src/hotspot/share/jvmci/jvmciCompilerToVM.cpp
1061       nmethod* nm = cb->as_nmethod_or_null();

This appears to be dead code now.

------------------------------------------------------------------------------ 
src/hotspot/share/code/nmethod.cpp
1023   assert(Universe::heap()->is_gc_active(), "should only be called during gc");
...
1036     if (!Universe::heap()->is_gc_active() && cause != NULL)
1037       cause->klass()->print_on(&ls);

I was going to mention that lines 1036-1037 are missing braces around
the if-body.  However, those lines appear to be dead code, given the
assertion on line 1023.

------------------------------------------------------------------------------ 
src/hotspot/share/code/nmethod.cpp
1504 bool nmethod::do_unloading_jvmci(BoolObjectClosure* is_alive, bool unloading_occurred) {
...
1506     oop installed_code = JNIHandles::resolve(_jvmci_installed_code);

Resolving a weak reference can keep an otherwise dead referent alive.
See JDK-8188055 for a discussion of the corresponding problem for
j.l.r.Reference.

Right now, I think JNIHandles doesn't provide a (public) solution to
what I think is being attempted here that works for all collectors.
There is in-progress work toward a solution, but it's just that, "in
progress".

As a (possibly interim) solution, a function like the following might
be added to JNIHandles (put the definition near resolve_jweak).

bool JNIHandles::is_global_weak_cleared(jweak handle) {
  assert(is_jweak(handle), "not a weak handle");
  return guard_value<false>(jweak_ref(handle)) == NULL;
}

(That's completely untested, and I haven't thought carefully about the
name.  And should get input from other GC folks on how to deal with
this.)  I *think* do_unloading_jvmci then becomes something like the
following (again, completely untested)

bool nmethod::do_unloading_jvmci(BoolObjectClosure* is_alive, bool unloading_occurred) {
  if (_jvmci_installed_code != NULL) {
    if (JNIHandles::is_global_weak_cleared(_jvmci_installed_code)) {
      if (_jvmci_installed_code_triggers_unloading) {
        make_unloaded(is_alive, NULL);
        return true;
      } else {
        clear_jvmci_installed_code();
      }
    }
  }
  return false;
}

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