Skip to content

Commit

Permalink
[Wisp] Remove CoroutineSupport Lock in Java and unnecessary function …
Browse files Browse the repository at this point in the history
…coroutineMove.

Summary:
In Java17, we protect double linked list of coroutine in C++. So there is no need to use Locker in Java to protect it.
And also, function moveCoroutine is unnecessary, so we should remove it.
Refine nmethods_do mark active.

Test Plan: all wisp tests

Reviewed-by: yulei

Issue:
dragonwell-project#189
  • Loading branch information
nebula-xm committed Jan 4, 2024
1 parent 717f638 commit 0863dba
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 174 deletions.
25 changes: 9 additions & 16 deletions src/hotspot/share/prims/unsafe.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -968,14 +968,6 @@ JVM_ENTRY (jobject, CoroutineSupport_getNextCoroutine(JNIEnv* env, jclass klass,
return JNIHandles::make_local(THREAD, coro->next()->coroutine());
JVM_END

JVM_ENTRY (void, CoroutineSupport_moveCoroutine(JNIEnv* env, jclass klass, jlong coroPtr, jlong targetPtr))
assert(EnableCoroutine, "pre-condition");
Coroutine* coro = (Coroutine*)coroPtr;
Coroutine* target = (Coroutine*)targetPtr;
CoroutineListLocker cll(coro->thread());
Coroutine::move(coro, target);
JVM_END

JVM_ENTRY (void, CoroutineSupport_markThreadCoroutine(JNIEnv* env, jclass klass, jlong coroPtr, jobject coroObj))
assert(EnableCoroutine, "pre-condition");
Coroutine* coro = (Coroutine*)coroPtr;
Expand All @@ -999,14 +991,16 @@ JVM_ENTRY(jboolean, CoroutineSupport_stealCoroutine(JNIEnv* env, jclass klass, j
assert(coro->thread() != thread, "steal from self");
assert(coro->state() != Coroutine::_current, "running");

CoroutineListLocker cll(coro->thread(), thread);
// check if the source thread and the target thread has been scanned or not
// only if hold the CoroutineListLocker, and has the same scanned state, then we can steal
if (coro->thread()->nmethod_traversals() != thread->nmethod_traversals()) {
return false;
{
CoroutineListLocker cll(coro->thread(), thread);
// check if the source thread and the target thread has been scanned or not
// only if hold the CoroutineListLocker, and has the same scanned state, then we can steal
if (coro->thread()->nmethod_traversals() < thread->nmethod_traversals()) {
return false;
}
coro->remove_from_list(coro->thread()->coroutine_list());
coro->insert_into_list(thread->coroutine_list());
}
coro->remove_from_list(coro->thread()->coroutine_list());
coro->insert_into_list(thread->coroutine_list());
// change thread logic
if (coro->last_handle_mark() != NULL) {
coro->last_handle_mark()->change_thread_for_wisp(thread);
Expand Down Expand Up @@ -1148,7 +1142,6 @@ JNINativeMethod coroutine_support_methods[] = {
{CC"setWispBooted", CC "()V", FN_PTR(CoroutineSupport_setWispBooted)},
{CC"stealCoroutine", CC "(J)Z", FN_PTR(CoroutineSupport_stealCoroutine)},
{CC"getNextCoroutine", CC "(J)" COR, FN_PTR(CoroutineSupport_getNextCoroutine)},
{CC"moveCoroutine", CC "(JJ)V", FN_PTR(CoroutineSupport_moveCoroutine)},
{CC"markThreadCoroutine", CC "(J" COBA ")V", FN_PTR(CoroutineSupport_markThreadCoroutine)},
{CC"getCoroutineStack", CC "(J)[" STE, FN_PTR(CoroutineSupport_getCoroutineStack)},
{CC"shouldThrowException0", CC "(J)Z", FN_PTR(CoroutineSupport_shouldThrowException0)},
Expand Down
7 changes: 4 additions & 3 deletions src/hotspot/share/runtime/sweeper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,6 @@ class NMethodMarkingClosure : public HandshakeClosure {
void do_thread(Thread* thread) {
if (thread->is_Java_thread() && ! thread->is_Code_cache_sweeper_thread()) {
thread->as_Java_thread()->nmethods_do(_cl);
if (EnableCoroutine) {
thread->as_Java_thread()->set_nmethod_traversals(NMethodSweeper::traversal_count());
}
}
}
};
Expand Down Expand Up @@ -195,6 +192,10 @@ CodeBlobClosure* NMethodSweeper::prepare_mark_active_nmethods() {
return &mark_activation_closure;
}

CodeBlobClosure* NMethodSweeper::mark_active_closure() {
return &mark_activation_closure;
}

/**
* This function triggers a VM operation that does stack scanning of active
* methods. Stack scanning is mandatory for the sweeper to make progress.
Expand Down
1 change: 1 addition & 0 deletions src/hotspot/share/runtime/sweeper.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ class NMethodSweeper : public AllStatic {
#endif

static CodeBlobClosure* prepare_mark_active_nmethods();
static CodeBlobClosure* mark_active_closure();
static void sweeper_loop();
static bool should_start_aggressive_sweep();
static void force_sweep();
Expand Down
6 changes: 6 additions & 0 deletions src/hotspot/share/runtime/thread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@
#include "runtime/stackFrameStream.inline.hpp"
#include "runtime/stackWatermarkSet.hpp"
#include "runtime/statSampler.hpp"
#include "runtime/sweeper.hpp"
#include "runtime/task.hpp"
#include "runtime/thread.inline.hpp"
#include "runtime/threadCritical.hpp"
Expand Down Expand Up @@ -2179,6 +2180,11 @@ void JavaThread::nmethods_do(CodeBlobClosure* cf) {
current->compiledMethods_do(cf);
current = current->next();
} while (current != _coroutine_list);

if (NMethodSweeper::mark_active_closure() == cf) {
// mark for current thread has been scanned.
set_nmethod_traversals(NMethodSweeper::traversal_count());
}
} else {
if (jvmti_thread_state() != NULL) {
jvmti_thread_state()->nmethods_do(cf);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ static void switchTo(WispTask current, WispTask next, boolean terminal) {
// store load barrier is not necessary
}
if (terminal == true) {
current.carrier.thread.getCoroutineSupport().terminateCoroutine(next.ctx);
current.carrier.thread.getCoroutineSupport().terminateCoroutine();
// should never run here.
assert false: "should not reach here";
} else {
Expand Down
10 changes: 1 addition & 9 deletions src/java.base/share/classes/java/dyn/CoroutineBase.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@ public abstract class CoroutineBase {

boolean finished = false;

boolean needsUnlock = false;

transient CoroutineSupport threadSupport;

/**
Expand Down Expand Up @@ -68,20 +66,14 @@ public abstract class CoroutineBase {
private final void startInternal() {
assert threadSupport.getThread() == SharedSecrets.getJavaLangAccess().currentThread0();
try {
// When we symmetricYieldTo a newly created coroutine,
// we'll expect the new coroutine release lock as soon as possible
threadSupport.beforeResume(this);
run();
} catch (Throwable t) {
if (!(t instanceof CoroutineExitException)) {
t.printStackTrace();
}
} finally {
finished = true;
// threadSupport is fixed by steal()
threadSupport.beforeResume(this);

threadSupport.terminateCoroutine(null);
threadSupport.terminateCoroutine();
}
assert threadSupport.getThread() == SharedSecrets.getJavaLangAccess().currentThread0();
}
Expand Down

0 comments on commit 0863dba

Please sign in to comment.