RFR: JDK-8268088: Clarify Method::clear_jmethod_ids() related comments in ClassLoaderData::~ClassLoaderData() [v2]

[Jiangli Zhou]

On Tue, 8 Jun 2021 13:34:43 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

> Hi, I put a short comment in JDK-8268088 which I'll expand later. There's already a JVMTI callback for unloading classes that the user could use to walk the saved jmethodID stacks:
> 
> // notify the debugger
> if (JvmtiExport::should_post_class_unload()) {
> JvmtiExport::post_class_unload(ik);
> }
> 
> Otherwise, don't change class unloading to add a callback because it could be sensitive to safepoints and unsafe places.

A new capability as Ioi suggested in https://bugs.openjdk.java.net/browse/JDK-8268364?focusedCommentId=14426310&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14426310 would be better than using the existing callback for unloading. The possible approach that I described in https://bugs.openjdk.java.net/browse/JDK-8268088?focusedCommentId=14424635&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14424635 also looks for the same direction (however there are still other issues as described in the bug comment).  Also, we need a good strategy for existing code that expects the current behavior. 

> 
> My comment in this bug echoes what Ioi said above. To prevent class unloading, the class mirror should be saved with the Method. This is the approach taken throughout the vm. I think hunting down applications that need to fix their profilers may be difficult, but we could add some deprecation messages and warnings (or even an option, which I thought was what you were going to do).
> 

Prevent or defer class unloading would keep more memory for longer time. It could cause other unwanted side-effects. 

> I'm not sure why so many lines are changed in your above pull request.

Do you refer to https://github.com/openjdk/jdk/commit/66ef04af110862a1a6b727f12ad5eed9c26cd5a9? That's based on my internal change on JDK 11 (with other local changes). I restructured code by moving JNIMethodBlock and JNIMethodBlockNodes class declarations to the header file. That's needed to trigger destructor properly from classLoaderData.cpp. The original logic was kept the same, and only the code was being moved around. 

I opened my internal change for reference purposes and assisting the discussions to address the memory leak.

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

PR: https://git.openjdk.java.net/jdk/pull/4383