Question on ciInstanceKlass::has_subklass() and unique_concrete_subklass()

[Krystal Mok]

Hi all,

There something I'm confused about around the way ciInstanceKlass caches
values of shared ci klasses. A "shared" ci klass means it's created during
ciObjectFactory initialization, and shared among all ciEnv instances.

Let me use the code in current JDK9 tip to be concrete:
has_subklass():
http://hg.openjdk.java.net/jdk9/jdk9/hotspot/file/tip/src/share/vm/ci/ciInstanceKlass.hpp#l137
  bool                   has_subklass()   {
    assert(is_loaded(), "must be loaded");
    if (_is_shared && !_has_subklass) {
      if (flags().is_final()) {
        return false;
      } else {
        return compute_shared_has_subklass();
      }
    }
    return _has_subklass;
  }

The way it's implemented, it makes sure that if a shared ci klass didn't
see a subklass before, it can go fetch the current value from the runtime.
The problem is: once such a klass has seen a subklass, it stays that way,
and disregards class unloading that may happen later on, which may make the
cached value different from the current actual value in the runtime
InstanceKlass.

unique_concrete_subklass():
http://hg.openjdk.java.net/jdk9/jdk9/hotspot/file/tip/src/share/vm/ci/ciInstanceKlass.cpp#l347
// ------------------------------------------------------------------
// ciInstanceKlass::unique_concrete_subklass
ciInstanceKlass* ciInstanceKlass::unique_concrete_subklass() {
  if (!is_loaded())     return NULL; // No change if class is not loaded
  if (!is_abstract())   return NULL; // Only applies to abstract classes.
  if (!has_subklass())  return NULL; // Must have at least one subklass.
  VM_ENTRY_MARK;
  InstanceKlass* ik = get_instanceKlass();
  Klass* up = ik->up_cast_abstract();
  assert(up->oop_is_instance(), "must be InstanceKlass");
  if (ik == up) {
    return NULL;
  }
  return CURRENT_THREAD_ENV->get_instance_klass(up);
}

This function depends on the klass having a subklass, but as mentioned
above, has_subklass() could return cached true for a shared ci klass even
if the real value in the runtime is false. That's not necessarily unsafe,
but logically it doesn't look right.

It's interesting to see that ciInstanceKlass never caches the _implementor
value for shared ci klasses. It may be a bit slow having to call into the
runtime to get the _implementor value every time, but at least it's safe
and sane.

My question is why doesn't has_subklass() follow the model of
implementor(), and call into runtime every time for shared ci klasses?

Thanks,
Kris
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20140313/c10743da/attachment-0001.html>