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
Another way to look at the safety of the new mechanism is to
consider the use of jmethodIDs from the `AsyncGetCallTrace` API:
src/hotspot/share/prims/forte.cpp:
108 2025.06.25 12:09:14 $ cat /tmp/fred
// Forte Analyzer AsyncGetCallTrace() entry point. Currently supported
// on Linux X86, Solaris SPARC and Solaris X86.
//
// Async-safe version of GetCallTrace being called from a signal handler
// when a LWP gets interrupted by SIGPROF but the stack traces are filled
// with different content (see below).
//
// This function must only be called when JVM/TI
// CLASS_LOAD events have been enabled since agent startup. The enabled
// event will cause the jmethodIDs to be allocated at class load time.
// The jmethodIDs cannot be allocated in a signal handler because locks
// cannot be grabbed in a signal handler safely.
//
// void (*AsyncGetCallTrace)(ASGCT_CallTrace *trace, jint depth, void* ucontext)
//
// Called by the profiler to obtain the current method call stack trace for
// a given thread. The thread is identified by the env_id field in the
// ASGCT_CallTrace structure. The profiler agent should allocate a
ASGCT_CallTrace
// structure with enough memory for the requested stack depth. The VM fills in
// the frames buffer and the num_frames field.
//
// Arguments:
//
// trace - trace data structure to be filled by the VM.
// depth - depth of the call stack trace.
// ucontext - ucontext_t of the LWP
//
// ASGCT_CallTrace:
// typedef struct {
// JNIEnv *env_id;
// jint num_frames;
// ASGCT_CallFrame *frames;
// } ASGCT_CallTrace;
//
// Fields:
// env_id - ID of thread which executed this trace.
// num_frames - number of frames in the trace.
// (< 0 indicates the frame is not walkable).
// frames - the ASGCT_CallFrames that make up this trace. Callee followed
by callers.
//
// ASGCT_CallFrame:
// typedef struct {
// jint lineno;
// jmethodID method_id;
// } ASGCT_CallFrame;
//
// Fields:
// 1) For Java frame (interpreted and compiled),
// lineno - bci of the method being executed or -1 if bci is not
available
// method_id - jmethodID of the method being executed
// 2) For native method
// lineno - (-3)
// method_id - jmethodID of the method being executed
The `AsyncGetCallTrace` API returns jmethodIDs without explicitly
associated jclass references. The `AsyncGetCallTrace` API relies on
JVM/TI CLASS_LOAD events to have been enabled since agent startup for
jmethodID creation. Implicit in that requirement is the agent keeping a
jclass reference for each CLASS_LOAD event so that the class cannot be
unloaded while the data returned by `AsyncGetCallTrace` is processed.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/25267#issuecomment-3005716494
