On Wed, 18 Jun 2025 12:46:56 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: > > Add a basic gtest. I feel apprehensive about this; the solution feels pretty complex and I am not fully convinced this is the simplest solution for this problem. How much space to we lose in real life? Side note: I see the payload of the jmethodID block in NMT is allocated with mtInternal, so we don't see it in NMT. We should add jmethodIDs as an own category to NMT. A pragmatic alternative solution could be to do delete them, but delayed: keep the last N methodblocks undeleted. It is rare that JNI accesses jmethodIDs long after they have been deleted. Typically, the bad access happens close after class unloading, e.g. because of concurrency problems in customer code. We could then make the parameter N configurable, and thus give customers and supporters a tool to check for these kind of errors. (I briefly wondered whether we could just mmap these blocks, and uncommit/mprotect them on release, so that we stop paying the memory costs but don't release the address space; but the coarser page size allocation granularity would make this probably forbidding in terms of mem cost per class) ------------- PR Comment: https://git.openjdk.org/jdk/pull/25267#issuecomment-2986801229
