Bug 1216175, ensure we GC/CC often enough on workers, r=baku
☠☠ backed out by 0959ed75ce40 ☠ ☠
authorOlli Pettay <Olli.Pettay@helsinki.fi>
Wed, 18 Nov 2015 00:03:01 +0200
changeset 273051 bf4205bd5198673a50fd6f1adff9d6c9288a059e
parent 273050 72fd251e770e667a10fb9578c611c189f0b24355
child 273052 ccba8398b89aefc5842c50089881ec152ef9b418
push id29693
push usercbook@mozilla.com
push dateWed, 18 Nov 2015 13:50:33 +0000
treeherdermozilla-central@1d6155d7e6c9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku
bugs1216175
milestone45.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 1216175, ensure we GC/CC often enough on workers, r=baku
dom/workers/WorkerPrivate.cpp
dom/workers/WorkerPrivate.h
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -1443,16 +1443,21 @@ private:
     // Silence bad assertions, this can be dispatched from either the main
     // thread or the timer thread..
   }
 
   virtual bool
   WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
   {
     aWorkerPrivate->GarbageCollectInternal(aCx, mShrinking, mCollectChildren);
+    if (mShrinking) {
+      // Either we've run the idle GC or explicit GC call from the parent,
+      // we can cancel the current timers.
+      aWorkerPrivate->CancelGCTimers();
+    }
     return true;
   }
 };
 
 class CycleCollectRunnable : public WorkerControlRunnable
 {
   bool mCollectChildren;
 
@@ -4479,17 +4484,19 @@ WorkerPrivate::DoRunLoop(JSContext* aCx)
         // Unroot the globals
         mScope = nullptr;
         mDebuggerScope = nullptr;
 
         return;
       }
     }
 
+    bool hadDebuggerOrNormalRunnables = false;
     if (debuggerRunnablesPending || normalRunnablesPending) {
+      hadDebuggerOrNormalRunnables = true;
       // Start the periodic GC timer if it is not already running.
       SetGCTimerMode(PeriodicTimer);
     }
 
     if (debuggerRunnablesPending) {
       WorkerRunnable* runnable;
 
       {
@@ -4520,17 +4527,18 @@ WorkerPrivate::DoRunLoop(JSContext* aCx)
       normalRunnablesPending = NS_HasPendingEvents(mThread);
       if (normalRunnablesPending && GlobalScope()) {
         // Now *might* be a good time to GC. Let the JS engine make the decision.
         JSAutoCompartment ac(aCx, GlobalScope()->GetGlobalJSObject());
         JS_MaybeGC(aCx);
       }
     }
 
-    if (!debuggerRunnablesPending && !normalRunnablesPending) {
+    if (hadDebuggerOrNormalRunnables &&
+        (!debuggerRunnablesPending && !normalRunnablesPending)) {
       // Both the debugger event queue and the normal event queue has been
       // exhausted, cancel the periodic GC timer and schedule the idle GC timer.
       SetGCTimerMode(IdleTimer);
     }
   }
 
   MOZ_CRASH("Shouldn't get here!");
 }
@@ -4573,109 +4581,103 @@ WorkerPrivate::MaybeDispatchLoadFailedRu
   MOZ_ALWAYS_TRUE(NS_SUCCEEDED(NS_DispatchToMainThread(runnable.forget())));
 }
 
 void
 WorkerPrivate::InitializeGCTimers()
 {
   AssertIsOnWorkerThread();
 
-  // We need a timer for GC. The basic plan is to run a non-shrinking GC
+  // We need timers for GC. The basic plan is to run a non-shrinking GC
   // periodically (PERIODIC_GC_TIMER_DELAY_SEC) while the worker is running.
   // Once the worker goes idle we set a short (IDLE_GC_TIMER_DELAY_SEC) timer to
-  // run a shrinking GC. If the worker receives more messages then the short
-  // timer is canceled and the periodic timer resumes.
-  mGCTimer = do_CreateInstance(NS_TIMER_CONTRACTID);
-  MOZ_ASSERT(mGCTimer);
-
+  // run a shrinking GC.
+  mPeriodicGCTimer = do_CreateInstance(NS_TIMER_CONTRACTID);
   RefPtr<GarbageCollectRunnable> runnable =
     new GarbageCollectRunnable(this, false, false);
-  mPeriodicGCTimerTarget = new TimerThreadEventTarget(this, runnable);
-
+  nsCOMPtr<nsIEventTarget> target = new TimerThreadEventTarget(this, runnable);
+  MOZ_ALWAYS_TRUE(NS_SUCCEEDED(mPeriodicGCTimer->SetTarget(target)));
+
+  mIdleGCTimer = do_CreateInstance(NS_TIMER_CONTRACTID);
   runnable = new GarbageCollectRunnable(this, true, false);
-  mIdleGCTimerTarget = new TimerThreadEventTarget(this, runnable);
+  target = new TimerThreadEventTarget(this, runnable);
+  MOZ_ALWAYS_TRUE(NS_SUCCEEDED(mIdleGCTimer->SetTarget(target)));
 
   mPeriodicGCTimerRunning = false;
   mIdleGCTimerRunning = false;
 }
 
 void
 WorkerPrivate::SetGCTimerMode(GCTimerMode aMode)
 {
   AssertIsOnWorkerThread();
-  MOZ_ASSERT(mGCTimer);
-  MOZ_ASSERT(mPeriodicGCTimerTarget);
-  MOZ_ASSERT(mIdleGCTimerTarget);
-
-  if ((aMode == PeriodicTimer && mPeriodicGCTimerRunning) ||
-      (aMode == IdleTimer && mIdleGCTimerRunning)) {
+  MOZ_ASSERT(mPeriodicGCTimer);
+  MOZ_ASSERT(mIdleGCTimer);
+
+  // If we schedule the idle timer, cancel the periodic timer.
+  // But if the idle timer is running, don't cancel it when the periodic timer
+  // is scheduled since we do want shrinking GC to be called occasionally.
+  if (aMode == PeriodicTimer && mPeriodicGCTimerRunning) {
     return;
   }
 
-  MOZ_ALWAYS_TRUE(NS_SUCCEEDED(mGCTimer->Cancel()));
-
-  mPeriodicGCTimerRunning = false;
-  mIdleGCTimerRunning = false;
-
-  LOG(("Worker %p canceled GC timer because %s\n", this,
-       aMode == PeriodicTimer ?
-       "periodic" :
-       aMode == IdleTimer ? "idle" : "none"));
+  if (aMode == IdleTimer || aMode == NoTimer) {
+    MOZ_ALWAYS_TRUE(NS_SUCCEEDED(mPeriodicGCTimer->Cancel()));
+    mPeriodicGCTimerRunning = false;
+  }
+
+  if (aMode == IdleTimer && mIdleGCTimerRunning) {
+    return;
+  }
 
   if (aMode == NoTimer) {
+    MOZ_ALWAYS_TRUE(NS_SUCCEEDED(mIdleGCTimer->Cancel()));
+    mIdleGCTimerRunning = false;
     return;
   }
 
   MOZ_ASSERT(aMode == PeriodicTimer || aMode == IdleTimer);
 
-  nsIEventTarget* target;
-  uint32_t delay;
-  int16_t type;
-
   if (aMode == PeriodicTimer) {
-    target = mPeriodicGCTimerTarget;
-    delay = PERIODIC_GC_TIMER_DELAY_SEC * 1000;
-    type = nsITimer::TYPE_REPEATING_SLACK;
-  }
-  else {
-    target = mIdleGCTimerTarget;
-    delay = IDLE_GC_TIMER_DELAY_SEC * 1000;
-    type = nsITimer::TYPE_ONE_SHOT;
-  }
-
-  MOZ_ALWAYS_TRUE(NS_SUCCEEDED(mGCTimer->SetTarget(target)));
-  MOZ_ALWAYS_TRUE(NS_SUCCEEDED(
-    mGCTimer->InitWithNamedFuncCallback(DummyCallback, nullptr, delay, type,
-                                        "dom::workers::DummyCallback(2)")));
-
-  if (aMode == PeriodicTimer) {
+    uint32_t delay = PERIODIC_GC_TIMER_DELAY_SEC * 1000;
+    int16_t type = nsITimer::TYPE_REPEATING_SLACK;
+    MOZ_ALWAYS_TRUE(NS_SUCCEEDED(
+      mPeriodicGCTimer->
+        InitWithNamedFuncCallback(DummyCallback, nullptr, delay, type,
+                                  "dom::workers::DummyCallback(2)")));
     LOG(("Worker %p scheduled periodic GC timer\n", this));
     mPeriodicGCTimerRunning = true;
-  }
-  else {
+  } else {
+    uint32_t delay = IDLE_GC_TIMER_DELAY_SEC * 1000;
+    int16_t type = nsITimer::TYPE_ONE_SHOT;
+    MOZ_ALWAYS_TRUE(NS_SUCCEEDED(
+      mIdleGCTimer->
+        InitWithNamedFuncCallback(DummyCallback, nullptr, delay, type,
+                                  "dom::workers::DummyCallback(3)")));
     LOG(("Worker %p scheduled idle GC timer\n", this));
     mIdleGCTimerRunning = true;
   }
 }
 
 void
 WorkerPrivate::ShutdownGCTimers()
 {
   AssertIsOnWorkerThread();
 
-  MOZ_ASSERT(mGCTimer);
-
-  // Always make sure the timer is canceled.
-  MOZ_ALWAYS_TRUE(NS_SUCCEEDED(mGCTimer->Cancel()));
-
-  LOG(("Worker %p killed the GC timer\n", this));
-
-  mGCTimer = nullptr;
-  mPeriodicGCTimerTarget = nullptr;
-  mIdleGCTimerTarget = nullptr;
+  MOZ_ASSERT(mPeriodicGCTimer);
+  MOZ_ASSERT(mIdleGCTimer);
+
+  // Always make sure the timers are canceled.
+  MOZ_ALWAYS_TRUE(NS_SUCCEEDED(mPeriodicGCTimer->Cancel()));
+  MOZ_ALWAYS_TRUE(NS_SUCCEEDED(mIdleGCTimer->Cancel()));
+
+  LOG(("Worker %p killed the GC timers\n", this));
+
+  mPeriodicGCTimer = nullptr;
+  mIdleGCTimer = nullptr;
   mPeriodicGCTimerRunning = false;
   mIdleGCTimerRunning = false;
 }
 
 bool
 WorkerPrivate::InterruptCallback(JSContext* aCx)
 {
   AssertIsOnWorkerThread();
@@ -6083,17 +6085,17 @@ WorkerPrivate::RescheduleTimeoutTimer(JS
   NS_ASSERTION(mTimer, "Should have a timer!");
 
   double delta =
     (mTimeouts[0]->mTargetTime - TimeStamp::Now()).ToMilliseconds();
   uint32_t delay = delta > 0 ? std::min(delta, double(UINT32_MAX)) : 0;
 
   nsresult rv = mTimer->InitWithNamedFuncCallback(
     DummyCallback, nullptr, delay, nsITimer::TYPE_ONE_SHOT,
-    "dom::workers::DummyCallback(3)");
+    "dom::workers::DummyCallback(4)");
   if (NS_FAILED(rv)) {
     JS_ReportError(aCx, "Failed to start timer!");
     return false;
   }
 
   return true;
 }
 
--- a/dom/workers/WorkerPrivate.h
+++ b/dom/workers/WorkerPrivate.h
@@ -922,19 +922,18 @@ class WorkerPrivate : public WorkerPriva
 
   // This is only modified on the worker thread, but in DEBUG builds
   // AssertValidSyncLoop function iterates it on other threads. Therefore
   // modifications are done with mMutex held *only* in DEBUG builds.
   nsTArray<nsAutoPtr<SyncLoopInfo>> mSyncLoopStack;
 
   nsCOMPtr<nsITimer> mTimer;
 
-  nsCOMPtr<nsITimer> mGCTimer;
-  nsCOMPtr<nsIEventTarget> mPeriodicGCTimerTarget;
-  nsCOMPtr<nsIEventTarget> mIdleGCTimerTarget;
+  nsCOMPtr<nsITimer> mPeriodicGCTimer;
+  nsCOMPtr<nsITimer> mIdleGCTimer;
 
   RefPtr<MemoryReporter> mMemoryReporter;
 
   // fired on the main thread if the worker script fails to load
   nsCOMPtr<nsIRunnable> mLoadFailedRunnable;
 
   TimeStamp mKillTime;
   uint32_t mErrorHandlerRecursionCount;
@@ -1289,16 +1288,22 @@ public:
   {
     AssertIsOnWorkerThread();
     return mWorkerScriptExecutedSuccessfully;
   }
 
   void
   MaybeDispatchLoadFailedRunnable();
 
+  void
+  CancelGCTimers()
+  {
+    SetGCTimerMode(NoTimer);
+  }
+
 private:
   WorkerPrivate(JSContext* aCx, WorkerPrivate* aParent,
                 const nsAString& aScriptURL, bool aIsChromeWorker,
                 WorkerType aWorkerType, const nsACString& aSharedWorkerName,
                 WorkerLoadInfo& aLoadInfo);
 
   bool
   MayContinueRunning()