RFR: 8369238: Allow virtual thread preemption on some common class initialization paths [v2]

[Patricio Chilano Mateo]

On Wed, 15 Oct 2025 16:54:42 GMT, Richard Reingruber <rrich at openjdk.org> wrote:

>> Patricio Chilano Mateo has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains four commits:
>> 
>>  - Merge branch 'master' into JDK-8369238
>>  - RISC-V support
>>  - Fix whitespaces
>>  - v1
>
> src/hotspot/share/runtime/continuationFreezeThaw.cpp line 1751:
> 
>> 1749:                   RegisterMap::ProcessFrames::skip,
>> 1750:                   RegisterMap::WalkContinuation::skip);
>> 1751:       frame fr = top.sender(&reg_map);
> 
> I think there's a problem here. I get an assertion on ppc if `top` is a heap frame (calling from `log_preempt_after_freeze`) because in `frame::sender_raw()` we don't take the path we normally would for a frame on heap. Instead `sender_for_compiled_frame()` is called which uses a constructor that asserts alignment of `sp` (see [here](https://github.com/openjdk/jdk/blob/1bd814c3b24eb7ef5633ee34bb418e0981ca1708/src/hotspot/cpu/ppc/frame_ppc.inline.hpp#L81-L86)). The assertion fails because `_on_heap` is false but should be `true`.
> 
> I think in `sender_raw` `map->in_cont()` should return true if this frame is on heap.
> 
> It's not quite easy to fix though since `top` can also be on stack.

Right, it should be walked as a heap frame. Could you verify if this patch fixes the issue?


diff --git a/src/hotspot/share/runtime/continuationFreezeThaw.cpp b/src/hotspot/share/runtime/continuationFreezeThaw.cpp
index 86c56fe583f..fb1f66c60f4 100644
--- a/src/hotspot/share/runtime/continuationFreezeThaw.cpp
+++ b/src/hotspot/share/runtime/continuationFreezeThaw.cpp
@@ -196,7 +196,7 @@ static bool do_verify_after_thaw(JavaThread* thread, stackChunkOop chunk, output
 static void log_frames(JavaThread* thread, bool dolog = true);
 static void log_frames_after_thaw(JavaThread* thread, ContinuationWrapper& cont, intptr_t* sp);
 static void print_frame_layout(const frame& f, bool callee_complete, outputStream* st = tty);
-static void verify_frame_kind(const frame& top, Continuation::preempt_kind preempt_kind, Method** m_ptr = nullptr, const char** code_name_ptr = nullptr, int* bci_ptr = nullptr);
+static void verify_frame_kind(frame& top, Continuation::preempt_kind preempt_kind, Method** m_ptr = nullptr, const char** code_name_ptr = nullptr, int* bci_ptr = nullptr, stackChunkOop chunk = nullptr);

 #define assert_pfl(p, ...) \
 do {                                           \
@@ -1723,7 +1723,7 @@ bool FreezeBase::check_valid_fast_path() {
   return true;
 }

-static void verify_frame_kind(const frame& top, Continuation::preempt_kind preempt_kind, Method** m_ptr, const char** code_name_ptr, int* bci_ptr) {
+static void verify_frame_kind(frame& top, Continuation::preempt_kind preempt_kind, Method** m_ptr, const char** code_name_ptr, int* bci_ptr, stackChunkOop chunk) {
   Method* m;
   const char* code_name;
   int bci;
@@ -1747,7 +1747,13 @@ static void verify_frame_kind(const frame& top, Continuation::preempt_kind preem
       RegisterMap reg_map(current,
                   RegisterMap::UpdateMap::skip,
                   RegisterMap::ProcessFrames::skip,
-                  RegisterMap::WalkContinuation::skip);
+                  RegisterMap::WalkContinuation::include);
+      if (top.is_heap_frame()) {
+        assert(chunk != nullptr, "");
+        reg_map.set_stack_chunk(chunk);
+        top = chunk->relativize(top);
+        top.set_frame_index(0);
+      }
       frame fr = top.sender(&reg_map);
       vframe*  vf  = vframe::new_vframe(&fr, &reg_map, current);
       compiledVFrame* cvf = compiledVFrame::cast(vf);
@@ -1803,7 +1809,7 @@ static void log_preempt_after_freeze(ContinuationWrapper& cont) {
   Method* m = nullptr;
   const char* code_name = nullptr;
   int bci = InvalidFrameStateBci;
-  verify_frame_kind(top_frame, pk, &m, &code_name, &bci);
+  verify_frame_kind(top_frame, pk, &m, &code_name, &bci, cont.tail());
   assert(m != nullptr && code_name != nullptr && bci != InvalidFrameStateBci, "should be set");

   ResourceMark rm(current);

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27802#discussion_r2433721003