On Fri, 20 Jun 2025 20:41:21 GMT, Coleen Phillimore <cole...@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:
> 
>   Fix the test

Thumbs up. Reviewed the changes between v02 and v11
and the changed look good.

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

Marked as reviewed by dcubed (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/25267#pullrequestreview-2951838637

Reply via email to