Bug 1373660 - Block the preallocated process manager while a content process is being launched.
☠☠ backed out by 7efddd72ba59 ☠ ☠
authorGabor Krizsanits <gkrizsanits@mozilla.com>
Thu, 29 Jun 2017 21:44:40 +0200
changeset 366734 aa05515c1c8702719a5d79c52e7202f53406171c
parent 366733 96c0145be026cb7555fb1465ff0706f6a310cf11
child 366735 1351044edbaa5a08208af54447e38a885321566d
push id92034
push usergkrizsanits@mozilla.com
push dateThu, 29 Jun 2017 19:51:48 +0000
treeherdermozilla-inbound@aa05515c1c87 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1373660
milestone56.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 1373660 - Block the preallocated process manager while a content process is being launched. We should not let the ppm to do work before the first paint in a new cp. This patch makes sure that we only let the ppm spawn a new process after the last process reached an idle state AND the main process becomes idle too. r=mrbkap
dom/ipc/ContentParent.cpp
dom/ipc/PreallocatedProcessManager.cpp
dom/ipc/PreallocatedProcessManager.h
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -598,16 +598,18 @@ static const char* sObserverTopics[] = {
 // ContentParent then takes this process back within GetNewOrUsedBrowserProcess.
 /*static*/ already_AddRefed<ContentParent>
 ContentParent::PreallocateProcess()
 {
   RefPtr<ContentParent> process =
     new ContentParent(/* aOpener = */ nullptr,
                       NS_LITERAL_STRING(DEFAULT_REMOTE_TYPE));
 
+  PreallocatedProcessManager::AddBlocker(process);
+
   if (!process->LaunchSubprocess(PROCESS_PRIORITY_PREALLOC)) {
     return nullptr;
   }
 
   process->Init();
   return process.forget();
 }
 
@@ -875,16 +877,19 @@ ContentParent::GetNewOrUsedBrowserProces
       p->mActivateTS = TimeStamp::Now();
       return p.forget();
     }
   }
 
   // Create a new process from scratch.
   RefPtr<ContentParent> p = new ContentParent(aOpener, aRemoteType);
 
+  // Until the new process is ready let's not allow to start up any preallocated processes.
+  PreallocatedProcessManager::AddBlocker(p);
+
   if (!p->LaunchSubprocess(aPriority)) {
     return nullptr;
   }
 
   p->Init();
 
   contentParents.AppendElement(p);
   p->mActivateTS = TimeStamp::Now();
@@ -2722,20 +2727,19 @@ ContentParent::RecvGetShowPasswordSettin
 #endif
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 ContentParent::RecvFirstIdle()
 {
   // When the ContentChild goes idle, it sends us a FirstIdle message
-  // which we use as a good time to prelaunch another process. If we
-  // prelaunch any sooner than this, then we'll be competing with the
-  // child process and slowing it down.
-  PreallocatedProcessManager::AllocateAfterDelay();
+  // which we use as a good time to signal the PreallocatedProcessManager
+  // that it can start allocating processes from now on.
+  PreallocatedProcessManager::RemoveBlocker(this);
   return IPC_OK();
 }
 
 // We want ContentParent to show up in CC logs for debugging purposes, but we
 // don't actually cycle collect it.
 NS_IMPL_CYCLE_COLLECTION_0(ContentParent)
 
 NS_IMPL_CYCLE_COLLECTING_ADDREF(ContentParent)
--- a/dom/ipc/PreallocatedProcessManager.cpp
+++ b/dom/ipc/PreallocatedProcessManager.cpp
@@ -34,65 +34,61 @@ class PreallocatedProcessManagerImpl fin
 {
 public:
   static PreallocatedProcessManagerImpl* Singleton();
 
   NS_DECL_ISUPPORTS
   NS_DECL_NSIOBSERVER
 
   // See comments on PreallocatedProcessManager for these methods.
-  void AllocateAfterDelay();
-  void AllocateOnIdle();
-  void AllocateNow();
+  void AddBlocker(ContentParent* aParent);
+  void RemoveBlocker(ContentParent* aParent);
   already_AddRefed<ContentParent> Take();
   bool Provide(ContentParent* aParent);
 
 private:
   static mozilla::StaticRefPtr<PreallocatedProcessManagerImpl> sSingleton;
 
   PreallocatedProcessManagerImpl();
   ~PreallocatedProcessManagerImpl() {}
   DISALLOW_EVIL_CONSTRUCTORS(PreallocatedProcessManagerImpl);
 
   void Init();
 
+  bool CanAllocate();
+  void AllocateAfterDelay();
+  void AllocateOnIdle();
+  void AllocateNow();
+
   void RereadPrefs();
   void Enable();
   void Disable();
   void CloseProcess();
 
   void ObserveProcessShutdown(nsISupports* aSubject);
 
   bool mEnabled;
   bool mShutdown;
   RefPtr<ContentParent> mPreallocatedProcess;
+  nsTHashtable<nsUint64HashKey> mBlockers;
 };
 
 /* static */ StaticRefPtr<PreallocatedProcessManagerImpl>
 PreallocatedProcessManagerImpl::sSingleton;
 
 /* static */ PreallocatedProcessManagerImpl*
 PreallocatedProcessManagerImpl::Singleton()
 {
   MOZ_ASSERT(NS_IsMainThread());
   if (!sSingleton) {
     sSingleton = new PreallocatedProcessManagerImpl();
     sSingleton->Init();
     ClearOnShutdown(&sSingleton);
   }
 
-  // First time when we init sSingleton, the pref database might not be in a
-  // reliable state (we are too early), so despite dom.ipc.processPrelaunch.enabled
-  // is set to true Preferences::GetBool will return false (it cannot find the pref).
-  // Since Init() above will be called only once, and the pref will not be changed,
-  // the manger will stay disabled. To prevent that let's re-read the pref each time
-  // someone accessing the manager singleton. This is a hack but this is not a hot code
-  // so it should be fine.
-  sSingleton->RereadPrefs();
-
   return sSingleton;
 }
 
 NS_IMPL_ISUPPORTS(PreallocatedProcessManagerImpl, nsIObserver)
 
 PreallocatedProcessManagerImpl::PreallocatedProcessManagerImpl()
   : mEnabled(false)
   , mShutdown(false)
@@ -163,16 +159,21 @@ PreallocatedProcessManagerImpl::RereadPr
 
 already_AddRefed<ContentParent>
 PreallocatedProcessManagerImpl::Take()
 {
   if (!mEnabled || mShutdown) {
     return nullptr;
   }
 
+  if (mPreallocatedProcess) {
+    // The preallocated process is taken. Let's try to start up a new one soon.
+    AllocateOnIdle();
+  }
+
   return mPreallocatedProcess.forget();
 }
 
 bool
 PreallocatedProcessManagerImpl::Provide(ContentParent* aParent)
 {
   if (mEnabled && !mShutdown && !mPreallocatedProcess) {
     mPreallocatedProcess = aParent;
@@ -191,51 +192,80 @@ PreallocatedProcessManagerImpl::Enable()
     return;
   }
 
   mEnabled = true;
   AllocateAfterDelay();
 }
 
 void
+PreallocatedProcessManagerImpl::AddBlocker(ContentParent* aParent)
+{
+  uint64_t childID = aParent->ChildID();
+  MOZ_ASSERT(!mBlockers.Contains(childID));
+  mBlockers.PutEntry(childID);
+}
+
+void
+PreallocatedProcessManagerImpl::RemoveBlocker(ContentParent* aParent)
+{
+  uint64_t childID = aParent->ChildID();
+  MOZ_ASSERT(mBlockers.Contains(childID));
+  mBlockers.RemoveEntry(childID);
+  if (!mPreallocatedProcess && mBlockers.IsEmpty()) {
+    AllocateAfterDelay();
+  }
+}
+
+bool
+PreallocatedProcessManagerImpl::CanAllocate()
+{
+  return mEnabled &&
+         mBlockers.IsEmpty() &&
+         !mPreallocatedProcess &&
+         !mShutdown &&
+         !ContentParent::IsMaxProcessCountReached(NS_LITERAL_STRING(DEFAULT_REMOTE_TYPE));
+}
+
+void
 PreallocatedProcessManagerImpl::AllocateAfterDelay()
 {
-  if (!mEnabled || mPreallocatedProcess || mShutdown) {
+  if (!mEnabled) {
     return;
   }
 
-  // Originally AllocateOnIdle() was post here, but since the gecko parent
-  // message loop in practice never goes idle, that didn't work out well.
-  // Let's just launch the process after the delay.
   NS_DelayedDispatchToCurrentThread(
-    NewRunnableMethod("PreallocatedProcessManagerImpl::AllocateNow",
+    NewRunnableMethod("PreallocatedProcessManagerImpl::AllocateOnIdle",
                       this,
-                      &PreallocatedProcessManagerImpl::AllocateNow),
+                      &PreallocatedProcessManagerImpl::AllocateOnIdle),
     Preferences::GetUint("dom.ipc.processPrelaunch.delayMs",
                          DEFAULT_ALLOCATE_DELAY));
 }
 
 void
 PreallocatedProcessManagerImpl::AllocateOnIdle()
 {
-  if (!mEnabled || mPreallocatedProcess || mShutdown) {
+  if (!mEnabled) {
     return;
   }
 
   NS_IdleDispatchToCurrentThread(
     NewRunnableMethod("PreallocatedProcessManagerImpl::AllocateNow",
                       this,
                       &PreallocatedProcessManagerImpl::AllocateNow));
 }
 
 void
 PreallocatedProcessManagerImpl::AllocateNow()
 {
-  if (!mEnabled || mPreallocatedProcess || mShutdown ||
-      ContentParent::IsMaxProcessCountReached(NS_LITERAL_STRING(DEFAULT_REMOTE_TYPE))) {
+  if (!CanAllocate()) {
+    if (mEnabled && !mPreallocatedProcess && !mBlockers.IsEmpty()) {
+      // If it's too early to allocate a process let's retry later.
+      AllocateAfterDelay();
+    }
     return;
   }
 
   mPreallocatedProcess = ContentParent::PreallocateProcess();
 }
 
 void
 PreallocatedProcessManagerImpl::Disable()
@@ -255,53 +285,45 @@ PreallocatedProcessManagerImpl::ClosePro
     mPreallocatedProcess->ShutDownProcess(ContentParent::SEND_SHUTDOWN_MESSAGE);
     mPreallocatedProcess = nullptr;
   }
 }
 
 void
 PreallocatedProcessManagerImpl::ObserveProcessShutdown(nsISupports* aSubject)
 {
-  if (!mPreallocatedProcess) {
-    return;
-  }
-
   nsCOMPtr<nsIPropertyBag2> props = do_QueryInterface(aSubject);
   NS_ENSURE_TRUE_VOID(props);
 
   uint64_t childID = CONTENT_PROCESS_ID_UNKNOWN;
   props->GetPropertyAsUint64(NS_LITERAL_STRING("childID"), &childID);
   NS_ENSURE_TRUE_VOID(childID != CONTENT_PROCESS_ID_UNKNOWN);
 
-  if (childID == mPreallocatedProcess->ChildID()) {
+  if (mPreallocatedProcess && childID == mPreallocatedProcess->ChildID()) {
     mPreallocatedProcess = nullptr;
   }
+
+  mBlockers.RemoveEntry(childID);
 }
 
 inline PreallocatedProcessManagerImpl* GetPPMImpl()
 {
   return PreallocatedProcessManagerImpl::Singleton();
 }
 
 /* static */ void
-PreallocatedProcessManager::AllocateAfterDelay()
+PreallocatedProcessManager::AddBlocker(ContentParent* aParent)
 {
-  GetPPMImpl()->AllocateAfterDelay();
+  GetPPMImpl()->AddBlocker(aParent);
 }
 
 /* static */ void
-PreallocatedProcessManager::AllocateOnIdle()
+PreallocatedProcessManager::RemoveBlocker(ContentParent* aParent)
 {
-  GetPPMImpl()->AllocateOnIdle();
-}
-
-/* static */ void
-PreallocatedProcessManager::AllocateNow()
-{
-  GetPPMImpl()->AllocateNow();
+  GetPPMImpl()->RemoveBlocker(aParent);
 }
 
 /* static */ already_AddRefed<ContentParent>
 PreallocatedProcessManager::Take()
 {
   return GetPPMImpl()->Take();
 }
 
--- a/dom/ipc/PreallocatedProcessManager.h
+++ b/dom/ipc/PreallocatedProcessManager.h
@@ -28,41 +28,24 @@ class ContentParent;
  * We don't expect this pref to flip between true and false in production, but
  * flipping the pref is important for tests.
  */
 class PreallocatedProcessManager final
 {
   typedef mozilla::dom::ContentParent ContentParent;
 
 public:
-  /**
-   * Create a process after a delay.  We wait for a period of time (specified
-   * by the dom.ipc.processPrelaunch.delayMs pref), then wait for this process
-   * to go idle, then allocate the new process.
-   *
-   * If the dom.ipc.processPrelaunch.enabled pref is false, or if we already
-   * have a preallocated process, this function does nothing.
-   */
-  static void AllocateAfterDelay();
 
   /**
-   * Create a process once this process goes idle.
-   *
-   * If the dom.ipc.processPrelaunch.enabled pref is false, or if we already
-   * have a preallocated process, this function does nothing.
+   * Before first paint we don't want to allocate any processes in the background.
+   * To avoid that, the PreallocatedProcessManager won't start up any processes while
+   * there is a blocker active.
    */
-  static void AllocateOnIdle();
-
-  /**
-   * Create a process right now.
-   *
-   * If the dom.ipc.processPrelaunch.enabled pref is false, or if we already
-   * have a preallocated process, this function does nothing.
-   */
-  static void AllocateNow();
+  static void AddBlocker(ContentParent* aParent);
+  static void RemoveBlocker(ContentParent* aParent);
 
   /**
    * Take the preallocated process, if we have one.  If we don't have one, this
    * returns null.
    *
    * If you call Take() twice in a row, the second call is guaranteed to return
    * null.
    *