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

[Kim Barrett]

On Wed, 9 Jul 2025 15:59:03 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> [JDK-8163511](https://bugs.openjdk.org/browse/JDK-8163511) made the `CompileTask` improvement to avoid blocking class unloading if a relevant compile task is in queue. Current code does a sleight-of-hand to make sure the the `method*` in `CompileTask` are still valid before using them. Still a noble goal, so we keep trying to do this.
>> 
>> The code tries to switch weak JNI handle with a strong one when it wants to capture the holder to block unloading. Since we are reusing the same field, we have to do type checks like `JNIHandles::is_weak_global_handle(_method_holder)`. Unfortunately, that type-check goes all the way to `OopStorage` allocation code to verify the handle is really allocated in the relevant `OopStorage`. This takes internal `OopStorage` locks, and thus is slow.
>> 
>> This issue is clearly visible in Leyden, when there are lots of `CompileTask`-s in the queue, dumped by AOT code loader. It also does not help that `CompileTask::select_task` is effectively quadratic in number of methods in queue, so we end up calling `CompileTask::is_unloaded` very often.
>> 
>> It is possible to mitigate this issue by splitting the related fields into weak and strong ones. But as Kim mentions in the bug, we should not be using JNI handles here at all, and instead go directly for relevant `OopStorage`-s. This is what this PR does, among other things that should hopefully make the whole mechanics clearer.
>> 
>> Additional testing:
>>  - [x] Linux x86_64 server fastdebug, `compiler/classUnloading`, 100x still passes; these tests are sensitive to bugs in this code
>>  - [x] Linux x86_64 server fastdebug, `all`
>>  - [x] Linux AArch64 server fastdebug, `all`
>
> Aleksey Shipilev has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 37 commits:
> 
>  - Merge branch 'master' into JDK-8231269-compile-task-weaks
>  - Merge branch 'master' into JDK-8231269-compile-task-weaks
>  - Switch to mutable
>  - Merge branch 'master' into JDK-8231269-compile-task-weaks
>  - More touchups
>  - Spin lock induces false sharing
>  - Merge branch 'master' into JDK-8231269-compile-task-weaks
>  - Merge branch 'master' into JDK-8231269-compile-task-weaks
>  - Rename CompilerTask::is_unloaded back to avoid losing comment context
>  - Simplify select_for_compilation
>  - ... and 27 more: https://git.openjdk.org/jdk/compare/a41d3507...d5a8a27d

src/hotspot/share/oops/unloadableMethodHandle.hpp line 81:

> 79:   friend class VMStructs;
> 80: private:
> 81:   enum State {

Not really a review, just a drive-by comment.
I think the only argument against using an enum class here is the lack of C++20's "using enums"
feature: https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1099r5.html
Personally I'd prefer to just make it an enum class and scope the references. YMMV.

Also, someday we should try to come to some consensus about the naming of constants.

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

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