Bug 1495268 - Watch for checkpoint activity whenever a thread blocks on the helper thread state lock, r=jandem.
authorBrian Hackett <bhackett1024@gmail.com>
Wed, 03 Oct 2018 15:05:10 -1000
changeset 487998 b148d2cc2558f6f38c9d9d9723cfbe897a506ce6
parent 487997 de02c78c82b754299fefda4d10c9f137cabeca95
child 487999 922b85723d9f859be0e37f8454e1e76e8e8e9af9
push id246
push userfmarier@mozilla.com
push dateSat, 13 Oct 2018 00:15:40 +0000
reviewersjandem
bugs1495268
milestone64.0a1
Bug 1495268 - Watch for checkpoint activity whenever a thread blocks on the helper thread state lock, r=jandem.
js/src/vm/HelperThreads.cpp
--- a/js/src/vm/HelperThreads.cpp
+++ b/js/src/vm/HelperThreads.cpp
@@ -116,16 +116,61 @@ js::SetFakeCPUCount(size_t count)
 void
 JS::SetProfilingThreadCallbacks(JS::RegisterThreadCallback registerThread,
                                 JS::UnregisterThreadCallback unregisterThread)
 {
     HelperThreadState().registerThread = registerThread;
     HelperThreadState().unregisterThread = unregisterThread;
 }
 
+/* static */ void
+HelperThread::WakeupAll()
+{
+    AutoLockHelperThreadState lock;
+    HelperThreadState().notifyAll(GlobalHelperThreadState::PRODUCER, lock);
+}
+
+// Uses of the helper thread state lock are not included in Web Replay
+// recordings and threads which block on the lock indefinitely need to
+// coordinate with the record/replay system to make sure they enter the right
+// idle state when the main thread wants to save or restore a checkpoint.
+// This method must be used before any thread might wait on a condition
+// variable associated with the lock.
+//
+// This can release the lock, however, so the normal way of using this is:
+//
+// while (true) {
+//   MaybeWaitForRecordReplayCheckpointSave(...);
+//   if (doneWaiting) {
+//     break;
+//   }
+//   HelperThreadState().wait(...);
+// }
+static void
+MaybeWaitForRecordReplayCheckpointSave(AutoLockHelperThreadState& locked)
+{
+    if (mozilla::recordreplay::IsRecordingOrReplaying()) {
+        // Unlock the helper thread state lock before potentially blocking
+        // while the main thread waits for all threads to become idle.
+        // Otherwise we would need to see if we need to block at every point
+        // where any thread acquires the helper thread state lock.
+        {
+            AutoUnlockHelperThreadState unlock(locked);
+            mozilla::recordreplay::MaybeWaitForCheckpointSave();
+        }
+
+        // For the std::function destructor invoked below.
+        JS::AutoSuppressGCAnalysis nogc;
+
+        // Now that we are holding the helper thread state lock again,
+        // notify the record/replay system that we might block soon.
+        mozilla::recordreplay::NotifyUnrecordedWait(HelperThread::WakeupAll);
+    }
+}
+
 bool
 js::StartOffThreadWasmCompile(wasm::CompileTask* task, wasm::CompileMode mode)
 {
     AutoLockHelperThreadState lock;
 
     if (!HelperThreadState().wasmWorklist(lock, mode).pushBack(task)) {
         return false;
     }
@@ -181,17 +226,21 @@ CancelOffThreadWasmTier2GeneratorLocked(
             // Set a flag that causes compilation to shortcut itself.
             helper.wasmTier2GeneratorTask()->cancel();
 
             // Wait for the generator task to finish.  This avoids a shutdown race where
             // the shutdown code is trying to shut down helper threads and the ongoing
             // tier2 compilation is trying to finish, which requires it to have access
             // to helper threads.
             uint32_t oldFinishedCount = HelperThreadState().wasmTier2GeneratorsFinished(lock);
-            while (HelperThreadState().wasmTier2GeneratorsFinished(lock) == oldFinishedCount) {
+            while (true) {
+                MaybeWaitForRecordReplayCheckpointSave(lock);
+                if (HelperThreadState().wasmTier2GeneratorsFinished(lock) != oldFinishedCount) {
+                    break;
+                }
                 HelperThreadState().wait(lock, GlobalHelperThreadState::CONSUMER);
             }
 
             // At most one of these tasks.
             break;
         }
     }
 }
@@ -326,16 +375,17 @@ CancelOffThreadIonCompileLocked(const Co
             FinishOffThreadIonCompile(builder, lock);
             HelperThreadState().remove(worklist, &i);
         }
     }
 
     /* Wait for in progress entries to finish up. */
     bool cancelled;
     do {
+        MaybeWaitForRecordReplayCheckpointSave(lock);
         cancelled = false;
         for (auto& helper : *HelperThreadState().threads) {
             if (helper.ionBuilder() &&
                 IonBuilderMatches(selector, helper.ionBuilder()))
             {
                 helper.ionBuilder()->cancel();
                 cancelled = true;
             }
@@ -660,16 +710,17 @@ js::CancelOffThreadParses(JSRuntime* rt)
         MOZ_ASSERT(!waitingOnGC[i]->runtimeMatches(rt));
     }
 #endif
 
     // Instead of forcibly canceling pending parse tasks, just wait for all scheduled
     // and in progress ones to complete. Otherwise the final GC may not collect
     // everything due to zones being used off thread.
     while (true) {
+        MaybeWaitForRecordReplayCheckpointSave(lock);
         bool pending = false;
         GlobalHelperThreadState::ParseTaskVector& worklist = HelperThreadState().parseWorklist(lock);
         for (size_t i = 0; i < worklist.length(); i++) {
             ParseTask* task = worklist[i];
             if (task->runtimeMatches(rt)) {
                 pending = true;
             }
         }
@@ -1173,17 +1224,21 @@ GlobalHelperThreadState::waitForAllThrea
 }
 
 void
 GlobalHelperThreadState::waitForAllThreadsLocked(AutoLockHelperThreadState& lock)
 {
     CancelOffThreadIonCompileLocked(CompilationSelector(AllCompilations()), false, lock);
     CancelOffThreadWasmTier2GeneratorLocked(lock);
 
-    while (hasActiveThreads(lock)) {
+    while (true) {
+        MaybeWaitForRecordReplayCheckpointSave(lock);
+        if (!hasActiveThreads(lock)) {
+            break;
+        }
         wait(lock, CONSUMER);
     }
 }
 
 // A task can be a "master" task, ie, it will block waiting for other worker
 // threads that perform work on its behalf.  If so it must not take the last
 // available thread; there must always be at least one worker thread able to do
 // the actual work.  (Or the system may deadlock.)
@@ -1641,17 +1696,21 @@ js::GCParallelTask::start()
 
 void
 js::GCParallelTask::joinWithLockHeld(AutoLockHelperThreadState& lock)
 {
     if (isNotStarted(lock)) {
         return;
     }
 
-    while (!isFinished(lock)) {
+    while (true) {
+        MaybeWaitForRecordReplayCheckpointSave(lock);
+        if (isFinished(lock)) {
+            break;
+        }
         HelperThreadState().wait(lock, GlobalHelperThreadState::CONSUMER);
     }
 
     setNotStarted(lock);
     cancel_ = false;
 }
 
 void
@@ -2345,16 +2404,18 @@ js::CancelOffThreadCompressions(JSRuntim
 
     // Cancel all pending compression tasks.
     ClearCompressionTaskList(HelperThreadState().compressionPendingList(lock), runtime);
     ClearCompressionTaskList(HelperThreadState().compressionWorklist(lock), runtime);
 
     // Cancel all in-process compression tasks and wait for them to join so we
     // clean up the finished tasks.
     while (true) {
+        MaybeWaitForRecordReplayCheckpointSave(lock);
+
         bool inProgress = false;
         for (auto& thread : *HelperThreadState().threads) {
             SourceCompressionTask* task = thread.compressionTask();
             if (task && task->runtimeMatches(runtime)) {
                 inProgress = true;
             }
         }
 
@@ -2449,23 +2510,16 @@ GlobalHelperThreadState::trace(JSTracer*
     for (auto parseTask : parseFinishedList_) {
         parseTask->trace(trc);
     }
     for (auto parseTask : parseWaitingOnGC_) {
         parseTask->trace(trc);
     }
 }
 
-/* static */ void
-HelperThread::WakeupAll()
-{
-    AutoLockHelperThreadState lock;
-    HelperThreadState().notifyAll(GlobalHelperThreadState::PRODUCER, lock);
-}
-
 void
 JSContext::setHelperThread(HelperThread* thread)
 {
     if (helperThread_) {
         nurserySuppressions_--;
     }
 
     helperThread_ = thread;
@@ -2562,32 +2616,17 @@ HelperThread::threadLoop()
         }
     }
     cx.setHelperThread(this);
     JS_SetNativeStackQuota(&cx, HELPER_STACK_QUOTA);
 
     while (!terminate) {
         MOZ_ASSERT(idle());
 
-        if (mozilla::recordreplay::IsRecordingOrReplaying()) {
-            // Unlock the helper thread state lock before potentially
-            // blocking while the main thread waits for all threads to
-            // become idle. Otherwise we would need to see if we need to
-            // block at every point where a helper thread acquires the
-            // helper thread state lock.
-            {
-                AutoUnlockHelperThreadState unlock(lock);
-                mozilla::recordreplay::MaybeWaitForCheckpointSave();
-            }
-            // Now that we are holding the helper thread state lock again,
-            // notify the record/replay system that we might block soon.
-            // The helper thread state lock may not be released until the
-            // block occurs.
-            mozilla::recordreplay::NotifyUnrecordedWait(WakeupAll);
-        }
+        MaybeWaitForRecordReplayCheckpointSave(lock);
 
         maybeFreeUnusedMemory(&cx);
 
         // The selectors may depend on the HelperThreadState not changing
         // between task selection and task execution, in particular, on new
         // tasks not being added (because of the lifo structure of the work
         // lists). Unlocking the HelperThreadState between task selection and
         // execution is not well-defined.