On Sat, 26 Oct 2024 06:51:08 GMT, Richard Reingruber <rr...@openjdk.org> wrote:
>> src/hotspot/cpu/aarch64/interp_masm_aarch64.cpp line 1555: >> >>> 1553: // Make VM call. In case of preemption set last_pc to the one we >>> want to resume to. >>> 1554: adr(rscratch1, resume_pc); >>> 1555: str(rscratch1, Address(rthread, JavaThread::last_Java_pc_offset())); >> >> Is it really needed to set an alternative last_Java_pc()? I couldn't find >> where it's used in a way that would require a different value. > > Its indeed difficult to see how the value is propagaged. I think it goes like > this: > > - read from the frame anchor and set as pc of `_last_frame`: > https://github.com/pchilano/jdk/blob/66d5385f8a1c84e73cdbf385239089a7a9932a9e/src/hotspot/share/runtime/continuationFreezeThaw.cpp#L517 > - copied to the result of `new_heap_frame`: > https://github.com/pchilano/jdk/blob/66d5385f8a1c84e73cdbf385239089a7a9932a9e/src/hotspot/cpu/aarch64/continuationFreezeThaw_aarch64.inline.hpp#L99 > - Written to the frame here: > https://github.com/pchilano/jdk/blob/66d5385f8a1c84e73cdbf385239089a7a9932a9e/src/hotspot/cpu/aarch64/continuationFreezeThaw_aarch64.inline.hpp#L177 > - Here it's done when freezing fast: > https://github.com/pchilano/jdk/blob/66d5385f8a1c84e73cdbf385239089a7a9932a9e/src/hotspot/share/runtime/continuationFreezeThaw.cpp#L771 Thanks, that's what I was missing. >> src/hotspot/cpu/aarch64/interp_masm_aarch64.cpp line 1567: >> >>> 1565: >>> 1566: // In case of preemption, this is where we will resume once we >>> finally acquire the monitor. >>> 1567: bind(resume_pc); >> >> If the idea is that we return directly to `resume_pc`, because of >> `last_Java_pc`(), then why do we poll `preempt_alternate_return_offset` >> above? > > The address at `preempt_alternate_return_offset` is how to continue > immediately after the call was preempted. It's where the vthread frames are > popped off the carrier stack. > > At `resume_pc` execution continues when the vthread becomes runnable again. > Before its frames were thawed and copied to its carriers stack. OK, that makes sense now. >> src/hotspot/cpu/x86/stubGenerator_x86_64.cpp line 3796: >> >>> 3794: __ movbool(rscratch1, Address(r15_thread, >>> JavaThread::preemption_cancelled_offset())); >>> 3795: __ testbool(rscratch1); >>> 3796: __ jcc(Assembler::notZero, preemption_cancelled); >> >> If preemption was canceled, then I wouldn't expect >> patch_return_pc_with_preempt_stub() to get called. Does this mean >> preemption can get canceled (asynchronously be a different thread?) even >> afgter patch_return_pc_with_preempt_stub() is called? > > The comment at the `preemption_cancelled` label explains that a second > attempt to acquire the monitor succeeded after freezing. The vthread has to > continue execution. For that its frames (removed just above) need to be > thawed again. If preemption was cancelled, we skip over the cleanup. The native frames haven't been unwound yet. So when we call thaw, does it cleanup the native frames first, or does it copy the frames back on top of the existing frames (overwrite)? It seems like we could avoid redundant copying if we could somehow throw out the freeze data and use the native frames still on the stack, which would probably involve not patching in this stub until we know that the preemption wasn't canceled. Some some finalize actions would be delated, like a two-stage commit. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1819586705 PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1819605366 PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1819657858
