On Mon, 2 Nov 2026 14:23:58 GMT, Shiv Shah <[email protected]> wrote:
>> When a virtual thread suspended at a breakpoint gets unmounted and mounted
>> again only the top frame is thawed. The caller stays frozen in the stack
>> chunk and the popped frame returns into the continuation return barrier. The
>> interpreter's popframe entry treats that return address like a deoptimized
>> caller, preserves the arguments, sets the reexecution bit and returns. The
>> barrier then thawed the caller as a plain return. Arguments and bit left
>> behind, no reexecution of the invoke, run() went on with a zero result.
>>
>> Thaw finishes it now. An interpreted caller gets the preserved arguments
>> back on its expression stack and its return slot is patched to a new
>> popframe reexecution entry of the interpreter, the mdp is set back to the
>> invoke as well. A compiled caller frozen in the chunk is deoptimized from
>> UpdateForPopTopFrameClosure::doit the same way set_frame_pop does it. Thaw
>> leaves the arguments alone in that case and unpack_on_stack puts them back.
>> A frozen compiled frame being popped stays opaque.
>>
>> A linkTo caller needs one more thing. The linker consumed the trailing
>> MemberName before the target ran and the preserved arguments are sized by
>> the target, so the MemberName is not among them. Thaw takes it from the
>> caller's local 0 the way the interpreter's popframe entry does, the lookup
>> from member_name_arg_or_null moved into a plain helper both can call. The
>> arguments go one slot up and the MemberName goes back on top.
>>
>> popframes002 runs popframes001 on a virtual thread with the join Chris
>> Plummer wrote enabled by a property in the debuggee. It unmounts the tested
>> thread before the breakpoint so the caller is frozen when popped, and run()
>> is excluded from compilation so this stays the interpreted case.
>> popframes003 is the compiled caller case, -Xcomp with the breakpoint in a
>> separate stop() so testedMethod keeps its compiled frame and run() stays
>> frozen, the debugger pops frame 1 and the debuggee checks with WhiteBox that
>> run() and testedMethod are compiled and that testedMethod ran twice.
>> -XX:+PreserveFramePointer keeps the thaw on the slow path. Both fail on
>> master without the hotspot change. popframes001 itself runs the plain case
>> as before and is off the virtual problem list. If separate copies of the
>> debugger and debuggee are preferred over the properties that is easy to
>> change. PopFrameMethodNameInvariantTest got an unmount mode, the same
>> linkToStatic pop on a virtual thread that sleeps
before the breakpoint so its caller is frozen.
>>
>>...
>
> Shiv Shah has updated the pull request incrementally with three additional
> commits since the last revision:
>
> - Say in the interpreter's popframe comment that the return barrier case is
> covered too
> - Review comments on the tests, virtual and invokeExact ids, isVirtual
> instead of the unmount property
> - Dedicated interpreter entry for the popframe reexecution after thaw, mdp
> reset, review comments
Thanks for the changes, I only have a few extra comments/suggestions.
src/hotspot/cpu/aarch64/templateInterpreterGenerator_aarch64.cpp line 555:
> 553: // Entry for the caller of a frame popped by JVMTI PopFrame when that
> caller was
> 554: // thawed for the return, the invoke is reexecuted. The deopt entry
> already
> 555: // restores sp and esp from the frame.
Suggestion:
// Continuation entry for an interpreted frame thawed after its callee was
// popped by JVMTI PopFrame.
Same on other platforms.
src/hotspot/cpu/aarch64/templateInterpreterGenerator_aarch64.cpp line 558:
> 556: address
> TemplateInterpreterGenerator::generate_popframe_reexecute_entry() {
> 557: address entry = __ pc();
> 558: __ jump_to_entry(Interpreter::deopt_entry(vtos, 0));
Suggestion:
// We reuse the deopt entry with step=0, which restores the interpreter
// state and reexecutes the invoke.
__ jump_to_entry(Interpreter::deopt_entry(vtos, 0));
Same on other platforms (except ppc and s390).
src/hotspot/cpu/aarch64/templateInterpreterGenerator_aarch64.cpp line 1899:
> 1897: // dispatch_next() after the frame is popped, but instead must save
> the
> 1898: // incoming arguments and restore them after deoptimization or thaw
> has
> 1899: // occurred.
Suggestion:
// Check to see whether we are returning to a deoptimized frame or
// into the continuation return barrier (the PopFrame call ensures that
// the caller of the popped frame is either interpreted or compiled and
// deoptimizes it if compiled). In both cases, we can't call dispatch_next()
// after the frame is popped, but instead must save the incoming arguments
// and restore them after deoptimization or thaw has occurred.
Same on other platforms.
src/hotspot/cpu/arm/templateInterpreterGenerator_arm.cpp line 431:
> 429: // thawed for the return. Only jumps to the deopt entry, this port has no
> 430: // continuation support.
> 431: address
> TemplateInterpreterGenerator::generate_popframe_reexecute_entry() {
This method should just return `nullptr` (see
`generate_cont_resume_interpreter_adapter`).
src/hotspot/cpu/ppc/templateInterpreterGenerator_ppc.cpp line 694:
> 692: // Entry for the caller of a frame popped by JVMTI PopFrame when that
> caller was
> 693: // thawed for the return. Only jumps to the deopt entry, the interpreter
> state the
> 694: // deopt blob restores before that entry is not restored here.
@reinrich maybe you could take a look at the required changes here?
src/hotspot/cpu/s390/templateInterpreterGenerator_s390.cpp line 718:
> 716: // Entry for the caller of a frame popped by JVMTI PopFrame when that
> caller was
> 717: // thawed for the return. Only jumps to the deopt entry, the resize to
> top_frame_sp
> 718: // the return entry does is not done here.
@offamitkumar Maybe you could take a look at the required changes here?
src/hotspot/cpu/x86/templateInterpreterGenerator_x86.cpp line 257:
> 255: address entry = __ pc();
> 256: // The thawed frame arrives with an aligned rsp that can sit below the
> 257: // expression stack top, last_sp has the stack bottom at the call.
Suggestion:
// The thawed frame arrives with an aligned rsp that can sit below the
// expression stack top. Restore it from last_sp, which was saved before
// jumping to the callee.
src/hotspot/share/interpreter/templateInterpreter.hpp line 124:
> 122: static EntryPoint _deopt_entry[number_of_deopt_entries]; // entry
> points to return to from a deoptimization
> 123: static address _deopt_reexecute_return_entry;
> 124: static address _popframe_reexecute_entry; //
> reexecutes the invoke in a caller thawed after JVMTI PopFrame
Suggestion:
static address _cont_popframe_reexecute_entry; // reexecutes
the invoke in a caller thawed after JVMTI PopFrame
Following `_cont_resume_interpreter_adapter` style. Related code would need to
be adjusted too.
src/hotspot/share/interpreter/templateInterpreterGenerator.cpp line 264:
> 262: vmassert(return_continuation != nullptr, "return entry not generated
> yet");
> 263: Interpreter::_deopt_reexecute_return_entry =
> generate_deopt_entry_for(vtos, 0, return_continuation);
> 264: Interpreter::_popframe_reexecute_entry =
> generate_popframe_reexecute_entry();
This should be separated with its own `CodeletMark`.
src/hotspot/share/runtime/continuationFreezeThaw.cpp line 1730:
> 1728: // as usual. If the caller is interpreted, we need to copy the arguments
> 1729: // back in the expression stack and patch the return address so that the
> 1730: // invoke is reexecuted.
I would move this comment on top of `popframe_reexecute_invoke` now since it
contains the main logic, and would also adjust it as we don’t know yet if we
are in the pop frame case:
// Check if the callee frame was popped by JVMTI PopFrame. If so, the
interpreter's
// popframe entry has preserved the arguments and set the deopt reexecution bit
as for a
// deoptimized caller. If the top frame is compiled,
UpdateForPopTopFrameClosure already
// marked it for deoptimization, so we just return and let the deoptimization
code handle
// the pop frame as usual. If the top frame is interpreted, we need to copy the
arguments
// back in the expression stack and patch the return address so that the invoke
is reexecuted.
-------------
PR Review: https://git.openjdk.org/jdk/pull/32661#pullrequestreview-5744771209
PR Review Comment: https://git.openjdk.org/jdk/pull/32661#discussion_r4449083208
PR Review Comment: https://git.openjdk.org/jdk/pull/32661#discussion_r4448934107
PR Review Comment: https://git.openjdk.org/jdk/pull/32661#discussion_r4448957041
PR Review Comment: https://git.openjdk.org/jdk/pull/32661#discussion_r4448967974
PR Review Comment: https://git.openjdk.org/jdk/pull/32661#discussion_r4449013349
PR Review Comment: https://git.openjdk.org/jdk/pull/32661#discussion_r4449007031
PR Review Comment: https://git.openjdk.org/jdk/pull/32661#discussion_r4449027639
PR Review Comment: https://git.openjdk.org/jdk/pull/32661#discussion_r4449041319
PR Review Comment: https://git.openjdk.org/jdk/pull/32661#discussion_r4449042915
PR Review Comment: https://git.openjdk.org/jdk/pull/32661#discussion_r4449054434
