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 jclass is a strong root so jclass will keep the Method alive until the jni local is released. If the method belongs to jclass. I think the whole worry about stale jmethodIDs is if there's native or JVMTI (also native) code that squirrels them away somewhere then tries to call JNI call Method or JVMTI with this old jmethodID value. Once we validate the method inside the VM, a safepoint cannot make the method go away if it's holder is alive. We might need to strengthen this by holding a class holder if we don't already. This is preexisting the change here. The jmethodID table is so that jmethodID isn't a stale pointer itself and doesn't require us to hold a stale pointer, but whether it can return a stale Method* (now and before this change) is something we should figure out how it should work. ------------- PR Comment: https://git.openjdk.org/jdk/pull/25267#issuecomment-3000411580
