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

[Serguei Spitsyn]

On Tue, 17 Dec 2024 03:27:16 GMT, Chris Plummer <cjplummer at openjdk.org> wrote:

>> Serguei Spitsyn has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   added extra check to post_method_exit_inner before clear_frame_pop to avoid assert
>
> src/hotspot/share/prims/jvmtiEnvBase.cpp line 1365:
> 
>> 1363:   if (ets->is_frame_pop(frame_number)) {
>> 1364:     return JVMTI_ERROR_DUPLICATE;
>> 1365:   }
> 
> It seems that this change is unrelated to ClearAllFramePops and perhaps deserves it's own CR, especially since it is a behavior change.

Okay. Removed the `JVMTI_ERROR_DUPLICATE` related changes. Will fix it separately including the CSR.

> src/hotspot/share/prims/jvmtiEnvThreadState.cpp line 263:
> 
>> 261: #ifdef ASSERT
>> 262:   Thread *current = Thread::current();
>> 263: #endif
> 
> I think you can just get rid of these lines and inline the Thread.current() call below.

Okay, fixed.

> test/hotspot/jtreg/serviceability/jvmti/vthread/MethodExitTest/libMethodExitTest.cpp line 505:
> 
>> 503:   err = jvmti->NotifyFramePop(thread, 0);
>> 504:   check_jvmti_status(jni, err, "VirtualThreadUnmount: error in JVMTI NotifyFramePop");
>> 505: 
> 
> I assume this is needed as the result of your fix to return DUPLICATE for NotifyFramePop. Do we know that it always returns DUPLICATE for this particular call site?

Yes, it is related to the DUPLICATE change.
The test calls JVMTI `NotifyFramePop` in the `VirtualThreadMount`/`VirtualThreadUnmount` event handlers.
One of them is a dup for sure and not needed.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22744#discussion_r1888942395
PR Review Comment: https://git.openjdk.org/jdk/pull/22744#discussion_r1888940655
PR Review Comment: https://git.openjdk.org/jdk/pull/22744#discussion_r1888939839