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

Ping @jbachorik for discussions about the safety of jmethodIDs...

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

PR Comment: https://git.openjdk.org/jdk/pull/25267#issuecomment-3005733626

Reply via email to