RFR: 8346143: add ClearAllFramePops function to speedup debugger single stepping in some cases [v5]

[Serguei Spitsyn]

On Thu, 19 Dec 2024 00:12:52 GMT, Alex Menkov <amenkov at openjdk.org> wrote:

>> Serguei Spitsyn has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   review: removed unneeded check for JvmtiExport::can_post_frame_pop()
>
> src/hotspot/share/prims/jvmtiEnvThreadState.cpp line 261:
> 
>> 259:   Thread *current = Thread::current();
>> 260: #endif
>> 261:   assert(get_thread() == nullptr || get_thread()->is_handshake_safe_for(current),
> 
> Suggestion:
> 
>   assert(get_thread() == nullptr || get_thread()->is_handshake_safe_for(Thread::current()),

Thank you for the comment. Chris suggested the same.
I've fixed all such places in the file to make it consistent.

> test/hotspot/jtreg/serviceability/jvmti/events/FramePop/ClearAllFramePops/libClearAllFramePops.cpp line 44:
> 
>> 42: static
>> 43: bool isTestThread(JNIEnv *jni, jvmtiEnv *jvmti, jthread thr) {
>> 44:   jvmtiThreadInfo inf = get_thread_info(jvmti, jni, thr);
> 
> Only thread name is required, `get_thread_name` can be used

Thanks. Updated now.

> test/hotspot/jtreg/serviceability/jvmti/events/FramePop/ClearAllFramePops/libClearAllFramePops.cpp line 55:
> 
>> 53:   jclass cls;
>> 54:   char *mname, *msig, *csig;
>> 55:   jvmtiThreadInfo inf = get_thread_info(jvmti, jni, thr);
> 
> Only thread name is required, `get_thread_name` can be used

Thank. Updated now.

> test/hotspot/jtreg/serviceability/jvmti/events/FramePop/ClearAllFramePops/libClearAllFramePops.cpp line 81:
> 
>> 79:   jclass cls;
>> 80:   char *csig;
>> 81:   jvmtiThreadInfo inf = get_thread_info(jvmti, jni, thr);
> 
> Looks like this is not needed (and inf.name not deallocated)

Good catch. Removed the line now.

> test/hotspot/jtreg/serviceability/jvmti/events/FramePop/ClearAllFramePops/libClearAllFramePops.cpp line 137:
> 
>> 135:     LOG("(GetCapabilities) unexpected error: %s (%d)\n", TranslateError(err), err);
>> 136:     return JNI_ERR;
>> 137:   }
> 
> I don't think this is needed

Thanks. Removed now.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22744#discussion_r1891157836
PR Review Comment: https://git.openjdk.org/jdk/pull/22744#discussion_r1891161525
PR Review Comment: https://git.openjdk.org/jdk/pull/22744#discussion_r1891162238
PR Review Comment: https://git.openjdk.org/jdk/pull/22744#discussion_r1891159383
PR Review Comment: https://git.openjdk.org/jdk/pull/22744#discussion_r1891160447