RFR: 8229012: When single stepping, the debug agent can cause the thread to remain in interpreter mode after single stepping completes

[Serguei Spitsyn]

On Fri, 17 Jan 2025 20:31:27 GMT, Chris Plummer <cjplummer at openjdk.org> wrote:

> When doing a  STEP_OVER, the debug agent does a NotifyFramePop() on the current frame as a safety net. After the STEP_OVER completes, the NotifyFramePop() is usually still in place. This keeps the thread in interp_only mode, which hurts performance. JVMTI has added a new ClearAllFramePops() API to allow clearing of the NotifyFramePop() and normal performance to resume.
> 
> Testing:
> 
> - [x] Tier1 CI
> - [x] Tier2 CI svc testing
> - [x] Tier3 CI svc testing
> - [x] Tier5 CI svc testing
> - [x] ran all svc test 10 times each on all supported platforms

The fix looks good in general but posted some minor comments.

It is nice the update is reasonably simple. :)

src/jdk.jdwp.agent/share/native/libjdwp/stepControl.c line 940:

> 938: 
> 939:         if (needsSuspending) {
> 940:             tty_message("clearStep: resuming thread");

Q: Lines: 913, 940. Should such a message printing be under the `#ifdef DEBUG` ?

test/jdk/com/sun/jdi/SingleStepCompilationTest.java line 30:

> 28:  * @bug 8229012
> 29:  * @summary Verify that during single stepping a method is not compiled and
> 30:  *          after single stepping it is compiled..

Nit: The dots are not needed at the end.

test/jdk/com/sun/jdi/SingleStepCompilationTest.java line 32:

> 30:  *          after single stepping it is compiled..
> 31:  * @requires vm.compMode == "Xmixed"
> 32:  * @library /test/lib /

Q: Just want to make sure the`/` at the end is not a typo. :)

test/jdk/com/sun/jdi/SingleStepCompilationTest.java line 228:

> 226:         /*
> 227:          * Deal with results of test. If anything has called failure("foo")
> 228:          * then testFailed will be true

Nit: Need dots at the end of lines: 200, 207 and 228.

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

PR Review: https://git.openjdk.org/jdk/pull/23182#pullrequestreview-2566380444
PR Comment: https://git.openjdk.org/jdk/pull/23182#issuecomment-2606515615
PR Review Comment: https://git.openjdk.org/jdk/pull/23182#discussion_r1924818178
PR Review Comment: https://git.openjdk.org/jdk/pull/23182#discussion_r1924820257
PR Review Comment: https://git.openjdk.org/jdk/pull/23182#discussion_r1924821505
PR Review Comment: https://git.openjdk.org/jdk/pull/23182#discussion_r1924839521