On Fri, 24 Oct 2025 11:55:28 GMT, Coleen Phillimore <[email protected]> wrote:
>> Patricio Chilano Mateo has updated the pull request incrementally with three
>> additional commits since the last revision:
>>
>> - rename _monitor to _init_lock
>> - Extra comments from Coleen
>> - define methods in resolve_from_cache with TRAPS and use
>> CHECK_AND_CLEAR_PREEMPTED
>
> src/hotspot/cpu/aarch64/continuationFreezeThaw_aarch64.inline.hpp line 347:
>
>> 345: intptr_t* sp = enterSpecial.sp();
>> 346:
>> 347: sp[-1] = (intptr_t)StubRoutines::cont_preempt_stub();
>
> push_cleanup_continuation sets sp[-2]. This doesn't have to set that?
`push_cleanup_continuation()` doesn’t need it. I removed it there and added a
comment on both methods.
> src/hotspot/cpu/x86/continuationFreezeThaw_x86.inline.hpp line 220:
>
>> 218: intptr_t* sp = _top_frame.sp();
>> 219: if (sp != _last_sp_from_frame) {
>> 220: sp[-1] = (intptr_t)_top_frame.pc();
>
> Same coment as aarch64, does this need to set sp[-2] to fp like above? Or
> should it preserve the value? Can you add a comment for each telling why?
Here we need to set it because it’s the value that will be set as
`_last_Java_fp` in the `_anchor`. Added a comment.
> src/hotspot/share/runtime/frame.cpp line 951:
>
>> 949: // code in the interpreter calls a blocking runtime
>> 950: // routine which can cause this code to be executed).
>> 951: // (was bug gri 7/27/98)
>
> I like the refactoring of this condition. It may be finally time to remove
> this line from 1998. I, for one, will miss it but it doesn't really help
> anyone with anything.
Ok, I removed that part of the comment.
> test/jdk/java/lang/Thread/virtual/KlassInit.java line 63:
>
>> 61: * @library /test/lib
>> 62: * @requires vm.debug == true & vm.continuations
>> 63: * @run junit/othervm/timeout=480 -XX:+UnlockDiagnosticVMOptions
>> -XX:+FullGCALot -XX:FullGCALotInterval=1000
>> -XX:CompileCommand=exclude,KlassInit::lambda$testReleaseAtKlassInit*
>> -XX:CompileCommand=exclude,KlassInit$$Lambda*::run
>> -XX:CompileCommand=exclude,KlassInit$1Driver::foo KlassInit
>
> I think you can use multiple lines for the run command and not have to wrap
> like this.
Done.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2466822550
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2466825853
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2466828401
PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2466828651
