Bug ID: JDK-8335059 Consider renaming ClassLoaderData::keep

Type: Enhancement
Component: hotspot
Sub-Component: runtime

Priority: P4
Status: Resolved
Resolution: Fixed

Submitted: 2024-06-25
Updated: 2024-08-12
Resolved: 2024-08-02

JDK 24
24 b10Fixed

When iterating over the ClassLoaderDataGraph and handing out CLDs, you sometimes need to make sure that the CLD and its oops are kept alive. I think it would be easy to think that you should call ClassLoaderData::keep_alive() to do that, just like we have CollectedHeap::keep_alive(oop). However, ClassLoaderData::keep_alive already exists and returns whether the CLD is forcefully kept alive or not. So, what you need to do today is to call ClassLoaderData::holder(), and that will keep the CLD alive.

I propose that we rename the current ClassLoaderData::keep_alive() to something else, and then use ClassLoaderData to apply the "keep alive" property.

Changeset: 328a0533 Branch: master Author: Coleen Phillimore <coleenp@openjdk.org> Date: 2024-08-02 11:47:26 +0000 URL: https://git.openjdk.org/jdk/commit/328a0533b2ee6793130dfb68d931e0ebd60c6b5d
02-08-2024
A pull request was submitted for review. Branch: master URL: https://git.openjdk.org/jdk/pull/20408 Date: 2024-07-31 18:35:12 +0000
31-07-2024
I think that holder_keepalive() has two bad properties: 1) It returns something. One reason for this RFE was to have a function that was didn't return anything. 2) That naming scheme is more obscure. A second reason for this RFE. Could we call this property _strong_count instead? Could currently using this property tend to talk about strong CLDs: ``` CLDClosure* closure = cld->keep_alive() ? strong : weak; ``` This could then be: ``` CLDClosure* closure = cld->is_strong() ? strong : weak; ``` and: ``` // Fast path: If class loader is strong, the holder cannot be unloaded. __ lwz(tmp2, in_bytes(ClassLoaderData::keep_alive_offset()), tmp1_class_loader_data); __ cmpdi(CCR0, tmp2, 0); __ bne(CCR0, skip_barrier); ``` would become: ``` // Fast path: If class loader is strong, the holder cannot be unloaded. __ lwz(tmp2, in_bytes(ClassLoaderData::strong_count_offset()), tmp1_class_loader_data); __ cmpdi(CCR0, tmp2, 0); __ bne(CCR0, skip_barrier); ``` ``` // Unloading support bool ClassLoaderData::is_alive() const { bool alive = keep_alive() // null class loader and incomplete non-strong hidden class. \|\| (_holder.peek() != nullptr); // and not cleaned by the GC weak handle processing. return alive; } ``` would be: ``` // Unloading support bool ClassLoaderData::is_alive() const { bool alive = is_strong() // null class loader and incomplete weak hidden class. \|\| (_holder.peek() != nullptr); // and not cleaned by the GC weak handle processing. return alive; } ```
31-07-2024
From a GCs perspective they are strongly reachable, albeit temporarily strongly-reachable. I think that name is good enough.
31-07-2024
"strong" isn't a great word for this. It used to be is_strongly_reachable, but we want to say that for the refcounting version that we want it to be strongly_reachable even if it isn't.
31-07-2024
I can't really think of a better name for this. It forces the CLD to not be unloaded. Maybe no_unload() ? There's a refcounter, which could be inc_no_unload(), dec_no_unload(). // Unloading support bool ClassLoaderData::is_alive() const { bool alive = keep_alive() // null class loader and incomplete non-strong hidden class. \|\| (_holder.peek() != nullptr); // and not cleaned by the GC weak handle processing. return alive; } Why not just rename holder() => holder_keepalive() vs. holder_no_keepalive() which already exists.
30-07-2024
Here's an example that we have today: void ClassLoaderDataGraph::modules_do_keepalive(void f(ModuleEntry)) { assert_locked_or_safepoint(Module_lock); ClassLoaderDataGraphIterator iter; while (ClassLoaderData cld = iter.get_next()) { // Keep the holder alive. (void)cld->holder(); cld->modules_do(f); } } I would like to change that to: void ClassLoaderDataGraph::modules_do_keepalive(void f(ModuleEntry)) { assert_locked_or_safepoint(Module_lock); ClassLoaderDataGraphIterator iter; while (ClassLoaderData cld = iter.get_next()) { // Keep the CLD alive. cld->keep_alive(); cld->modules_do(f); } }
27-06-2024
This came from a discussion in PR https://github.com/openjdk/jdk/pull/19769#issuecomment-2185885243
27-06-2024
This proposal comes from a discussion with me, Axel, and Coleen. Assigning this to Coleen for now.
25-06-2024

Relates :	JDK-8280454 - G1: ClassLoaderData verification keeps CLDs live that causes problems with VerifyDuringGC during Remark
Relates :	JDK-8326820 - Metadata artificially kept alive