From d599536f3304c24fe4bd7604e2cf9426875329f0 Mon Sep 17 00:00:00 2001 From: Tobi Ajila Date: Wed, 4 Oct 2023 11:14:48 -0700 Subject: [PATCH] Introduce JVMPortableRestoreMode Previously, the CRIURestoreNonPortableMode was used to determine if a checkpoint could be taken after restore. If CRIURestoreNonPortableMode was enabled (default mode) only a single checkpoint is allowed and the JVM doesn't need to generate portable code upon restore. This capability also allows the JVM to use standard security providers since there is no risk for that state to be serialized. There is a need to separate out the portability capabilites and the behavioural changes that CRIURestoreNonPortableMode offers. There are cases where one wants portability as well as standard security capabilites for debugging purposes. This PR separates out the portability capabilites and the behavioural aspects by providing an option that forces portability upon restore regardless of what CRIURestoreNonPortableMode is set to. In the future we can add another option that forces portability in non-CRIU mode, it its desired. Signed-off-by: Tobi Ajila --- runtime/compiler/compile/J9Compilation.cpp | 5 +++-- runtime/compiler/control/HookedByTheJit.cpp | 2 +- runtime/compiler/control/J9Options.cpp | 4 ++-- .../control/JITClientCompilationThread.cpp | 3 ++- runtime/compiler/control/rossa.cpp | 2 +- runtime/compiler/env/VMJ9.cpp | 10 ++++++++++ runtime/compiler/env/VMJ9.h | 7 +++++++ runtime/compiler/env/VMJ9Server.cpp | 8 ++++++++ runtime/compiler/env/VMJ9Server.hpp | 1 + runtime/compiler/net/CommunicationStream.hpp | 2 +- runtime/compiler/runtime/JITClientSession.hpp | 1 + runtime/oti/j9nonbuilder.h | 2 ++ runtime/oti/jvminit.h | 2 ++ runtime/oti/vm_api.h | 13 +++++++++++++ runtime/vm/CRIUHelpers.cpp | 6 ++++++ runtime/vm/intfunc.c | 1 + runtime/vm/jvminit.c | 10 ++++++++++ test/functional/cmdLineTests/criu/playlist.xml | 4 ++++ .../src/org/openj9/criu/CRIUSimpleTest.java | 17 +++++++++++++++++ 19 files changed, 92 insertions(+), 8 deletions(-) diff --git a/runtime/compiler/compile/J9Compilation.cpp b/runtime/compiler/compile/J9Compilation.cpp index 8545557ef44..4064937c3da 100644 --- a/runtime/compiler/compile/J9Compilation.cpp +++ b/runtime/compiler/compile/J9Compilation.cpp @@ -857,8 +857,9 @@ bool J9::Compilation::compilePortableCode() { return (self()->fej9()->inSnapshotMode() || - (self()->compileRelocatableCode() && - self()->fej9()->isPortableSCCEnabled())); + self()->fej9()->isPortableRestoreModeEnabled() || + (self()->compileRelocatableCode() && + self()->fej9()->isPortableSCCEnabled())); } diff --git a/runtime/compiler/control/HookedByTheJit.cpp b/runtime/compiler/control/HookedByTheJit.cpp index 8208949d498..22dfb172c18 100644 --- a/runtime/compiler/control/HookedByTheJit.cpp +++ b/runtime/compiler/control/HookedByTheJit.cpp @@ -1855,7 +1855,7 @@ static void jitHookPrepareRestore(J9HookInterface * * hookInterface, UDATA event * remove the portability restrictions on the target CPU (used * for JIT compiles) to allow optimal code generation */ - if (!javaVM->internalVMFunctions->isCheckpointAllowed(vmThread)) + if (!javaVM->internalVMFunctions->isJVMInPortableRestoreMode(vmThread)) { TR::Compiler->target.cpu = TR::CPU::detect(TR::Compiler->omrPortLib); jitConfig->targetProcessor = TR::Compiler->target.cpu.getProcessorDescription(); diff --git a/runtime/compiler/control/J9Options.cpp b/runtime/compiler/control/J9Options.cpp index 3591e0579be..61837a47c7a 100644 --- a/runtime/compiler/control/J9Options.cpp +++ b/runtime/compiler/control/J9Options.cpp @@ -1489,7 +1489,7 @@ void J9::Options::preProcessMmf(J9JavaVM *vm, J9JITConfig *jitConfig) if (J9_ARE_ANY_BITS_SET(vm->extendedRuntimeFlags2, J9_EXTENDED_RUNTIME2_ENABLE_PORTABLE_SHARED_CACHE) #if defined(J9VM_OPT_CRIU_SUPPORT) - || vm->internalVMFunctions->isCheckpointAllowed(vmThread) + || vm->internalVMFunctions->isJVMInPortableRestoreMode(vmThread) #endif /* defined(J9VM_OPT_CRIU_SUPPORT) */ ) { @@ -2368,7 +2368,7 @@ bool J9::Options::preProcessJitServer(J9JavaVM *vm, J9JITConfig *jitConfig) if (implicitClientMode && useJitServerExplicitlySpecified) { compInfo->setRemoteCompilationRequestedAtBootstrap(true); - if (!ifuncs->isNonPortableRestoreMode(currentThread)) + if (ifuncs->isJVMInPortableRestoreMode(currentThread)) compInfo->setCanPerformRemoteCompilationInCRIUMode(true); } #endif diff --git a/runtime/compiler/control/JITClientCompilationThread.cpp b/runtime/compiler/control/JITClientCompilationThread.cpp index 7585b5393b8..1856c80c982 100644 --- a/runtime/compiler/control/JITClientCompilationThread.cpp +++ b/runtime/compiler/control/JITClientCompilationThread.cpp @@ -524,9 +524,10 @@ handleServerMessage(JITServer::ClientStream *client, TR_J9VM *fe, JITServer::Mes vmInfo._aotHeader = *header; } vmInfo._inSnapshotMode = fe->inSnapshotMode(); + vmInfo._isPortableRestoreMode = fe->isPortableRestoreModeEnabled(); vmInfo._isSnapshotModeEnabled = fe->isSnapshotModeEnabled(); #if defined(J9VM_OPT_CRIU_SUPPORT) - vmInfo._isNonPortableRestoreMode = javaVM->internalVMFunctions->isNonPortableRestoreMode(vmThread); + vmInfo._isNonPortableRestoreMode = !javaVM->internalVMFunctions->isJVMInPortableRestoreMode(vmThread); #else vmInfo._isNonPortableRestoreMode = false; #endif /* defined(J9VM_OPT_CRIU_SUPPORT) */ diff --git a/runtime/compiler/control/rossa.cpp b/runtime/compiler/control/rossa.cpp index da5e8388f10..835fbde1065 100644 --- a/runtime/compiler/control/rossa.cpp +++ b/runtime/compiler/control/rossa.cpp @@ -1999,7 +1999,7 @@ aboutToBootstrap(J9JavaVM * javaVM, J9JITConfig * jitConfig) * is because, the restore run may not be on the same machine as the one that created * the snapshot; thus the JIT code must be portable. */ - if (javaVM->internalVMFunctions->isCheckpointAllowed(curThread)) + if (javaVM->internalVMFunctions->isJVMInPortableRestoreMode(curThread)) { TR::Compiler->target.cpu = TR::CPU::detectRelocatable(TR::Compiler->omrPortLib); if (!J9_ARE_ANY_BITS_SET(javaVM->extendedRuntimeFlags2, J9_EXTENDED_RUNTIME2_ENABLE_PORTABLE_SHARED_CACHE)) diff --git a/runtime/compiler/env/VMJ9.cpp b/runtime/compiler/env/VMJ9.cpp index 8e5a5da305f..357f9dfd280 100644 --- a/runtime/compiler/env/VMJ9.cpp +++ b/runtime/compiler/env/VMJ9.cpp @@ -9375,6 +9375,16 @@ TR_J9VMBase::inSnapshotMode() #endif /* defined(J9VM_OPT_CRIU_SUPPORT) */ } +bool +TR_J9VMBase::isPortableRestoreModeEnabled() + { +#if defined(J9VM_OPT_CRIU_SUPPORT) + return getJ9JITConfig()->javaVM->internalVMFunctions->isJVMInPortableRestoreMode(vmThread()); +#else /* defined(J9VM_OPT_CRIU_SUPPORT) */ + return false; +#endif /* defined(J9VM_OPT_CRIU_SUPPORT) */ + } + bool TR_J9VMBase::isSnapshotModeEnabled() { diff --git a/runtime/compiler/env/VMJ9.h b/runtime/compiler/env/VMJ9.h index 346b85083e0..f2d593b4e73 100644 --- a/runtime/compiler/env/VMJ9.h +++ b/runtime/compiler/env/VMJ9.h @@ -1398,6 +1398,13 @@ class TR_J9VMBase : public TR_FrontEnd */ virtual bool inSnapshotMode(); + /** + * \brief Answers whether the JIT should generate portable restore code. + * + * \return True if portable restore code should be generated, false otherwise. + */ + virtual bool isPortableRestoreModeEnabled(); + /** * \brief Answers whether checkpoint and restore mode is enabled (but not necessarily * whether snapshots can be taken or if any restores have already occurred). diff --git a/runtime/compiler/env/VMJ9Server.cpp b/runtime/compiler/env/VMJ9Server.cpp index e58200304d8..71ff94ce80e 100644 --- a/runtime/compiler/env/VMJ9Server.cpp +++ b/runtime/compiler/env/VMJ9Server.cpp @@ -2481,6 +2481,14 @@ TR_J9ServerVM::isSnapshotModeEnabled() return vmInfo->_isSnapshotModeEnabled; } +bool +TR_J9ServerVM::isPortableRestoreModeEnabled() + { + JITServer::ServerStream *stream = _compInfoPT->getMethodBeingCompiled()->_stream; + auto *vmInfo = _compInfoPT->getClientData()->getOrCacheVMInfo(stream); + return vmInfo->_isPortableRestoreMode; + } + bool TR_J9SharedCacheServerVM::isClassLibraryMethod(TR_OpaqueMethodBlock *method, bool vettedForAOT) { diff --git a/runtime/compiler/env/VMJ9Server.hpp b/runtime/compiler/env/VMJ9Server.hpp index 27e47a1f87f..e7268adbf1b 100644 --- a/runtime/compiler/env/VMJ9Server.hpp +++ b/runtime/compiler/env/VMJ9Server.hpp @@ -247,6 +247,7 @@ class TR_J9ServerVM: public TR_J9VM virtual bool isMethodHandleExpectedType(TR::Compilation *comp, TR::KnownObjectTable::Index mhIndex, TR::KnownObjectTable::Index expectedTypeIndex) override; virtual bool inSnapshotMode() override; virtual bool isSnapshotModeEnabled() override; + virtual bool isPortableRestoreModeEnabled() override; private: bool instanceOfOrCheckCastHelper(J9Class *instanceClass, J9Class* castClass, bool cacheUpdate); diff --git a/runtime/compiler/net/CommunicationStream.hpp b/runtime/compiler/net/CommunicationStream.hpp index 3ba27ce0c39..40cdc322c38 100644 --- a/runtime/compiler/net/CommunicationStream.hpp +++ b/runtime/compiler/net/CommunicationStream.hpp @@ -118,7 +118,7 @@ class CommunicationStream // likely to lose an increment when merging/rebasing/etc. // static const uint8_t MAJOR_NUMBER = 1; - static const uint16_t MINOR_NUMBER = 51; // ID: cGK/z3DilpnVN85FsPsD + static const uint16_t MINOR_NUMBER = 53; // ID: 7dZlozupV5RwUvR62RqE static const uint8_t PATCH_NUMBER = 0; static uint32_t CONFIGURATION_FLAGS; diff --git a/runtime/compiler/runtime/JITClientSession.hpp b/runtime/compiler/runtime/JITClientSession.hpp index eef78c6fe88..5cc4922a44f 100644 --- a/runtime/compiler/runtime/JITClientSession.hpp +++ b/runtime/compiler/runtime/JITClientSession.hpp @@ -326,6 +326,7 @@ class ClientSessionData // Do not protect them with #if defined(J9VM_OPT_CRIU_SUPPORT) because we want JITServer to be // able to handle all clients whether or not they have CRIU support enabled bool _inSnapshotMode; + bool _isPortableRestoreMode; bool _isSnapshotModeEnabled; bool _isNonPortableRestoreMode; }; // struct VMInfo diff --git a/runtime/oti/j9nonbuilder.h b/runtime/oti/j9nonbuilder.h index b0fd3f1b4f5..99a79f46c67 100644 --- a/runtime/oti/j9nonbuilder.h +++ b/runtime/oti/j9nonbuilder.h @@ -4199,6 +4199,7 @@ typedef struct J9DelayedLockingOpertionsRecord { #define J9VM_CRIU_IS_NON_PORTABLE_RESTORE_MODE 0x4 #define J9VM_CRIU_IS_JDWP_ENABLED 0x8 #define J9VM_CRIU_IS_THROW_ON_DELAYED_CHECKPOINT_ENABLED 0x10 +#define J9VM_CRIU_IS_PORTABLE_JVM_RESTORE_MODE 0x20 typedef struct J9CRIUCheckpointState { U_32 flags; @@ -5012,6 +5013,7 @@ typedef struct J9InternalVMFunctions { BOOLEAN (*isCRIUSupportEnabled_VM)(struct J9JavaVM *vm); BOOLEAN (*isCheckpointAllowed)(struct J9VMThread *currentThread); BOOLEAN (*isNonPortableRestoreMode)(struct J9VMThread *currentThread); + BOOLEAN (*isJVMInPortableRestoreMode)(struct J9VMThread *currentThread); BOOLEAN (*runInternalJVMCheckpointHooks)(struct J9VMThread *currentThread, const char **nlsMsgFormat); BOOLEAN (*runInternalJVMRestoreHooks)(struct J9VMThread *currentThread, const char **nlsMsgFormat); BOOLEAN (*runDelayedLockRelatedOperations)(struct J9VMThread *currentThread); diff --git a/runtime/oti/jvminit.h b/runtime/oti/jvminit.h index a654c7831e0..8bac5ab7699 100644 --- a/runtime/oti/jvminit.h +++ b/runtime/oti/jvminit.h @@ -426,6 +426,8 @@ enum INIT_STAGE { #define VMOPT_XXDISABLECRIU "-XX:-EnableCRIUSupport" #define VMOPT_XXENABLECRIUNONPORTABLEMODE "-XX:+CRIURestoreNonPortableMode" #define VMOPT_XXDISABLECRIUNONPORTABLEMODE "-XX:-CRIURestoreNonPortableMode" +#define VMOPT_XXENABLEJVMRESTOREPORTABLEMODE "-XX:+JVMPortableRestoreMode" +#define VMOPT_XXDISABLEJVMRESTOREPORTABLEMODE "-XX:-JVMPortableRestoreMode" #define VMOPT_XSHARECLASSES_DISABLEONRESTORE "-Xshareclasses:disableOnRestore" #define VMOPT_XXENABLETHROWONDELAYECHECKPOINTOPERATION "-XX:+ThrowOnDelayedCheckpointOperation" #define VMOPT_XXDISABLETHROWONDELAYECHECKPOINTOPERATION "-XX:-ThrowOnDelayedCheckpointOperation" diff --git a/runtime/oti/vm_api.h b/runtime/oti/vm_api.h index d227d08b1fc..0588a02aa68 100644 --- a/runtime/oti/vm_api.h +++ b/runtime/oti/vm_api.h @@ -541,6 +541,19 @@ isCheckpointAllowed(J9VMThread *currentThread); BOOLEAN isNonPortableRestoreMode(J9VMThread *currentThread); +/** + * @brief Queries if portable restore mode (specified via + * -XX:+JVMPortableRestoreMode) is enabled. If so, the JVM + * will remain in portable mode after restore. However, this + * will have no impact on the JCL and taking another checkpoint + * will still not be permitted. + * + * @param currentThread vmthread token + * @return TRUE if enabled, FALSE otherwise + */ +BOOLEAN +isJVMInPortableRestoreMode(J9VMThread *currentThread); + /** * @brief JVM hooks to run before performing a JVM checkpoint * diff --git a/runtime/vm/CRIUHelpers.cpp b/runtime/vm/CRIUHelpers.cpp index d9bab03ca37..ddb64b17039 100644 --- a/runtime/vm/CRIUHelpers.cpp +++ b/runtime/vm/CRIUHelpers.cpp @@ -135,6 +135,12 @@ isNonPortableRestoreMode(J9VMThread *currentThread) return J9_ARE_ALL_BITS_SET(currentThread->javaVM->checkpointState.flags, J9VM_CRIU_IS_NON_PORTABLE_RESTORE_MODE); } +BOOLEAN +isJVMInPortableRestoreMode(J9VMThread *currentThread) +{ + return (!isNonPortableRestoreMode(currentThread) || J9_ARE_ALL_BITS_SET(currentThread->javaVM->checkpointState.flags, J9VM_CRIU_IS_PORTABLE_JVM_RESTORE_MODE)) && isCRIUSupportEnabled(currentThread); +} + /** * This adds an internal CRIU hook to trace all heap objects of instanceType and its subclasses if specified. * diff --git a/runtime/vm/intfunc.c b/runtime/vm/intfunc.c index 108301c1451..fe417d4c209 100644 --- a/runtime/vm/intfunc.c +++ b/runtime/vm/intfunc.c @@ -409,6 +409,7 @@ J9InternalVMFunctions J9InternalFunctions = { isCRIUSupportEnabled_VM, isCheckpointAllowed, isNonPortableRestoreMode, + isJVMInPortableRestoreMode, runInternalJVMCheckpointHooks, runInternalJVMRestoreHooks, runDelayedLockRelatedOperations, diff --git a/runtime/vm/jvminit.c b/runtime/vm/jvminit.c index 09fcb536bdb..24b5a119ad0 100644 --- a/runtime/vm/jvminit.c +++ b/runtime/vm/jvminit.c @@ -3898,6 +3898,16 @@ processVMArgsFromFirstToLast(J9JavaVM * vm) } } + { + IDATA enableJVMRestorePortableeMode = FIND_AND_CONSUME_VMARG(EXACT_MATCH, VMOPT_XXENABLEJVMRESTOREPORTABLEMODE, NULL); + IDATA disableJVMRestorePortableMode = FIND_AND_CONSUME_VMARG(EXACT_MATCH, VMOPT_XXDISABLEJVMRESTOREPORTABLEMODE, NULL); + if (enableJVMRestorePortableeMode > disableJVMRestorePortableMode) { + if (J9_ARE_ALL_BITS_SET(vm->checkpointState.flags, J9VM_CRIU_IS_CHECKPOINT_ENABLED)) { + vm->checkpointState.flags |= J9VM_CRIU_IS_PORTABLE_JVM_RESTORE_MODE; + } + } + } + { IDATA enableThrowOnDelayedCheckpointOperation = FIND_AND_CONSUME_VMARG(EXACT_MATCH, VMOPT_XXENABLETHROWONDELAYECHECKPOINTOPERATION, NULL); IDATA disableThrowOnDelayedCheckpointOperation = FIND_AND_CONSUME_VMARG(EXACT_MATCH, VMOPT_XXDISABLETHROWONDELAYECHECKPOINTOPERATION, NULL); diff --git a/test/functional/cmdLineTests/criu/playlist.xml b/test/functional/cmdLineTests/criu/playlist.xml index cdade80263a..573ce6f8ff2 100644 --- a/test/functional/cmdLineTests/criu/playlist.xml +++ b/test/functional/cmdLineTests/criu/playlist.xml @@ -91,6 +91,8 @@ -Xjit -XX:+CRIURestoreNonPortableMode -Xint -XX:+CRIURestoreNonPortableMode + -XX:+JVMPortableRestoreMode + -Xjit:count=0 -XX:+JVMPortableRestoreMode -Xjit:count=0 -XX:+CRIURestoreNonPortableMode -Xgcpolicy:optthruput -Xgcpolicy:optavgpause @@ -191,6 +193,7 @@ -Xjit -Xjit:count=0 -Xjit:vlog=vlog + -XX:+JVMPortableRestoreMode if [ -x $(Q)$(TEST_JDK_BIN)$(D)jitserver$(Q) ]; \ @@ -352,6 +355,7 @@ -Xjit -XX:+CRIURestoreNonPortableMode -Xint -XX:+CRIURestoreNonPortableMode -Xjit:count=0 -XX:+CRIURestoreNonPortableMode + -XX:+JVMPortableRestoreMode $(JAVA_COMMAND) $(CMDLINETESTER_JVM_OPTIONS) -Xdump \ diff --git a/test/functional/cmdLineTests/criu/src/org/openj9/criu/CRIUSimpleTest.java b/test/functional/cmdLineTests/criu/src/org/openj9/criu/CRIUSimpleTest.java index b7a91d98342..59eed76054d 100644 --- a/test/functional/cmdLineTests/criu/src/org/openj9/criu/CRIUSimpleTest.java +++ b/test/functional/cmdLineTests/criu/src/org/openj9/criu/CRIUSimpleTest.java @@ -25,6 +25,11 @@ import java.nio.file.Paths; import java.nio.file.Path; +import javax.swing.SwingUtilities; +import javax.swing.JFrame; +import java.lang.management.*; + + public class CRIUSimpleTest { public static void main(String args[]) { @@ -36,12 +41,24 @@ public static void main(String args[]) { } } + private static void loadNewClasses() { + try { + SwingUtilities.isEventDispatchThread(); + JFrame frame = new JFrame("Test code"); + } catch (Throwable t) { + //ignore + } + + ThreadMXBean mxb = ManagementFactory.getThreadMXBean(); + } + public static void checkpoints(int num_checkpoints) { Path path = Paths.get("cpData"); System.out.println("Total checkpoint(s) " + num_checkpoints + ":\nPre-checkpoint"); for (int cur_checkpint = 1; cur_checkpint <= num_checkpoints; ++cur_checkpint) { CRIUTestUtils.checkPointJVM(path); System.out.println("Post-checkpoint " + cur_checkpint); + loadNewClasses(); } } }