Merge pull request #16022 from LinHu2016/avoiddoublescan

Avoid double scan stacks in continuation for compact cases
eclipse-openj9 · Oct 5, 2022 · 52ab41f · 52ab41f
2 parents 8faf666 + 304431e
commit 52ab41f
Show file tree

Hide file tree

Showing 10 changed files with 42 additions and 12 deletions.
diff --git a/runtime/gc/gctable.c b/runtime/gc/gctable.c
@@ -252,7 +252,7 @@ J9MemoryManagerFunctions MemoryManagerFunctions = {
 	ownableSynchronizerObjectCreated,
 	continuationObjectCreated,
 	preMountContinuation,
-	postDismountContinuation,
+	postUnmountContinuation,
 	j9gc_notifyGCOfClassReplacement,
 	j9gc_get_jit_string_dedup_policy,
 	j9gc_stringHashFn,

diff --git a/runtime/gc_base/gc_internal.h b/runtime/gc_base/gc_internal.h
@@ -269,7 +269,7 @@ extern J9_CFUNC UDATA ownableSynchronizerObjectCreated(J9VMThread *vmThread, j9o
 extern J9_CFUNC UDATA continuationObjectCreated(J9VMThread *vmThread, j9object_t object);
 
 extern J9_CFUNC void preMountContinuation(J9VMThread *vmThread, j9object_t object);
-extern J9_CFUNC void postDismountContinuation(J9VMThread *vmThread, j9object_t object);
+extern J9_CFUNC void postUnmountContinuation(J9VMThread *vmThread, j9object_t object);
 
 extern J9_CFUNC void j9gc_notifyGCOfClassReplacement(J9VMThread *vmThread, J9Class *originalClass, J9Class *replacementClass, UDATA isFastHCR);
 

diff --git a/runtime/gc_base/modronapi.cpp b/runtime/gc_base/modronapi.cpp
@@ -56,11 +56,10 @@ void
 preMountContinuation(J9VMThread *vmThread, j9object_t object)
 {
 	/* need read barrier to handle concurrent scavenger and gcpolicy:metronome case */
-
 }
 
 void
-postDismountContinuation(J9VMThread *vmThread, j9object_t object)
+postUnmountContinuation(J9VMThread *vmThread, j9object_t object)
 {
 	/* Conservatively assume that via mutations of stack slots (which are not subject to access barriers),
 	 * all post-write barriers have been triggered on this Continuation object, since it's been mounted. */

diff --git a/runtime/gc_base/modronapi.hpp b/runtime/gc_base/modronapi.hpp
@@ -103,7 +103,7 @@ UDATA ownableSynchronizerObjectCreated(J9VMThread *vmThread, j9object_t object);
 
 UDATA continuationObjectCreated(J9VMThread *vmThread, j9object_t object);
 void preMountContinuation(J9VMThread *vmThread, j9object_t object);
-void postDismountContinuation(J9VMThread *vmThread, j9object_t object);
+void postUnmountContinuation(J9VMThread *vmThread, j9object_t object);
 
 /**
  * Called during class redefinition to notify the GC of replaced classes.In certain cases the GC needs to 

diff --git a/runtime/gc_glue_java/CompactSchemeFixupObject.cpp b/runtime/gc_glue_java/CompactSchemeFixupObject.cpp
@@ -73,7 +73,11 @@ MM_CompactSchemeFixupObject::fixupContinuationObject(MM_EnvironmentStandard *env
 	fixupMixedObject(objectPtr);
 	/* fixup Java Stacks in J9VMContinuation */
 	J9VMThread *currentThread = (J9VMThread *)env->getLanguageVMThread();
-	if (VM_VMHelpers::needScanStacksForContinuation(currentThread, objectPtr)) {
+	/* In sliding compaction we must fix slots exactly once. Since we will fixup stack slots of
+	 * mounted Virtual threads later during root fixup, we will skip it during this heap fixup pass
+	 * (hence passing true for scanOnlyUnmounted parameter).
+	 */
+	if (VM_VMHelpers::needScanStacksForContinuation(currentThread, objectPtr, true)) {
 		StackIteratorData4CompactSchemeFixupObject localData;
 		localData.compactSchemeFixupObject = this;
 		localData.env = env;

diff --git a/runtime/gc_vlhgc/WriteOnceCompactor.cpp b/runtime/gc_vlhgc/WriteOnceCompactor.cpp
@@ -1237,7 +1237,11 @@ MM_WriteOnceCompactor::fixupContinuationObject(MM_EnvironmentVLHGC* env, J9Objec
 	fixupMixedObject(env, objectPtr, cache);
 
 	J9VMThread *currentThread = (J9VMThread *)env->getLanguageVMThread();
-	if (VM_VMHelpers::needScanStacksForContinuation(currentThread, objectPtr)) {
+	/* In sliding compaction we must fix slots exactly once. Since we will fixup stack slots of
+	 * mounted Virtual threads later during root fixup, we will skip it during this heap fixup pass
+	 * (hence passing true for scanOnlyUnmounted parameter).
+	 */
+	if (VM_VMHelpers::needScanStacksForContinuation(currentThread, objectPtr, true)) {
 		StackIteratorData4WriteOnceCompactor localData;
 		localData.writeOnceCompactor = this;
 		localData.env = env;

diff --git a/runtime/oti/VMHelpers.hpp b/runtime/oti/VMHelpers.hpp
@@ -2057,13 +2057,34 @@ class VM_VMHelpers
 		return rc;
 	}
 
+#if JAVA_SPEC_VERSION >= 19
+	/**
+	 * Check if the related J9VMContinuation is mounted to carrier thread
+	 * @param[in] continuation the related J9VMContinuation
+	 * @return true if it is mounted.
+	 */
 	static VMINLINE bool
-	needScanStacksForContinuation(J9VMThread *vmThread, j9object_t continuationObject) {
+	isContinuationMounted(J9VMContinuation *continuation)
+	{
+		return (NULL != continuation->carrierThread);
+	}
+#endif /* JAVA_SPEC_VERSION >= 19 */
+
+	/**
+	 * Check if we need to scan the java stack for the Continuation Object
+	 * @param[in] vmThread the current J9VMThread
+	 * @param[in] continuationObject the continuation object
+	 * @param[in] scanOnlyUnmounted if it is true, only scan unmounted continuation object, default is false
+	 * @return true if we need to scan the java stack
+	 */
+	static VMINLINE bool
+	needScanStacksForContinuation(J9VMThread *vmThread, j9object_t continuationObject, bool scanOnlyUnmounted = false)
+	{
 		bool needScan = false;
 #if JAVA_SPEC_VERSION >= 19
 		jboolean started = J9VMJDKINTERNALVMCONTINUATION_STARTED(vmThread, continuationObject);
 		J9VMContinuation *continuation = J9VMJDKINTERNALVMCONTINUATION_VMREF(vmThread, continuationObject);
-		needScan = started && (NULL != continuation);
+		needScan = started && (NULL != continuation) && (!scanOnlyUnmounted || !isContinuationMounted(continuation));
 #endif /* JAVA_SPEC_VERSION >= 19 */
 		return needScan;
 	}

diff --git a/runtime/oti/j9nonbuilder.h b/runtime/oti/j9nonbuilder.h
@@ -4500,7 +4500,7 @@ typedef struct J9MemoryManagerFunctions {
 	UDATA ( *ownableSynchronizerObjectCreated)(struct J9VMThread *vmThread, j9object_t object) ;
 	UDATA ( *continuationObjectCreated)(struct J9VMThread *vmThread, j9object_t object) ;
 	void ( *preMountContinuation)(struct J9VMThread *vmThread, j9object_t object) ;
-	void ( *postDismountContinuation)(struct J9VMThread *vmThread, j9object_t object) ;
+	void ( *postUnmountContinuation)(struct J9VMThread *vmThread, j9object_t object) ;
 
 	void  ( *j9gc_notifyGCOfClassReplacement)(struct J9VMThread *vmThread, J9Class *originalClass, J9Class *replacementClass, UDATA isFastHCR) ;
 	I_32  ( *j9gc_get_jit_string_dedup_policy)(struct J9JavaVM *javaVM) ;
@@ -4998,6 +4998,7 @@ typedef struct J9VMContinuation {
 	UDATA* j2iFrame;
 	struct J9JITGPRSpillArea jitGPRs;
 	struct J9VMEntryLocalStorage* oldEntryLocalStorage;
+	struct J9VMThread* carrierThread;
 } J9VMContinuation;
 #endif /* JAVA_SPEC_VERSION >= 19 */
 

diff --git a/runtime/vm/BytecodeInterpreter.hpp b/runtime/vm/BytecodeInterpreter.hpp
@@ -5232,7 +5232,7 @@ class INTERPRETER_CLASS
 
 		j9object_t continuationObject = J9VMJAVALANGTHREAD_CONT(_currentThread, _currentThread->carrierThreadObject);
 		/* Notify GC of Continuation stack swap */
-		_vm->memoryManagerFunctions->postDismountContinuation(_currentThread, continuationObject);
+		_vm->memoryManagerFunctions->postUnmountContinuation(_currentThread, continuationObject);
 
 		VMStructHasBeenUpdated(REGISTER_ARGS);
 		restoreInternalNativeStackFrame(REGISTER_ARGS);

diff --git a/runtime/vm/ContinuationHelpers.cpp b/runtime/vm/ContinuationHelpers.cpp
@@ -98,6 +98,7 @@ enterContinuation(J9VMThread *currentThread, j9object_t continuationObject)
 	Assert_VM_Null(currentThread->currentContinuation);
 
 	VM_ContinuationHelpers::swapFieldsWithContinuation(currentThread, continuation);
+	continuation->carrierThread = currentThread;
 	currentThread->currentContinuation = continuation;
 
 	/* Reset counters which determine if the current continuation is pinned. */
@@ -142,7 +143,7 @@ yieldContinuation(J9VMThread *currentThread)
 	Assert_VM_notNull(currentThread->currentContinuation);
 
 	VM_ContinuationHelpers::swapFieldsWithContinuation(currentThread, continuation);
-
+	continuation->carrierThread = NULL;
 	currentThread->currentContinuation = NULL;
 
 	/* We need a full fence here to preserve happens-before relationship on PPC and other weakly