RFR: 8268406: Deallocate jmethodID native memory [v7]

[Serguei Spitsyn]

On Wed, 18 Jun 2025 12:46:56 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> This change uses a ConcurrentHashTable to associate Method* with jmethodID, instead of an indirection.  JNI is deprecated in favor of using Panama to call methods, so I don't think we're concerned about JNI performance going forward.  JVMTI uses a lot of jmethodIDs but there aren't any performance tests for JVMTI, but running vmTestbase/nsk/jvmti with in product build with and without this change had no difference in time.
>> 
>> The purpose of this change is to remove the memory leak when you unload classes: we were leaving the jmethodID memory just in case JVMTI code still had references to that jmethodID and instead of crashing, should get nullptr.  With this change, if JVMTI looks up a jmethodID, we've removed it from the table and will return nullptr.  Redefinition and the InstanceKlass::_jmethod_method_ids is somewhat complicated.  When a method becomes "obsolete" in redefinition, which means that the code in the method is changed, afterward creating a jmethodID from an "obsolete" method will create a new entry in the InstanceKlass table.  This mechanism increases the method_idnum to do this.  In the future maybe we could throw NoSuchMethodError if you try to create a jmethodID out of an obsolete method and remove all this code.  But that's not in this change.
>> 
>> Tested with tier1-4, 5-7.
>
> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add a basic gtest.

The update looks good. I've posted several nits though.

src/hotspot/share/classfile/classLoaderData.cpp line 605:

> 603:     JmethodIDTable::remove(mid);
> 604:   }
> 605:   delete _jmethod_ids;

Nit: I'd null out the `_jmethod_ids` field.

src/hotspot/share/memory/universe.cpp line 440:

> 438:     vmSymbols::initialize();
> 439: 
> 440:     // Initialize table for matching jmethodID, before SystemDictionary

Nit: Dot is missed.

src/hotspot/share/oops/instanceKlass.cpp line 4280:

> 4278: }
> 4279: 
> 4280: // This nulls out jmethodIDs for all obsolete methods in the previous version of the 'klass'

Nit: Dot is missed.

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

Marked as reviewed by sspitsyn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/25267#pullrequestreview-2940857934
PR Review Comment: https://git.openjdk.org/jdk/pull/25267#discussion_r2155610947
PR Review Comment: https://git.openjdk.org/jdk/pull/25267#discussion_r2155613281
PR Review Comment: https://git.openjdk.org/jdk/pull/25267#discussion_r2155617141