Merge pull request #16852 from dsouzai/postRestoreOptions

Post Restore Options Processing Part 4

doc/compiler/control/OptionsPostRestore.md

@@ -60,7 +60,7 @@ Finally, `postProcessInternalCompilerOptions` performs tasks that need
 to be done after the `-Xjit:`, `-Xaot:`, and external options have all
 been processed.
 
-# Subtleties
+# Important Notes
 
 ### JIT and AOT compiled code
 
@@ -138,3 +138,20 @@ file(s) are closed and the post-restore file(s) are opened. A
 consequence of this is that if the same file name is specified then a
 new file will be opened. This can have the consequence of overwriting
 the previous file if the PID of the restored process is the same.
+
+### Start and Elapsed Time
+
+The Checkpoint phase is conceptually part of building the application;
+therefore it does not make sense to expect a user who specifies an
+option such as `-XsamplingExpirationTime` to take into account the time
+spent executing in the Checkpoint phase. Therefore, on restore, both
+the start and elapsed time are reset.
+
+### `-Xrs`, `-Xtrace`, `-Xjit:disableTraps`, `-Xjit:noResumableTrapHandler`
+
+Currently, unless specified pre-checkpoint, the compiler generates will
+generate code assuming these options are not set. If these options are
+then specified post-restore, the options processing code will
+invalidate all JIT code in the code cache, as well as prevent further
+AOT compilation (since the AOT method header would have already been
+validated).

runtime/compiler/control/CompilationRuntime.hpp

@@ -724,6 +724,12 @@ class CompilationInfo
    void setReadyForCheckpointRestore()      {        _checkpointStatus  = TR_CheckpointStatus::READY_FOR_CHECKPOINT_RESTORE;   }
    bool shouldCheckpointBeInterrupted()     { return _checkpointStatus == TR_CheckpointStatus::INTERRUPT_CHECKPOINT;           }
    void interruptCheckpoint()               {        _checkpointStatus  = TR_CheckpointStatus::INTERRUPT_CHECKPOINT;           }
+
+   void setVMMethodTraceEnabled(bool trace) { _vmMethodTraceEnabled = trace; }
+   bool isVMMethodTraceEnabled()            { return _vmMethodTraceEnabled;  }
+
+   bool resetStartAndElapsedTime()              { return _resetStartAndElapsedTime;  }
+   void setResetStartAndElapsedTime(bool reset) { _resetStartAndElapsedTime = reset; }
 #endif
 
    TR_PersistentMemory *     persistentMemory() { return _persistentMemory; }
@@ -1368,6 +1374,8 @@ class CompilationInfo
 #if defined(J9VM_OPT_CRIU_SUPPORT)
    TR::Monitor *_crMonitor;
    TR_CheckpointStatus _checkpointStatus;
+   bool _vmMethodTraceEnabled;
+   bool _resetStartAndElapsedTime;
 #endif
 
    TR::Monitor *_vlogMonitor;

runtime/compiler/control/CompilationThread.cpp

@@ -1184,6 +1184,12 @@ TR::CompilationInfo::CompilationInfo(J9JITConfig *jitConfig) :
 #if defined(J9VM_OPT_CRIU_SUPPORT)
    _crMonitor = TR::Monitor::create("JIT-CheckpointRestoreMonitor");
    _checkpointStatus = TR_CheckpointStatus::NO_CHECKPOINT_IN_PROGRESS;
+
+   // TR::CompilationInfo is initialized in the JIT_INITIALIZED bootstrap
+   // stage, whereas J9_EXTENDED_RUNTIME_METHOD_TRACE_ENABLED is set in the
+   // TRACE_ENGINE_INITIALIZED stage, which happens first.
+   _vmMethodTraceEnabled = jitConfig->javaVM->extendedRuntimeFlags & J9_EXTENDED_RUNTIME_METHOD_TRACE_ENABLED;
+   _resetStartAndElapsedTime = false;
 #endif
    _iprofilerBufferArrivalMonitor = TR::Monitor::create("JIT-IProfilerBufferArrivalMonitor");
    _classUnloadMonitor = TR::MonitorTable::get()->getClassUnloadMonitor(); // by this time this variable is initialized
@@ -2890,18 +2896,18 @@ void TR::CompilationInfo::prepareForCheckpoint()
       acquireCompMonitor(vmThread);
       }
 
-   /* Check if the checkpoint is interrupted */
+   // Check if the checkpoint is interrupted
    if (shouldCheckpointBeInterrupted())
       return;
 
    TR_ASSERT_FATAL(!isCheckpointInProgress(), "Checkpoint already in progress!\n");
 
-   /* Compile methods for checkpoint */
+   // Compile methods for checkpoint
    if (!TR::Options::getCmdLineOptions()->getOption(TR_DisableCompilationBeforeCheckpoint))
       if (!compileMethodsForCheckpoint(vmThread))
          return;
 
-   /* Suspend compilation threads for checkpoint */
+   // Suspend compilation threads for checkpoint
    if (!suspendCompThreadsForCheckpoint(vmThread))
       return;
 
@@ -2920,17 +2926,21 @@ void TR::CompilationInfo::prepareForRestore()
    if (TR::Options::getCmdLineOptions()->getVerboseOption(TR_VerboseCheckpointRestore))
       TR_VerboseLog::writeLineLocked(TR_Vlog_CHECKPOINT_RESTORE, "Preparing for restore");
 
+   // Inform the Sampler Thread to reset the start and elapsed time it maintains
+   setResetStartAndElapsedTime(true);
+
+   // Process the post-restore options
    J9::OptionsPostRestore::processOptionsPostRestore(vmThread, _jitConfig, this);
 
    {
    OMR::CriticalSection resumeCompThreadsCriticalSection(getCompilationMonitor());
 
    TR_ASSERT_FATAL(readyForCheckpointRestore(), "Not ready for Checkpoint Restore\n");
 
-   /* Reset the checkpoint in progress flag. */
+   // Reset the checkpoint in progress flag.
    resetCheckpointInProgress();
 
-   /* Resume suspended compilation threads. */
+   // Resume suspended compilation threads.
    resumeCompilationThread();
    }
 

runtime/compiler/control/HookedByTheJit.cpp

@@ -6102,6 +6102,30 @@ static int32_t J9THREAD_PROC samplerThreadProc(void * entryarg)
       while (!shutdownSamplerThread && // watch for shutdown signals
              j9thread_sleep_interruptable((IDATA) samplingPeriod, 0) == 0) // Anything non-0 is an error condition so we shouldn't do the sampling //!= J9THREAD_INTERRUPTED)
          {
+#if defined(J9VM_OPT_CRIU_SUPPORT)
+         // Post-restore, reset the start and elapsed time. The Checkpoint
+         // phase is conceptually part of building the application; therefore
+         // it does not make sense to expect a user who specifies an option such
+         // as -XsamplingExpirationTime  to take into account the time spent
+         // executing in the Checkpoint phase.
+         if (compInfo->resetStartAndElapsedTime())
+            {
+            if (TR::Options::isAnyVerboseOptionSet())
+               TR_VerboseLog::writeLineLocked(TR_Vlog_CHECKPOINT_RESTORE, "Start and elapsed time: startTime=%6u, elapsedTime=%6u",
+                                              (uint32_t)persistentInfo->getStartTime(), (uint32_t)persistentInfo->getElapsedTime());
+
+            persistentInfo->setStartTime(j9time_current_time_millis());
+            persistentInfo->setElapsedTime(0);
+
+            if (TR::Options::isAnyVerboseOptionSet())
+               TR_VerboseLog::writeLineLocked(TR_Vlog_CHECKPOINT_RESTORE, "Reset start and elapsed time: startTime=%6u, elapsedTime=%6u",
+                                              (uint32_t)persistentInfo->getStartTime(), (uint32_t)persistentInfo->getElapsedTime());
+
+            // Only reset the time once
+            compInfo->setResetStartAndElapsedTime(false);
+            }
+#endif // #if defined(J9VM_OPT_CRIU_SUPPORT)
+
          J9VMThread * currentThread;
 
          persistentInfo->updateElapsedTime(samplingPeriod);

runtime/compiler/control/J9Options.cpp

@@ -295,6 +295,8 @@ int32_t J9::Options::_jProfilingEnablementSampleThreshold = 10000;
 
 bool J9::Options::_aggressiveLockReservation = false;
 
+bool J9::Options::_xrsSync = false;
+
 /**
  * This string array should be kept in sync with the
  * J9::ExternalOptions enum in J9Options.hpp
@@ -2975,7 +2977,8 @@ bool J9::Options::feLatePostProcess(void * base, TR::OptionSet * optionSet)
    // -Xrs Ignore all signals (J9_SIG_XRS_SYNC, J9_SIG_XRS_ASYNC)
    // -Xrs:sync Ignore synchronous signals (J9_SIG_XRS_SYNC)
    // -Xrs:async Ignore asynchronous signals (J9_SIG_XRS_ASYNC)
-   if (J9_ARE_ALL_BITS_SET(javaVM->sigFlags, J9_SIG_XRS_SYNC))
+   _xrsSync = J9_ARE_ALL_BITS_SET(javaVM->sigFlags, J9_SIG_XRS_SYNC);
+   if (_xrsSync)
       {
       self()->setOption(TR_NoResumableTrapHandler);
       self()->setOption(TR_DisablePackedDecimalIntrinsics);

runtime/compiler/control/J9Options.hpp

@@ -425,6 +425,8 @@ class OMR_EXTENSIBLE Options : public OMR::OptionsConnector
 
    static bool _aggressiveLockReservation;
 
+   static bool _xrsSync;
+
    static char * _externalOptionStrings[ExternalOptions::TR_NumExternalOptions];
 
    static void  printPID();

runtime/compiler/control/OptionsPostRestore.cpp

@@ -27,6 +27,7 @@
 #include "j9.h"
 #include "j9nonbuilder.h"
 #include "jvminit.h"
+#include "j9comp.h"
 #include "env/CompilerEnv.hpp"
 #include "env/TRMemory.hpp"
 #include "compile/Method.hpp"
@@ -72,7 +73,13 @@ J9::OptionsPostRestore::OptionsPostRestore(J9VMThread *vmThread, J9JITConfig *ji
    _argIndexDisableUseJITServer(-1),
    _argIndexJITServerAddress(-1),
    _argIndexJITServerAOTCacheName(-1)
-   {}
+   {
+   TR::Options *options = TR::Options::getCmdLineOptions();
+   _disableTrapsPreCheckpoint
+      = J9::Options::_xrsSync
+        || options->getOption(TR_NoResumableTrapHandler)
+        || options->getOption(TR_DisableTraps);
+   }
 
 void
 J9::OptionsPostRestore::iterateOverExternalOptions()
@@ -450,6 +457,23 @@ J9::OptionsPostRestore::invalidateCompiledMethodsIfNeeded(bool invalidateAll)
       }
    }
 
+void
+J9::OptionsPostRestore::disableAOTCompilation()
+   {
+   // Ideally when disabling AOT, the vmThread->aotVMwithThreadInfo->_sharedCache
+   // would also be freed and set to NULL. However, it isn't obvious whether non
+   // compilation and java threads that could be running at the moment could make
+   // use of it, in which case there could be a race. To avoid this, it's safer
+   // to just leave _sharedCache alone; at worst some SCC API could be invoked,
+   // but not during a compilation as all compilation threads are suspended at
+   // this point, and so it won't impact AOT code.
+
+   TR::Options::getAOTCmdLineOptions()->setOption(TR_NoLoadAOT);
+   TR::Options::getAOTCmdLineOptions()->setOption(TR_NoStoreAOT);
+   TR::Options::setSharedClassCache(false);
+   TR_J9SharedCache::setSharedCacheDisabledReason(TR_J9SharedCache::AOT_DISABLED);
+   }
+
 void
 J9::OptionsPostRestore::openNewVlog(char *vLogFileName)
    {
@@ -558,6 +582,7 @@ J9::OptionsPostRestore::preProcessInternalCompilerOptions()
 void
 J9::OptionsPostRestore::postProcessInternalCompilerOptions()
    {
+   J9JavaVM *vm = _jitConfig->javaVM;
    // TODO: Based on whether the following is enabled, do necessary compensation
    // - disabling recompilation (optLevel=, inhibit recomp, etc)
    // - OMR::Options::_logFile (both global and subsets)
@@ -575,8 +600,33 @@ J9::OptionsPostRestore::postProcessInternalCompilerOptions()
    // Open vlog and rtLog if applicable
    openLogFilesIfNeeded();
 
+   // If -Xrs, -Xtrace, or disabling traps is specified post-restore,
+   // invalidate all method bodies
+   bool invalidateAll = false;
+   bool disableAOT = false;
+   if (!_disableTrapsPreCheckpoint
+       && (J9_ARE_ALL_BITS_SET(vm->sigFlags, J9_SIG_XRS_SYNC)
+           || TR::Options::getCmdLineOptions()->getOption(TR_NoResumableTrapHandler)
+           || TR::Options::getCmdLineOptions()->getOption(TR_DisableTraps)))
+      {
+      J9:Options::_xrsSync = J9_ARE_ALL_BITS_SET(vm->sigFlags, J9_SIG_XRS_SYNC);
+      invalidateAll = true;
+      disableAOT = true;
+      }
+   else if (!_compInfo->isVMMethodTraceEnabled()
+            && (vm->extendedRuntimeFlags & J9_EXTENDED_RUNTIME_METHOD_TRACE_ENABLED))
+      {
+      _compInfo->setVMMethodTraceEnabled(true);
+      invalidateAll = true;
+      disableAOT = true;
+      }
+
    // Invalidate method bodies if needed
-   invalidateCompiledMethodsIfNeeded();
+   invalidateCompiledMethodsIfNeeded(invalidateAll);
+
+   // Disable AOT compilation if needed
+   if (disableAOT)
+      disableAOTCompilation();
 
    // Set option to print code cache if necessary
    if (_argIndexPrintCodeCache > _argIndexDisablePrintCodeCache)
@@ -621,19 +671,9 @@ J9::OptionsPostRestore::processCompilerOptions()
    if (aotEnabled)
       aotEnabled = (_argIndexXaot >= _argIndexXnoaot);
 
-   // Ideally when disabling AOT, the vmThread->aotVMwithThreadInfo->_sharedCache
-   // would also be freed and set to NULL. However, it isn't obvious whether non
-   // compilation and java threads that could be running at the moment could make
-   // use of it, in which case there could be a race. To avoid this, it's safer
-   // to just leave _sharedCache alone; at worst some SCC API could be invoked,
-   // but not during a compilation as all compilation threads are suspended at
-   // this point, and so it won't impact AOT code.
    if (!aotEnabled)
       {
-      TR::Options::getAOTCmdLineOptions()->setOption(TR_NoLoadAOT);
-      TR::Options::getAOTCmdLineOptions()->setOption(TR_NoStoreAOT);
-      TR::Options::setSharedClassCache(false);
-      TR_J9SharedCache::setSharedCacheDisabledReason(TR_J9SharedCache::AOT_DISABLED);
+      disableAOTCompilation();
       }
 
    if (!jitEnabled)

runtime/compiler/control/OptionsPostRestore.hpp

@@ -121,6 +121,10 @@ class OptionsPostRestore
     */
    void invalidateCompiledMethodsIfNeeded(bool invalidateAll = false);
 
+   /**
+    * \brief Helper method to disable further AOT compilation.
+    */
+   void disableAOTCompilation();
 
    /**
     * \brief Helper method to perform tasks prior to processing
@@ -162,6 +166,7 @@ class OptionsPostRestore
    char *_oldRtLogFileName;
 
    bool _asyncCompilationPreCheckpoint;
+   bool _disableTrapsPreCheckpoint;
 
    int32_t _argIndexXjit;
    int32_t _argIndexXjitcolon;