Skip to content

8369238: Allow virtual thread preemption on some common class initialization paths #27802

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
+2,059 −426
Open

8369238: Allow virtual thread preemption on some common class initialization paths #27802

Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
60cbca4
v1
pchilano Oct 13, 2025
88844bf
Fix whitespaces
pchilano Oct 14, 2025
d035366
RISC-V support
pchilano Oct 14, 2025
79fce80
Merge branch 'master' into JDK-8369238
pchilano Oct 14, 2025
ac55715
fix verify_frame_kind
pchilano Oct 16, 2025
9f4436a
Use ClassInitMode and shorter enum names
pchilano Oct 20, 2025
30bdf49
Extra comments from David
pchilano Oct 20, 2025
6882db0
Remove extra debugging param
pchilano Oct 20, 2025
6000edb
PPC support
pchilano Oct 20, 2025
a943c83
define methods in resolve_from_cache with TRAPS and use CHECK_AND_CLE…
pchilano Oct 23, 2025
0e483ba
Extra comments from Coleen
pchilano Oct 23, 2025
2786b1d
rename _monitor to _init_lock
pchilano Oct 23, 2025
bea5620
More comments from Coleen
pchilano Oct 27, 2025
1312486
Improve comment in anchor_mark_set_pd
pchilano Oct 27, 2025
3bf8ebd
add const to references
pchilano Oct 28, 2025
4cecd7e
Merge branch 'master' into JDK-8369238
pchilano Oct 29, 2025
c7d6f5c
More fixes from David's comments
pchilano Oct 29, 2025
ffcd92a
Improve comment and assert msg
pchilano Oct 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions src/hotspot/share/runtime/continuationFreezeThaw.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ static bool do_verify_after_thaw(JavaThread* thread, stackChunkOop chunk, output
static void log_frames(JavaThread* thread, bool dolog = true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
static void log_frames(JavaThread* thread, bool dolog = true);
static void log_frames(JavaThread* thread, bool do_log = true);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed it altogether. I used it during debugging when I wanted to disable all frame logging except from certain places. But it's not really needed.

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 { \
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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). 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your patch fixes the issue. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, pushed fix.

vframe* vf = vframe::new_vframe(&fr, &reg_map, current);
compiledVFrame* cvf = compiledVFrame::cast(vf);
Expand Down Expand Up @@ -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);
Expand Down