Remove CRIU Interpreter

- Remove CRIUBytcodeInterpreterCompressed - Remove CRIUBytcodeInterpreterFull - Add J9VM_CRIU_TRANSITION_TO_DEBUG_INTERPRETER flag - Add criuRestoreCheckForDebugInterpreterRequest hook - Add criuRestoreCheckForJdwp hook - Set transition flag if -Xdump, -Xtrace, or JDWP is specified - checkTransitionToDebugInterpreter becomes infallible - Call checkTransitionToDebugInterpreter after restore hooks Issues: #19835 Signed-off-by: Nathan Henderson <[email protected]>
eclipse-openj9 · Sep 23, 2024 · 0839261 · 0839261
1 parent c8520fe
commit 0839261
Show file tree

Hide file tree

Showing 11 changed files with 89 additions and 156 deletions.
diff --git a/runtime/makelib/targets.mk.ftl b/runtime/makelib/targets.mk.ftl
@@ -537,21 +537,13 @@ CLANG_CXXFLAGS+=-std=c++0x -D_CRT_SUPPRESS_RESTRICT -DVS12AndHigher
 	CLANG_CXXFLAGS+=-D_M_X64
 </#if>
 endif
-# special handling MHInterpreterFull.cpp, MHInterpreterCompressed.cpp, BytecodeInterpreterFull.cpp, BytecodeInterpreterCompressed.cpp, CRIUBytecodeInterpreterFull.cpp, CRIUBytecodeInterpreterCompressed.cpp, DebugBytecodeInterpreterFull.cpp and DebugBytecodeInterpreterCompressed.cpp
+# special handling MHInterpreterFull.cpp, MHInterpreterCompressed.cpp, BytecodeInterpreterFull.cpp, BytecodeInterpreterCompressed.cpp, DebugBytecodeInterpreterFull.cpp and DebugBytecodeInterpreterCompressed.cpp
 BytecodeInterpreterFull$(UMA_DOT_O):BytecodeInterpreterFull.cpp
 	$(CLANG_CXX) $(CLANG_CXXFLAGS) -c $< -o $@
 
 BytecodeInterpreterCompressed$(UMA_DOT_O):BytecodeInterpreterCompressed.cpp
 	$(CLANG_CXX) $(CLANG_CXXFLAGS) -c $< -o $@
 
-ifneq ($(J9VM_OPT_CRIU_SUPPORT),)
-CRIUBytecodeInterpreterFull$(UMA_DOT_O):CRIUBytecodeInterpreterFull.cpp
-	$(CLANG_CXX) $(CLANG_CXXFLAGS) -c $< -o $@
-
-CRIUBytecodeInterpreterCompressed$(UMA_DOT_O):CRIUBytecodeInterpreterCompressed.cpp
-	$(CLANG_CXX) $(CLANG_CXXFLAGS) -c $< -o $@
-endif
-
 DebugBytecodeInterpreterFull$(UMA_DOT_O):DebugBytecodeInterpreterFull.cpp
 	$(CLANG_CXX) $(CLANG_CXXFLAGS) -c $< -o $@
 
@@ -582,7 +574,7 @@ SharedService$(UMA_DOT_O):SharedService.c
 
 <#if uma.spec.processor.ppc>
 ifndef USE_PPC_GCC
-# special handling BytecodeInterpreterFull.cpp, BytecodeInterpreterCompressed.cpp, CRIUBytecodeInterpreterFull.cpp, CRIUBytecodeInterpreterCompressed.cpp, DebugBytecodeInterpreterFull.cpp and DebugBytecodeInterpreterCompressed.cpp
+# special handling BytecodeInterpreterFull.cpp, BytecodeInterpreterCompressed.cpp, DebugBytecodeInterpreterFull.cpp and DebugBytecodeInterpreterCompressed.cpp
 FLAGS_TO_REMOVE += -O3
 NEW_OPTIMIZATION_FLAG=-O2 -qdebug=lincomm:ptranl:tfbagg
 <#if uma.spec.type.linux>
@@ -597,14 +589,6 @@ BytecodeInterpreterFull$(UMA_DOT_O):BytecodeInterpreterFull.cpp
 BytecodeInterpreterCompressed$(UMA_DOT_O):BytecodeInterpreterCompressed.cpp
 	$(CXX) $(SPECIALCXXFLAGS) $(NEW_OPTIMIZATION_FLAG) -c $<
 
-ifneq ($(J9VM_OPT_CRIU_SUPPORT),)
-CRIUBytecodeInterpreterFull$(UMA_DOT_O):CRIUBytecodeInterpreterFull.cpp
-	$(CXX) $(SPECIALCXXFLAGS) $(NEW_OPTIMIZATION_FLAG) -c $<
-
-CRIUBytecodeInterpreterCompressed$(UMA_DOT_O):CRIUBytecodeInterpreterCompressed.cpp
-	$(CXX) $(SPECIALCXXFLAGS) $(NEW_OPTIMIZATION_FLAG) -c $<
-endif
-
 DebugBytecodeInterpreterFull$(UMA_DOT_O):DebugBytecodeInterpreterFull.cpp
 	$(CXX) $(SPECIALCXXFLAGS) $(NEW_OPTIMIZATION_FLAG) -c $<
 

diff --git a/runtime/makelib/targets.mk.linux.inc.ftl b/runtime/makelib/targets.mk.linux.inc.ftl
@@ -532,22 +532,14 @@ bcverify$(UMA_DOT_O) : bcverify.c
 
 ifdef USE_PPC_GCC
 
-# special handling MHInterpreterFull.cpp, MHInterpreterCompressed.cpp, BytecodeInterpreterFull.cpp, BytecodeInterpreterCompressed.cpp, CRIUBytecodeInterpreterFull.cpp, CRIUBytecodeInterpreterCompressed.cpp, DebugBytecodeInterpreterFull.cpp and DebugBytecodeInterpreterCompressed.cpp
+# special handling MHInterpreterFull.cpp, MHInterpreterCompressed.cpp, BytecodeInterpreterFull.cpp, BytecodeInterpreterCompressed.cpp, DebugBytecodeInterpreterFull.cpp and DebugBytecodeInterpreterCompressed.cpp
 
 BytecodeInterpreterFull$(UMA_DOT_O) : BytecodeInterpreterFull.cpp
 	$(PPC_GCC_CXX) $(PPC_GCC_CXXFLAGS) -c $<
 
 BytecodeInterpreterCompressed$(UMA_DOT_O) : BytecodeInterpreterCompressed.cpp
 	$(PPC_GCC_CXX) $(PPC_GCC_CXXFLAGS) -c $<
 
-ifneq ($(J9VM_OPT_CRIU_SUPPORT),)
-CRIUBytecodeInterpreterFull$(UMA_DOT_O) : CRIUBytecodeInterpreterFull.cpp
-	$(PPC_GCC_CXX) $(PPC_GCC_CXXFLAGS) -c $<
-
-CRIUBytecodeInterpreterCompressed$(UMA_DOT_O) : CRIUBytecodeInterpreterCompressed.cpp
-	$(PPC_GCC_CXX) $(PPC_GCC_CXXFLAGS) -c $<
-endif
-
 DebugBytecodeInterpreterFull$(UMA_DOT_O) : DebugBytecodeInterpreterFull.cpp
 	$(PPC_GCC_CXX) $(PPC_GCC_CXXFLAGS) -c $<
 

diff --git a/runtime/oti/j9nonbuilder.h b/runtime/oti/j9nonbuilder.h
@@ -4318,6 +4318,7 @@ typedef struct J9DelayedLockingOpertionsRecord {
 #define J9VM_CRIU_ENABLE_CRIU_SEC_PROVIDER 0x40
 #define J9VM_CRAC_IS_CHECKPOINT_ENABLED 0x80
 #define J9VM_CRIU_SUPPORT_DEBUG_ON_RESTORE 0x100
+#define J9VM_CRIU_TRANSITION_TO_DEBUG_INTERPRETER 0x200
 
 /* matches maximum count defined by JDWP in threadControl.c */
 #define J9VM_CRIU_MAX_DEBUG_THREADS_STORED 10

diff --git a/runtime/vm/CMakeLists.txt b/runtime/vm/CMakeLists.txt
@@ -217,13 +217,6 @@ set(interpreter_sources
 	DebugBytecodeInterpreterFull.cpp
 )
 
-if(J9VM_OPT_CRIU_SUPPORT)
-	list(APPEND interpreter_sources
-		CRIUBytecodeInterpreterCompressed.cpp
-		CRIUBytecodeInterpreterFull.cpp
-	)
-endif()
-
 if(J9VM_OPT_METHOD_HANDLE)
 	list(APPEND interpreter_sources
 		MHInterpreterCompressed.cpp

diff --git a/runtime/vm/CRIUBytecodeInterpreterCompressed.cpp b/runtime/vm/CRIUBytecodeInterpreterCompressed.cpp
diff --git a/runtime/vm/CRIUBytecodeInterpreterFull.cpp b/runtime/vm/CRIUBytecodeInterpreterFull.cpp
diff --git a/runtime/vm/CRIUHelpers.cpp b/runtime/vm/CRIUHelpers.cpp
@@ -58,6 +58,7 @@ static BOOLEAN criuRestoreInitializeTrace(J9VMThread *currentThread, void *userD
 static BOOLEAN criuRestoreInitializeXrs(J9VMThread *currentThread, void *userData, const char **nlsMsgFormat);
 static BOOLEAN criuRestoreDisableSharedClassCache(J9VMThread *currentThread, void *userData, const char **nlsMsgFormat);
 static BOOLEAN criuRestoreInitializeDump(J9VMThread *currentThread, void *userData, const char **nlsMsgFormat);
+static BOOLEAN criuRestoreCheckForDebugInterpreterRequest(J9VMThread *currentThread, void *userData, const char **nlsMsgFormat);
 static jvmtiIterationControl objectIteratorCallback(J9JavaVM *vm, J9MM_IterateObjectDescriptor *objectDesc, void *userData);
 #if defined(OMR_GC_FULL_POINTERS)
 UDATA debugBytecodeLoopFull(J9VMThread *currentThread);
@@ -187,6 +188,12 @@ isDebugOnRestoreEnabled(J9VMThread *currentThread)
 			&& isCRaCorCRIUSupportEnabled(currentThread);
 }
 
+BOOLEAN
+isTransitionToDebugInterpreterRequested_VM(J9JavaVM *vm)
+{
+	return J9_ARE_ALL_BITS_SET(vm->checkpointState.flags, J9VM_CRIU_TRANSITION_TO_DEBUG_INTERPRETER) && isCRaCorCRIUSupportEnabled_VM(vm);
+}
+
 void
 setRequiredGhostFileLimit(J9VMThread *currentThread, U_32 ghostFileLimit)
 {
@@ -321,6 +328,8 @@ criuRestoreInitializeTrace(J9VMThread *currentThread, void *userData, const char
 				*nlsMsgFormat = j9nls_lookup_message(J9NLS_DO_NOT_PRINT_MESSAGE_TAG | J9NLS_DO_NOT_APPEND_NEWLINE,
 						J9NLS_VM_CRIU_RESTORE_INITIALIZE_TRACE_FAILED, NULL);
 				result = FALSE;
+			} else {
+				vm->checkpointState.flags |=  J9VM_CRIU_TRANSITION_TO_DEBUG_INTERPRETER;
 			}
 		}
 	}
@@ -427,12 +436,61 @@ criuRestoreInitializeDump(J9VMThread *currentThread, void *userData, const char
 				*nlsMsgFormat = j9nls_lookup_message(J9NLS_DO_NOT_PRINT_MESSAGE_TAG | J9NLS_DO_NOT_APPEND_NEWLINE,
 						J9NLS_VM_CRIU_RESTORE_INITIALIZE_DUMP_FAILED, NULL);
 				result = FALSE;
+			} else {
+				vm->checkpointState.flags |=  J9VM_CRIU_TRANSITION_TO_DEBUG_INTERPRETER;
 			}
 		}
 	}
 	return result;
 }
 
+/**
+ * An internal JVM checkpoint hook to check for a debug interpreter request after restore.
+ *
+ * @param[in] currentThread vmThread token
+ * @param[in] userData J9InternalHookRecord pointer
+ * @param[in/out] nlsMsgFormat an NLS message
+ *
+ * @return BOOLEAN TRUE
+ */
+static BOOLEAN
+criuRestoreCheckForDebugInterpreterRequest(J9VMThread *currentThread, void *userData, const char **nlsMsgFormat)
+{
+	J9JavaVM *vm = currentThread->javaVM;
+
+	if (NULL != vm->checkpointState.restoreArgsList) {
+		J9VMInitArgs *restoreArgsList = vm->checkpointState.restoreArgsList;
+		IDATA debugOn = FIND_AND_CONSUME_ARG(restoreArgsList, EXACT_MATCH, VMOPT_XXDEBUGINTERPRETER, NULL);
+		IDATA debugOff = FIND_AND_CONSUME_ARG(restoreArgsList, EXACT_MATCH, VMOPT_XXNODEBUGINTERPRETER, NULL);
+		if (debugOn > debugOff) {
+			vm->checkpointState.flags |= J9VM_CRIU_TRANSITION_TO_DEBUG_INTERPRETER;
+		}
+	}
+
+	return TRUE;
+}
+
+/**
+ * An internal JVM checkpoint hook to check for a JDWP request after restore.
+ *
+ * @param[in] currentThread vmThread token
+ * @param[in] userData J9InternalHookRecord pointer
+ * @param[in/out] nlsMsgFormat an NLS message
+ *
+ * @return BOOLEAN TRUE
+ */
+static BOOLEAN
+criuRestoreCheckForJdwp(J9VMThread *currentThread, void *userData, const char **nlsMsgFormat)
+{
+	J9JavaVM *vm = currentThread->javaVM;
+
+	if (J9_ARE_ALL_BITS_SET(vm->checkpointState.flags, J9VM_CRIU_IS_JDWP_ENABLED)) {
+		vm->checkpointState.flags |= J9VM_CRIU_TRANSITION_TO_DEBUG_INTERPRETER;
+	}
+
+	return TRUE;
+}
+
 /**
  * This cleans up the instanceObjects associated with each J9JavaVM->checkpointState.hookRecords,
  * the hookRecords and classIterationRestoreHookRecords as well if checkpointState.isNonPortableRestoreMode is TRUE.
@@ -538,6 +596,8 @@ initializeCriuHooks(J9VMThread *currentThread)
 		addInternalJVMCheckpointHook(currentThread, TRUE, NULL, FALSE, criuRestoreInitializeTrace);
 		addInternalJVMCheckpointHook(currentThread, TRUE, NULL, FALSE, criuRestoreInitializeXrs);
 		addInternalJVMCheckpointHook(currentThread, TRUE, NULL, FALSE, criuRestoreInitializeDump);
+		addInternalJVMCheckpointHook(currentThread, TRUE, NULL, FALSE, criuRestoreCheckForJdwp);
+		addInternalJVMCheckpointHook(currentThread, TRUE, NULL, FALSE, criuRestoreCheckForDebugInterpreterRequest);
 	}
 
 	addInternalJVMCheckpointHook(currentThread, TRUE, NULL, FALSE, criuRestoreDisableSharedClassCache);
@@ -1485,28 +1545,14 @@ transitionToDebugInterpreter(J9JavaVM *vm)
 	}
 }
 
-static BOOLEAN
+static void
 checkTransitionToDebugInterpreter(J9VMThread *currentThread)
 {
-	BOOLEAN result = TRUE;
 	J9JavaVM *vm = currentThread->javaVM;
-	if (NULL != vm->checkpointState.restoreArgsList) {
-		J9VMInitArgs *restoreArgsList = vm->checkpointState.restoreArgsList;
-		IDATA debugOn = FIND_AND_CONSUME_ARG(restoreArgsList, EXACT_MATCH, VMOPT_XXDEBUGINTERPRETER, NULL);
-		IDATA debugOff = FIND_AND_CONSUME_ARG(restoreArgsList, EXACT_MATCH, VMOPT_XXNODEBUGINTERPRETER, NULL);
-		if (debugOn > debugOff) {
-			/*
-			 * The transition to the debug interpreter currently only works with -Xint,
-			 * and the null check for vm->jitConfig will be removed when the jit changes are completed.
-			 */
-			if (isDebugOnRestoreEnabled(currentThread) && (NULL == vm->jitConfig)) {
-				transitionToDebugInterpreter(vm);
-			} else {
-				result = FALSE;
-			}
-		}
+
+	if (isTransitionToDebugInterpreterRequested_VM(vm)) {
+		transitionToDebugInterpreter(vm);
 	}
-	return result;
 }
 
 void JNICALL
@@ -1910,15 +1956,6 @@ criuCheckpointJVMImpl(JNIEnv *env,
 			case RESTORE_ARGS_RETURN_OK:
 				break;
 			}
-
-			if (!checkTransitionToDebugInterpreter(currentThread)) {
-				currentExceptionClass = vm->checkpointState.criuJVMRestoreExceptionClass;
-				nlsMsgFormat = j9nls_lookup_message(
-					J9NLS_DO_NOT_PRINT_MESSAGE_TAG | J9NLS_DO_NOT_APPEND_NEWLINE,
-					J9NLS_VM_CRIU_CHECK_TRANSITION_TO_DEBUG_INTERPRETER_FAILED,
-					NULL);
-				goto wakeJavaThreadsWithExclusiveVMAccess;
-			}
 		}
 
 		if (hasDumpSucceeded) {
@@ -1932,6 +1969,9 @@ criuCheckpointJVMImpl(JNIEnv *env,
 			currentExceptionClass = vm->checkpointState.criuJVMRestoreExceptionClass;
 			goto wakeJavaThreadsWithExclusiveVMAccess;
 		}
+		if (hasDumpSucceeded) {
+			checkTransitionToDebugInterpreter(currentThread);
+		}
 		if (J9_ARE_ANY_BITS_SET(vm->checkpointState.flags, J9VM_CRIU_IS_JDWP_ENABLED)) {
 			/* Resuming the threads marked with J9_PRIVATE_FLAGS2_DELAY_HALT_FOR_CHECKPOINT
 			 * is only required for JDWP threads.

diff --git a/runtime/vm/jvminit.c b/runtime/vm/jvminit.c
@@ -326,17 +326,11 @@ J9_DECLARE_CONSTANT_UTF8(j9_dispatch, "dispatch");
 /* The appropriate bytecodeLoop is selected based on interpreter mode */
 #if defined(OMR_GC_FULL_POINTERS)
 UDATA bytecodeLoopFull(J9VMThread *currentThread);
-#if defined(J9VM_OPT_CRIU_SUPPORT)
-UDATA criuBytecodeLoopFull(J9VMThread *currentThread);
-#endif /* defined(J9VM_OPT_CRIU_SUPPORT) */
 UDATA debugBytecodeLoopFull(J9VMThread *currentThread);
 #endif /* defined(OMR_GC_FULL_POINTERS) */
 
 #if defined(OMR_GC_COMPRESSED_POINTERS)
 UDATA bytecodeLoopCompressed(J9VMThread *currentThread);
-#if defined(J9VM_OPT_CRIU_SUPPORT)
-UDATA criuBytecodeLoopCompressed(J9VMThread *currentThread);
-#endif /* defined(J9VM_OPT_CRIU_SUPPORT) */
 UDATA debugBytecodeLoopCompressed(J9VMThread *currentThread);
 #endif /* defined(OMR_GC_COMPRESSED_POINTERS) */
 
@@ -3009,21 +3003,7 @@ VMInitStages(J9JavaVM *vm, IDATA stage, void* reserved)
 					vm->bytecodeLoop = debugBytecodeLoopFull;
 #endif /* defined(OMR_GC_FULL_POINTERS) */
 				}
-			} else
-#if defined(J9VM_OPT_CRIU_SUPPORT)
-			if (J9_ARE_ALL_BITS_SET(vm->checkpointState.flags, J9VM_CRIU_IS_CHECKPOINT_ALLOWED)) {
-				if (J9JAVAVM_COMPRESS_OBJECT_REFERENCES(vm)) {
-#if defined(OMR_GC_COMPRESSED_POINTERS)
-					vm->bytecodeLoop = criuBytecodeLoopCompressed;
-#endif /* defined(OMR_GC_COMPRESSED_POINTERS) */
-				} else {
-#if defined(OMR_GC_FULL_POINTERS)
-					vm->bytecodeLoop = criuBytecodeLoopFull;
-#endif /* defined(OMR_GC_FULL_POINTERS) */
-				}
-			} else
-#endif /* defined(J9VM_OPT_CRIU_SUPPORT) */
-			{
+			} else {
 				if (J9JAVAVM_COMPRESS_OBJECT_REFERENCES(vm)) {
 #if defined(OMR_GC_COMPRESSED_POINTERS)
 					vm->bytecodeLoop = bytecodeLoopCompressed;

diff --git a/test/functional/cmdLineTests/criu/criu_nonPortable.xml b/test/functional/cmdLineTests/criu/criu_nonPortable.xml
@@ -763,10 +763,10 @@
     <output type="failure" caseSensitive="yes" regex="no">org.eclipse.openj9.criu.JVMRestoreException</output>
     <output type="failure" caseSensitive="yes" regex="no">User requested Java dump using</output>
   </test>
-
-  <test id="Test checkTransitionToDebugInterpreter with env var file">
+  <!-- "Test debug interpreter transition request via XX:+DebugInterpreter with env var file" -->
+  <test id="Envvar test17">
     <!-- The transition to the debug interpreter currently only works with -Xint, which will be removed when the jit changes are completed. -->
-    <command>bash $SCRIPPATH$ $TEST_RESROOT$ $JAVA_COMMAND$ "$JVM_OPTIONS$ -XX:+DebugOnRestore -Xint" $MAINCLASS_ENVVAR_TEST$ testCheckTransitionToDebugInterpreterWithEnvVarFile 1 false false</command>
+    <command>bash $SCRIPPATH$ $TEST_RESROOT$ $JAVA_COMMAND$ "$JVM_OPTIONS$ -Xint" $MAINCLASS_ENVVAR_TEST$ EnvVarFileTest17 1 false false</command>
     <output type="success" caseSensitive="no" regex="no">Killed</output>
     <output type="required" caseSensitive="yes" regex="no">Pre-checkpoint</output>
     <output type="success" caseSensitive="yes" regex="no">Post-checkpoint</output>
@@ -926,9 +926,9 @@
     <output type="failure" caseSensitive="yes" regex="no">User requested Java dump using</output>
   </test>
 
-  <test id="Test checkTransitionToDebugInterpreter with options file">
+  <test id="Test debug interpreter transition request via XX:+DebugInterpreter with options file">
     <!-- The transition to the debug interpreter currently only works with -Xint, which will be removed when the jit changes are completed. -->
-    <command>bash $SCRIPPATH$ $TEST_RESROOT$ $JAVA_COMMAND$ "$JVM_OPTIONS$ -XX:+DebugOnRestore -Xint" $MAINCLASS_OPTIONSFILE_TEST$ testCheckTransitionToDebugInterpreterWithOptionsFile 1 false false</command>
+    <command>bash $SCRIPPATH$ $TEST_RESROOT$ $JAVA_COMMAND$ "$JVM_OPTIONS$ -Xint" $MAINCLASS_OPTIONSFILE_TEST$ testTransitionToDebugInterpreterViaXXDebugInterpreterWithOptionsFile 1 false false</command>
     <output type="success" caseSensitive="no" regex="no">Killed</output>
     <output type="required" caseSensitive="yes" regex="no">Pre-checkpoint</output>
     <output type="success" caseSensitive="yes" regex="no">Post-checkpoint</output>