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

[Aleksey Shipilev]

On Fri, 25 Apr 2025 21:17:08 GMT, Vladimir Ivanov <vlivanov at openjdk.org> wrote:

>> Yes, this is about null (bootstrap) classloader; the system returns `nullptr` in this case. I don't think `UMH` gets to decide whether `!nullptr` holder is always alive or not, and it is safer to hold on to it.
>
>> I don't think UMH gets to decide whether !nullptr holder is always alive or not, and it is safer to hold on to it.
> 
> I looked around and stumbled upon the following code in `ClassLoaderData` [1]. I haven't checked myself, but it looks like a hidden class injected into bootstrap loader has `klass_holder == nullptr` while still is amenable to GC... 
> 
> IMO a check for `ik->class_loader_data()->is_permanent_class_loader_data()` would do a better job serving the immediate needs and communicating the intentions.
> 
> [1]
> 
> bool ClassLoaderData::is_permanent_class_loader_data() const {
>   return is_builtin_class_loader_data() && !has_class_mirror_holder();
> }
> 
> // Returns true if the class loader for this class loader data is one of
> // the 3 builtin (boot application/system or platform) class loaders,
> // including a user-defined system class loader.  Note that if the class
> // loader data is for a non-strong hidden class then it may
> // get freed by a GC even if its class loader is one of these loaders.
> bool ClassLoaderData::is_builtin_class_loader_data() const {
> ...

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.

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

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