Skip to content

Commit

Permalink
review: remove jvmti annotation for yield methods; simplify frames hi…
Browse files Browse the repository at this point in the history
…ding and events posting code
  • Loading branch information
sspitsyn committed Oct 24, 2024
1 parent 54dc2b4 commit 58ab005
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 54 deletions.
56 changes: 24 additions & 32 deletions src/hotspot/share/prims/jvmtiEnvBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -651,57 +651,49 @@ JavaThread* JvmtiEnvBase::get_JavaThread_or_null(oop vthread) {
return Continuation::is_continuation_mounted(java_thread, cont) ? java_thread : nullptr;
}

// An unmounted vthread always has the yield0() and yield() frames we need to hide.
javaVFrame*
JvmtiEnvBase::skip_top_jvmti_annotated_frames(javaVFrame* jvf) {
// The yield and yield0 may appear in an unmounted virtual thread.
// The notifyJvmti* may appear in both carrier or virtual threads.
for ( ; jvf != nullptr && jvf->method()->jvmti_mount_transition(); jvf = jvf->java_sender()) {
// skip frame with jvmti_mount_transition() annotated method
JvmtiEnvBase::skip_yield_frames_for_unmounted_vthread(javaVFrame* jvf) {
if (jvf == nullptr) { // check for safety
return jvf;
}
jvf = jvf->java_sender(); // skip yield0 frame
assert(jvf != nullptr, "sanity check");
jvf = jvf->java_sender(); // skip yield frame
return jvf;
}

javaVFrame*
JvmtiEnvBase::check_and_skip_hidden_frames(bool is_in_VTMS_transition, javaVFrame* jvf) {
// The second condition is needed to hide notification methods.
if (!is_in_VTMS_transition && (jvf == nullptr || !jvf->method()->jvmti_mount_transition())) {
return jvf; // No frames to skip.
if (jvf == nullptr) {
return jvf;
}
// Find jvf with a method annotated with @JvmtiMountTransition.
for ( ; jvf != nullptr; jvf = jvf->java_sender()) {
if (jvf->method()->jvmti_mount_transition()) {
jvf = jvf->java_sender(); // Skip annotated method.
break;
if (jvf->method()->jvmti_mount_transition()) {
// Skip frames annotated with @JvmtiMountTransition.
for ( ; jvf != nullptr && jvf->method()->jvmti_mount_transition(); jvf = jvf->java_sender()) {
}
if (jvf->method()->changes_current_thread()) {
break;
} else if (is_in_VTMS_transition) {
// Skip frames above the frame annotated with @ChangesCurrentThread.
for ( ; jvf != nullptr && !jvf->method()->changes_current_thread(); jvf = jvf->java_sender()) {
}
// Skip frame above annotated method.
}
return jvf;
}

javaVFrame*
JvmtiEnvBase::check_and_skip_hidden_frames(JavaThread* jt, javaVFrame* jvf) {
bool is_virtual = java_lang_VirtualThread::is_instance(jt->jvmti_vthread());

if (jt->is_in_VTMS_transition()) {
jvf = check_and_skip_hidden_frames(true, jvf);
} else if (is_virtual || jt->last_continuation() == nullptr) { // filter out pure continuations
jvf = skip_top_jvmti_annotated_frames(jvf);
}
jvf = check_and_skip_hidden_frames(jt->is_in_VTMS_transition(), jvf);
return jvf;
}

javaVFrame*
JvmtiEnvBase::check_and_skip_hidden_frames(oop vthread, javaVFrame* jvf) {
assert(java_lang_VirtualThread::is_instance(vthread), "sanity check");
if (java_lang_VirtualThread::is_instance(vthread)) { // paranoid check for safety
if (java_lang_Thread::is_in_VTMS_transition(vthread)) {
jvf = check_and_skip_hidden_frames(true, jvf);
} else {
jvf = skip_top_jvmti_annotated_frames(jvf);
}
if (java_lang_Thread::is_in_VTMS_transition(vthread)) {
jvf = check_and_skip_hidden_frames(true, jvf);
} else {
// if vthread is not in a VTMS transition then it is unmounted
jvf = skip_yield_frames_for_unmounted_vthread(jvf);
}
return jvf;
}
Expand All @@ -726,11 +718,11 @@ JvmtiEnvBase::get_vthread_jvf(oop vthread) {
vframeStream vfs(java_thread);
assert(!java_thread->is_in_VTMS_transition(), "invariant");
jvf = vfs.at_end() ? nullptr : vfs.asJavaVFrame();
jvf = check_and_skip_hidden_frames(java_thread, jvf);
jvf = check_and_skip_hidden_frames(false, jvf);
} else {
vframeStream vfs(cont);
jvf = vfs.at_end() ? nullptr : vfs.asJavaVFrame();
jvf = skip_top_jvmti_annotated_frames(jvf);
jvf = check_and_skip_hidden_frames(vthread, jvf);
}
return jvf;
}
Expand Down Expand Up @@ -1349,7 +1341,7 @@ JvmtiEnvBase::set_frame_pop(JvmtiThreadState* state, javaVFrame* jvf, jint depth
return JVMTI_ERROR_NO_MORE_FRAMES;
}
if (jvf->method()->is_native() || (depth == 0 && state->top_frame_is_exiting()) ||
(state->is_virtual() && (jvf->is_vthread_entry() || jvf->method()->jvmti_mount_transition()))) {
(state->is_virtual() && (jvf->is_vthread_entry() || jvf->method()->jvmti_mount_transition()))) {
return JVMTI_ERROR_OPAQUE_FRAME;
}
assert(jvf->frame_pointer() != nullptr, "frame pointer mustn't be null");
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/prims/jvmtiEnvBase.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ class JvmtiEnvBase : public CHeapObj<mtInternal> {
static bool get_field_descriptor(Klass* k, jfieldID field, fieldDescriptor* fd);

// check and skip frames hidden in mount/unmount transitions
static javaVFrame* skip_top_jvmti_annotated_frames(javaVFrame* jvf);
static javaVFrame* skip_yield_frames_for_unmounted_vthread(javaVFrame* jvf);
static javaVFrame* check_and_skip_hidden_frames(bool is_in_VTMS_transition, javaVFrame* jvf);
static javaVFrame* check_and_skip_hidden_frames(JavaThread* jt, javaVFrame* jvf);
static javaVFrame* check_and_skip_hidden_frames(oop vthread, javaVFrame* jvf);
Expand Down
16 changes: 4 additions & 12 deletions src/hotspot/share/prims/jvmtiExport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1810,9 +1810,7 @@ void JvmtiExport::post_method_entry(JavaThread *thread, Method* method, frame cu
// for any thread that actually wants method entry, interp_only_mode is set
return;
}
// pure continuations have no VTMS transitions but they use methods annotated with JvmtiMountTransition
if ((mh->jvmti_mount_transition() && (state->is_virtual() || thread->last_continuation() == nullptr)) ||
thread->is_in_any_VTMS_transition()) {
if (mh->jvmti_mount_transition() || thread->is_in_any_VTMS_transition()) {
return; // no events should be posted if thread is in any VTMS transition
}
EVT_TRIG_TRACE(JVMTI_EVENT_METHOD_ENTRY, ("[%s] Trg Method Entry triggered %s.%s",
Expand Down Expand Up @@ -1904,9 +1902,7 @@ void JvmtiExport::post_method_exit_inner(JavaThread* thread,
bool exception_exit,
frame current_frame,
jvalue& value) {
// pure continuations have no VTMS transitions but they use methods annotated with JvmtiMountTransition
if ((mh->jvmti_mount_transition() && (state->is_virtual() || thread->last_continuation() == nullptr)) ||
thread->is_in_any_VTMS_transition()) {
if (mh->jvmti_mount_transition() || thread->is_in_any_VTMS_transition()) {
return; // no events should be posted if thread is in any VTMS transition
}

Expand Down Expand Up @@ -1982,9 +1978,7 @@ void JvmtiExport::post_single_step(JavaThread *thread, Method* method, address l
if (state == nullptr) {
return;
}
// pure continuations have no VTMS transitions but they use methods annotated with JvmtiMountTransition
if ((mh->jvmti_mount_transition() && (state->is_virtual() || thread->last_continuation() == nullptr)) ||
thread->is_in_any_VTMS_transition()) {
if (mh->jvmti_mount_transition() || thread->is_in_any_VTMS_transition()) {
return; // no events should be posted if thread is in any VTMS transition
}

Expand Down Expand Up @@ -2148,9 +2142,7 @@ void JvmtiExport::notice_unwind_due_to_exception(JavaThread *thread, Method* met
assert(!state->is_exception_caught(), "exception must not be caught yet.");
state->set_exception_caught();

// pure continuations have no VTMS transitions but they use methods annotated with JvmtiMountTransition
if ((mh->jvmti_mount_transition() && (state->is_virtual() || thread->last_continuation() == nullptr)) ||
thread->is_in_any_VTMS_transition()) {
if (mh->jvmti_mount_transition() || thread->is_in_any_VTMS_transition()) {
return; // no events should be posted if thread is in any VTMS transition
}
JvmtiEnvThreadStateIterator it(state);
Expand Down
2 changes: 0 additions & 2 deletions src/java.base/share/classes/jdk/internal/vm/Continuation.java
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,6 @@ private boolean isEmpty() {
* @throws IllegalStateException if not currently in the given {@code scope},
*/
@Hidden
@JvmtiMountTransition
public static boolean yield(ContinuationScope scope) {
Continuation cont = JLA.getContinuation(currentCarrierThread());
Continuation c;
Expand All @@ -354,7 +353,6 @@ public static boolean yield(ContinuationScope scope) {
}

@Hidden
@JvmtiMountTransition
private boolean yield0(ContinuationScope scope, Continuation child) {
preempted = false;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,12 @@
import java.lang.annotation.*;

/**
* A method may be annotated as "jvmti mount transition" to hint
* A method may be annotated with JvmtiMountTransition to hint
* it is desirable to omit it from JVMTI stack traces.
* Normally, a method is annotated as "jvmti mount transition" if it starts
* or ends virtual thread mount state transition (VTMS transition), so that
* the thread identity is not clear or different at method entry and exit.
* Normally, a method is annotated with @JvmtiMountTransition if it starts
* or ends Virtual Thread Mount State (VTMS) transition, so the thread
* identity is undefined or different at method entry and exit.
* Specifically, it is used for all the VirtualThread notifyJvmti* methods.
* The Continuation yield and yield0 frames normally are in VTMS transition
* but can be found out of transition in an unmounted virtual thread.
* This inconsistency is the reason why they also need this annotation.
*
* @implNote
* This annotation is only used for some VirtualThread and Continuation methods.
Expand Down

0 comments on commit 58ab005

Please sign in to comment.