Merge pull request #18541 from dsouzai/coordsampler

Coordinate Sampler Thread across a Checkpoint/Restore

Author: mpirvu

doc/compiler/control/OptionsPostRestore.md

@@ -139,13 +139,14 @@ 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.
+### Start Time
+
+While the checkpoint phase is conceptually part of building the
+application, in order to ensure consistency with parts of the compiler
+that memoize elapsed time, the start time is reset to pretend like the
+JVM started `persistentInfo->getElapsedTime()` milliseconds ago. This
+will impact options such as `-XsamplingExpirationTime`. However, such
+an option may not make sense in the context of checkpoint/restore.
 
 ### `-Xrs`, `-Xtrace`, `-Xjit:disableTraps`, `-Xjit:noResumableTrapHandler`
 

runtime/compiler/control/CompilationRuntime.hpp

@@ -374,6 +374,8 @@ class CompilationInfo
       SAMPLE_THR_FAILED_TO_ATTACH,
       SAMPLE_THR_ATTACHED,
       SAMPLE_THR_INITIALIZED,
+      SAMPLE_THR_SUSPENDED,
+      SAMPLE_THR_RESUMING,
       SAMPLE_THR_STOPPING,
       SAMPLE_THR_DESTROYED,
       SAMPLE_THR_LAST_STATE // must be the last one
@@ -731,9 +733,6 @@ class CompilationInfo
    void setVMExceptionEventsHooked(bool trace) { _vmExceptionEventsHooked = trace; }
    bool isVMExceptionEventsHooked()            { return _vmExceptionEventsHooked;  }
 
-   bool resetStartAndElapsedTime()              { return _resetStartAndElapsedTime;  }
-   void setResetStartAndElapsedTime(bool reset) { _resetStartAndElapsedTime = reset; }
-
 #if defined(J9VM_OPT_JITSERVER)
    bool canPerformRemoteCompilationInCRIUMode()                   { return _canPerformRemoteCompilationInCRIUMode;       }
    void setCanPerformRemoteCompilationInCRIUMode(bool remoteComp) { _canPerformRemoteCompilationInCRIUMode = remoteComp; }
@@ -1395,7 +1394,6 @@ class CompilationInfo
    TR_CheckpointStatus _checkpointStatus;
    bool _vmMethodTraceEnabled;
    bool _vmExceptionEventsHooked;
-   bool _resetStartAndElapsedTime;
 #if defined(J9VM_OPT_JITSERVER)
    bool _canPerformRemoteCompilationInCRIUMode;
    bool _remoteCompilationRequestedAtBootstrap;
@@ -1595,6 +1593,29 @@ class CompilationInfo
     * @return false false if the checkpoint is interrupted, true otherwise.
     */
    bool suspendCompThreadsForCheckpoint(J9VMThread *vmThread);
+
+   /**
+    * @brief Suspend all JIT threads such as
+    *        * Compilation Threads
+    *        * Sampler Thread
+    *
+    * @param vmThread The J9VMThread
+    *
+    * @return false if the checkpoint is interrupted, true otherwise.
+    */
+   bool suspendJITThreadsForCheckpoint(J9VMThread *vmThread);
+
+   /**
+    * @brief Resume all JIT threads suspended by suspendCompilerThreadsForCheckpoint
+    *
+    * @param vmThread The J9VMThread
+    */
+   void resumeJITThreadsForRestore(J9VMThread *vmThread);
+
+   /**
+    * @brief Reset Start Time post retore
+    */
+   void resetStartTime();
 #endif
    }; // CompilationInfo
 }

runtime/compiler/control/CompilationThread.cpp

@@ -1207,8 +1207,6 @@ TR::CompilationInfo::CompilationInfo(J9JITConfig *jitConfig) :
         || J9_EVENT_IS_RESERVED(jitConfig->javaVM->hookInterface, J9HOOK_VM_EXCEPTION_THROW);
    _vmExceptionEventsHooked = exceptionCatchEventHooked || exceptionThrowEventHooked;
 
-   _resetStartAndElapsedTime = false;
-
 #if defined(J9VM_OPT_JITSERVER)
    _canPerformRemoteCompilationInCRIUMode = false;
    _remoteCompilationRequestedAtBootstrap = false;
@@ -2901,6 +2899,79 @@ bool TR::CompilationInfo::suspendCompThreadsForCheckpoint(J9VMThread *vmThread)
    return true;
    }
 
+bool
+TR::CompilationInfo::suspendJITThreadsForCheckpoint(J9VMThread *vmThread)
+   {
+   // Suspend compilation threads for checkpoint
+   if (!suspendCompThreadsForCheckpoint(vmThread))
+      return false;
+
+   // Suspend Sampler Thread
+   if (_jitConfig->samplerMonitor)
+      {
+      j9thread_monitor_enter(_jitConfig->samplerMonitor);
+      j9thread_interrupt(_jitConfig->samplerThread);
+
+      // Determine whether to wait on the CR Monitor.
+      //
+      // Note, this thread releases the sampler monitor and then
+      // acquires the CR monitor inside releaseCompMonitorUntilNotifiedOnCRMonitor.
+      while (!shouldCheckpointBeInterrupted()
+             && getSamplingThreadLifetimeState() != TR::CompilationInfo::SAMPLE_THR_SUSPENDED)
+         {
+         j9thread_monitor_exit(_jitConfig->samplerMonitor);
+         releaseCompMonitorUntilNotifiedOnCRMonitor(vmThread);
+         j9thread_monitor_enter(_jitConfig->samplerMonitor);
+         }
+
+      j9thread_monitor_exit(_jitConfig->samplerMonitor);
+      }
+
+   return !shouldCheckpointBeInterrupted();
+   }
+
+void
+TR::CompilationInfo::resumeJITThreadsForRestore(J9VMThread *vmThread)
+   {
+   // Resume suspended Sampler Thread
+   if (_jitConfig->samplerMonitor)
+      {
+      j9thread_monitor_enter(_jitConfig->samplerMonitor);
+      setSamplingThreadLifetimeState(TR::CompilationInfo::SAMPLE_THR_RESUMING);
+      j9thread_monitor_notify_all(_jitConfig->samplerMonitor);
+      j9thread_monitor_exit(_jitConfig->samplerMonitor);
+      }
+
+   // Resume suspended compilation threads.
+   resumeCompilationThread();
+   }
+
+/* Post-restore, reset the start time. While the Checkpoint phase is
+ * conceptually part of building the application, in order to ensure
+ * consistency with parts of the compiler that memoize elapsd time,
+ * the start time is reset to pretend like the JVM started
+ * persistentInfo->getElapsedTime() milliseconds ago. This will impact
+ * options such as -XsamplingExpirationTime. However, such an option
+ * may not make sense in the context of checkpoint/restore.
+ */
+void
+TR::CompilationInfo::resetStartTime()
+   {
+   PORT_ACCESS_FROM_JAVAVM(jitConfig->javaVM);
+   TR::PersistentInfo *persistentInfo = getPersistentInfo();
+
+   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());
+
+   uint64_t crtTime = j9time_current_time_millis() - persistentInfo->getElapsedTime();
+   persistentInfo->setStartTime(crtTime);
+
+   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());
+   }
+
 void TR::CompilationInfo::prepareForCheckpoint()
    {
    J9JavaVM   *vm       = _jitConfig->javaVM;
@@ -2933,8 +3004,8 @@ void TR::CompilationInfo::prepareForCheckpoint()
       if (!compileMethodsForCheckpoint(vmThread))
          return;
 
-   // Suspend compilation threads for checkpoint
-   if (!suspendCompThreadsForCheckpoint(vmThread))
+   // Suspend JIT threads for checkpoint
+   if (!suspendJITThreadsForCheckpoint(vmThread))
       return;
 
 #if defined(J9VM_OPT_JITSERVER)
@@ -2967,9 +3038,6 @@ 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);
 
@@ -2981,8 +3049,11 @@ void TR::CompilationInfo::prepareForRestore()
    // Reset the checkpoint in progress flag.
    resetCheckpointInProgress();
 
-   // Resume suspended compilation threads.
-   resumeCompilationThread();
+   // Reset the start time.
+   resetStartTime();
+
+   // Resume JIT threads.
+   resumeJITThreadsForRestore(vmThread);
    }
 
    // Check if there is no swap memory post restore

runtime/compiler/control/HookedByTheJit.cpp

@@ -5928,6 +5928,84 @@ class CompilationDensity
       }
    }; // class CompilationDensity
 
+#if defined(J9VM_OPT_CRIU_SUPPORT)
+static void suspendSamplerThreadForCheckpoint(J9VMThread *samplerThread, J9JITConfig *jitConfig, TR::CompilationInfo *compInfo)
+   {
+   compInfo->acquireCompMonitor(samplerThread);
+   if (compInfo->shouldSuspendThreadsForCheckpoint())
+      {
+      PORT_ACCESS_FROM_JITCONFIG(jitConfig);
+
+      // Must acquire this with the comp monitor in hand to ensure
+      // consistency with the checkpointing thread.
+      j9thread_monitor_enter(jitConfig->samplerMonitor);
+
+      // Because this thread has the comp monitor in hand, and because
+      // shouldSuspendThreadsForCheckpoint() returned true, it is not possible
+      // for the Sampler Thread to have any other state other than SAMPLE_THR_INITIALIZED
+      TR_ASSERT_FATAL(compInfo->getSamplingThreadLifetimeState() == TR::CompilationInfo::SAMPLE_THR_INITIALIZED,
+                      "Sampler Thread Lifetime State %d is not SAMPLE_THR_INITIALIZED!", compInfo->getSamplingThreadLifetimeState());
+
+      // Update the sampler thread state.
+      compInfo->setSamplingThreadLifetimeState(TR::CompilationInfo::SAMPLE_THR_SUSPENDED);
+
+      // Notify the checkpointing thread about the state change.
+      //
+      // Note, unlike the checkpointing thread, this thread does NOT
+      // release the sampler monitor before acquring the CR monitor.
+      // This ensures that the Sampling Thread Lifetime State does not
+      // change because of something like Shutdown. However, this
+      // can only cause a deadlock if the checkpointing thread
+      // decides to re-acquire the sampler monitor with the CR monitor
+      // in hand.
+      compInfo->acquireCRMonitor();
+      compInfo->getCRMonitor()->notifyAll();
+      compInfo->releaseCRMonitor();
+
+      if (TR::Options::isAnyVerboseOptionSet())
+         TR_VerboseLog::writeLineLocked(TR_Vlog_CHECKPOINT_RESTORE, "Suspending Sampler Thread for Checkpoint");
+
+      // Release the comp monitor before suspending.
+      compInfo->releaseCompMonitor(samplerThread);
+
+      // Wait until restore, at which point the lifetime state
+      // will be TR::CompilationInfo::SAMPLE_THR_RESUMING
+      while (compInfo->getSamplingThreadLifetimeState() == TR::CompilationInfo::SAMPLE_THR_SUSPENDED)
+         {
+         j9thread_monitor_wait(jitConfig->samplerMonitor);
+         }
+
+      if (TR::Options::isAnyVerboseOptionSet())
+         TR_VerboseLog::writeLineLocked(TR_Vlog_CHECKPOINT_RESTORE, "Resuming Sampler Thread from Checkpoint");
+
+      // Release the sampler monitor before reacquring both the
+      // comp monitor and sampler monitor. This is necessary to
+      // ensure consistency with the checkpointing thread.
+      j9thread_monitor_exit(jitConfig->samplerMonitor);
+      compInfo->acquireCompMonitor(samplerThread);
+      j9thread_monitor_enter(jitConfig->samplerMonitor);
+
+      // Ensure the sampler thread was resumed because of a restore
+      // rather than something else (such as shutdown)
+      if (compInfo->getSamplingThreadLifetimeState() == TR::CompilationInfo::SAMPLE_THR_RESUMING)
+         {
+         if (TR::Options::isAnyVerboseOptionSet())
+            TR_VerboseLog::writeLineLocked(TR_Vlog_CHECKPOINT_RESTORE, "Resetting Sampling Thread Lifetime State");
+         compInfo->setSamplingThreadLifetimeState(TR::CompilationInfo::SAMPLE_THR_INITIALIZED);
+         }
+      else
+         {
+         if (TR::Options::isAnyVerboseOptionSet())
+            TR_VerboseLog::writeLineLocked(TR_Vlog_CHECKPOINT_RESTORE, "Sampling Thread Lifetime State is %p which is not %p!", compInfo->getSamplingThreadLifetimeState(), TR::CompilationInfo::SAMPLE_THR_RESUMING);
+         }
+
+      // Release the reacquired sampler thread monitor.
+      j9thread_monitor_exit(jitConfig->samplerMonitor);
+      }
+   compInfo->releaseCompMonitor(samplerThread);
+   }
+#endif
+
 static int32_t J9THREAD_PROC samplerThreadProc(void * entryarg)
    {
    J9JITConfig * jitConfig = (J9JITConfig *) entryarg;
@@ -6124,35 +6202,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);
          crtTime += samplingPeriod;
 
+#if defined(J9VM_OPT_CRIU_SUPPORT)
+         if (vm->internalVMFunctions->isCheckpointAllowed(samplerThread)
+            /* It's ok to not acquire the comp monitor here. Even if at this
+             * point a checkpoint isn't in progress but later it is, the
+             * checkpoint will not complete until the sampler thread suspends
+             * itself. Therefore, on some iteration of the enclosing while
+             * loop, this condition will be true and the sampler thread will
+             * go through the process of suspending itself. Conversely, if
+             * this condition is true, but after acquring the comp monitor
+             * (in the call to suspendSamplerThreadForCheckpoint) this condition
+             * isn't true (e.g., due to shutdown), then the sampler thread will
+             * not suspend itself.
+             */
+             && compInfo->shouldSuspendThreadsForCheckpoint())
+            {
+            suspendSamplerThreadForCheckpoint(samplerThread,jitConfig, compInfo);
+            }
+#endif // #if defined(J9VM_OPT_CRIU_SUPPORT)
+
          // periodic chores
          // FIXME: make a constant/macro for the period, and make it 100
          if (crtTime - oldSyncTime >= 100) // every 100 ms