RFR: 8231269: CompileTask::is_unloaded is slow due to JNIHandles type checks [v6]

[Vladimir Ivanov]

On Mon, 28 Apr 2025 16:16:41 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> I remember looking at this, and convinced myself that non-strong hidden classes report their related Java mirror as `klass_holder`, and that is enough to maintain them as alive. See calls to `ClassLoaderData::initialize_holder`:
>> 
>> https://github.com/openjdk/jdk/blob/b1e778d9d2ad13ee5f1ed629a8805008580f86c0/src/hotspot/share/classfile/classLoaderData.cpp#L159-L162
>> 
>> https://github.com/openjdk/jdk/blob/b1e778d9d2ad13ee5f1ed629a8805008580f86c0/src/hotspot/share/classfile/systemDictionary.cpp#L810-L814
>> 
>> This is what the comment in `UnloadableMethodHandle::get_unload_blocker` refers to. So I believe current code is correct. 
>> 
>> I agree `is_permanent_class_loader_data()` captures the intent better. Let me see if it fits well here.
>
>> I agree is_permanent_class_loader_data() captures the intent better. Let me see if it fits well here.
> 
> Ah wait, it does not. We need to hold on to something that blocks the unloading. Just checking `is_permanent_class_loader_data()` does not get us there. We would need to ask for some holder for it. For the reasons above, `method->method_holder()->klass_holder()` works for non-strong hidden classes as well.
> 
> This is also why current mainline code works -- it captures the same thing.

Ok, thanks for checking! Good to know there's no existing bug.

What I had in mind is as follows:

InstanceKlass* holder = method->method_holder();
if (holder->class_loader_data()->is_permanent_class_loader_data()) {
  return nullptr; // method holder class can't be unloaded
} else {
  // Normal class, return the holder that would block unloading.
  // This would be either classloader oop for non-hidden classes,
  // or Java mirror oop for hidden classes.
  assert(holder->klass_holder() != nullptr, "");
  return holder->klass_holder();
}


IMO it makes the check more precise and, at the same time, communicates the intent better. What do you think?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24018#discussion_r2064315717