On Fri, 17 Jan 2025 20:31:27 GMT, Chris Plummer <cjplum...@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
