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 495439 b148d2cc2558f6f38c9d9d9723cfbe897a506ce6
parent 495438 de02c78c82b754299fefda4d10c9f137cabeca95
child 495440 922b85723d9f859be0e37f8454e1e76e8e8e9af9
push id9984
push userffxbld-merge
push dateMon, 15 Oct 2018 21:07:35 +0000
treeherdermozilla-beta@183d27ea8570 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1495268
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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.