Bug 774388 - Patch 5: Wrap the global raw compositor thread pointer, and global raw refcount integer, into a proper refcounted singleton class - r=nical,mattwoodrow
☠☠ backed out by 7d8281d3913a ☠ ☠
authorBenoit Jacob <bjacob@mozilla.com>
Fri, 06 Jun 2014 09:51:26 -0400
changeset 206429 6f2e001c5f3923c740bf2e3d8ca8750739c88758
parent 206428 7a0d8feb1575bb4883211ef09a8f029a205523e1
child 206430 0f81ceab808a25929596cad9b715d0b6e2ccf6a7
push id3741
push userasasaki@mozilla.com
push dateMon, 21 Jul 2014 20:25:18 +0000
treeherdermozilla-beta@4d6f46f5af68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnical, mattwoodrow
bugs774388
milestone32.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 774388 - Patch 5: Wrap the global raw compositor thread pointer, and global raw refcount integer, into a proper refcounted singleton class - r=nical,mattwoodrow
gfx/layers/ipc/CompositorParent.cpp
gfx/layers/ipc/CompositorParent.h
gfx/thebes/gfxPlatform.cpp
--- a/gfx/layers/ipc/CompositorParent.cpp
+++ b/gfx/layers/ipc/CompositorParent.cpp
@@ -48,16 +48,17 @@
 #include "mozilla/layers/CompositorD3D11.h"
 #include "mozilla/layers/CompositorD3D9.h"
 #endif
 #include "GeckoProfiler.h"
 #include "mozilla/ipc/ProtocolTypes.h"
 #include "mozilla/unused.h"
 #include "mozilla/Hal.h"
 #include "mozilla/HalTypes.h"
+#include "mozilla/StaticPtr.h"
 
 namespace mozilla {
 namespace layers {
 
 using namespace base;
 using namespace mozilla::ipc;
 using namespace mozilla::gfx;
 using namespace std;
@@ -68,155 +69,197 @@ CompositorParent::LayerTreeState::LayerT
   , mCrossProcessParent(nullptr)
   , mLayerTree(nullptr)
 {
 }
 
 typedef map<uint64_t, CompositorParent::LayerTreeState> LayerTreeMap;
 static LayerTreeMap sIndirectLayerTrees;
 
-// FIXME/bug 774386: we're assuming that there's only one
-// CompositorParent, but that's not always true.  This assumption only
-// affects CrossProcessCompositorParent below.
-static Thread* sCompositorThread = nullptr;
-// manual reference count of the compositor thread.
-static int sCompositorThreadRefCount = 0;
+/**
+  * A global map referencing each compositor by ID.
+  *
+  * This map is used by the ImageBridge protocol to trigger
+  * compositions without having to keep references to the
+  * compositor
+  */
+typedef map<uint64_t,CompositorParent*> CompositorMap;
+static CompositorMap* sCompositorMap;
+
+static void CreateCompositorMap()
+{
+  MOZ_ASSERT(!sCompositorMap);
+  sCompositorMap = new CompositorMap;
+}
+
+static void DestroyCompositorMap()
+{
+  MOZ_ASSERT(sCompositorMap);
+  MOZ_ASSERT(sCompositorMap->empty());
+  delete sCompositorMap;
+  sCompositorMap = nullptr;
+}
+
+class CompositorThreadHolder
+{
+  NS_INLINE_DECL_THREADSAFE_REFCOUNTING(CompositorThreadHolder)
+
+public:
+  CompositorThreadHolder()
+    : mCompositorThread(CreateCompositorThread())
+  {
+    MOZ_COUNT_CTOR(CompositorThreadHolder);
+  }
+
+  Thread* GetCompositorThread() const {
+    return mCompositorThread;
+  }
+
+private:
+  ~CompositorThreadHolder() {
+    MOZ_COUNT_DTOR(CompositorThreadHolder);
+    DeleteCompositorThread(mCompositorThread);
+  }
+
+  Thread* const mCompositorThread;
+
+  static Thread* CreateCompositorThread();
+  static void DeleteCompositorThread(Thread* aCompositorThread);
+};
+
+static StaticRefPtr<CompositorThreadHolder> gCompositorThreadHolder;
+
+static Thread* CompositorThread() {
+  MOZ_ASSERT(gCompositorThreadHolder);
+  return gCompositorThreadHolder->GetCompositorThread();
+}
+
+
 static MessageLoop* sMainLoop = nullptr;
 
 // See ImageBridgeChild.cpp
 void ReleaseImageBridgeParentSingleton();
 
 static void DeferredDeleteCompositorParent(CompositorParent* aNowReadyToDie)
 {
   aNowReadyToDie->Release();
 }
 
-static void DeleteCompositorThread()
+/* static */ void CompositorThreadHolder::DeleteCompositorThread(Thread* aCompositorThread)
 {
   if (NS_IsMainThread()){
+    DestroyCompositorMap();
     ReleaseImageBridgeParentSingleton();
-    delete sCompositorThread;
-    sCompositorThread = nullptr;
+    delete aCompositorThread;
   } else {
-    sMainLoop->PostTask(FROM_HERE, NewRunnableFunction(&DeleteCompositorThread));
-  }
-}
-
-static void ReleaseCompositorThread()
-{
-  if(--sCompositorThreadRefCount == 0) {
-    DeleteCompositorThread();
+    sMainLoop->PostTask(FROM_HERE,
+                        NewRunnableFunction(&CompositorThreadHolder::DeleteCompositorThread,
+                                            CompositorThread()));
   }
 }
 
 static void SetThreadPriority()
 {
   hal::SetCurrentThreadPriority(hal::THREAD_PRIORITY_COMPOSITOR);
 }
 
-void CompositorParent::StartUp()
+void CompositorParent::StartUpCompositorThread()
 {
+  MOZ_ASSERT(NS_IsMainThread(), "Should be on the main Thread!");
+  MOZ_ASSERT(!gCompositorThreadHolder, "The compositor thread has already been started!");
   CreateCompositorMap();
-  CreateThread();
+  gCompositorThreadHolder = new CompositorThreadHolder();
   sMainLoop = MessageLoop::current();
 }
 
-void CompositorParent::ShutDown()
+void CompositorParent::AllowShutdownOfCompositorThreadWhenUnreferenced()
 {
-  DestroyThread();
-  DestroyCompositorMap();
+  MOZ_ASSERT(NS_IsMainThread(), "Should be on the main Thread!");
+  MOZ_ASSERT(gCompositorThreadHolder, "The compositor thread has already been shut down!");
+  gCompositorThreadHolder = nullptr;
 }
 
-bool CompositorParent::CreateThread()
+/* static */ Thread* CompositorThreadHolder::CreateCompositorThread()
 {
-  NS_ASSERTION(NS_IsMainThread(), "Should be on the main Thread!");
-  MOZ_ASSERT(!sCompositorThread);
-  sCompositorThreadRefCount = 1;
-  sCompositorThread = new Thread("Compositor");
+  MOZ_ASSERT(NS_IsMainThread(), "Should be on the main Thread!");
+  MOZ_ASSERT(!gCompositorThreadHolder, "The compositor thread has already been started!");
+
+  Thread* compositorThread = new Thread("Compositor");
 
   Thread::Options options;
   /* Timeout values are powers-of-two to enable us get better data.
      128ms is chosen for transient hangs because 8Hz should be the minimally
      acceptable goal for Compositor responsiveness (normal goal is 60Hz). */
   options.transient_hang_timeout = 128; // milliseconds
   /* 8192ms is chosen for permanent hangs because it's several seconds longer
      than the default hang timeout on major platforms (about 5 seconds). */
   options.permanent_hang_timeout = 8192; // milliseconds
 
-  if (!sCompositorThread->StartWithOptions(options)) {
-    delete sCompositorThread;
-    sCompositorThread = nullptr;
-    return false;
+  if (!compositorThread->StartWithOptions(options)) {
+    delete compositorThread;
+    compositorThread = nullptr;
   }
 
-  return true;
-}
-
-void CompositorParent::DestroyThread()
-{
-  NS_ASSERTION(NS_IsMainThread(), "Should be on the main Thread!");
-  ReleaseCompositorThread();
+  return compositorThread;
 }
 
 MessageLoop* CompositorParent::CompositorLoop()
 {
-  return sCompositorThread ? sCompositorThread->message_loop() : nullptr;
+  return CompositorThread() ? CompositorThread()->message_loop() : nullptr;
 }
 
 CompositorParent::CompositorParent(nsIWidget* aWidget,
                                    bool aUseExternalSurfaceSize,
                                    int aSurfaceWidth, int aSurfaceHeight)
   : mWidget(aWidget)
   , mCurrentCompositeTask(nullptr)
   , mIsTesting(false)
   , mPendingTransaction(0)
   , mPaused(false)
   , mUseExternalSurfaceSize(aUseExternalSurfaceSize)
   , mEGLSurfaceSize(aSurfaceWidth, aSurfaceHeight)
   , mPauseCompositionMonitor("PauseCompositionMonitor")
   , mResumeCompositionMonitor("ResumeCompositionMonitor")
   , mOverrideComposeReadiness(false)
   , mForceCompositionTask(nullptr)
+  , mCompositorThreadHolder(gCompositorThreadHolder)
 {
-  MOZ_ASSERT(sCompositorThread != nullptr,
-             "The compositor thread must be Initialized before instanciating a CmpositorParent.");
+  MOZ_ASSERT(CompositorThread(),
+             "The compositor thread must be Initialized before instanciating a CompositorParent.");
   MOZ_COUNT_CTOR(CompositorParent);
   mCompositorID = 0;
   // FIXME: This holds on the the fact that right now the only thing that
   // can destroy this instance is initialized on the compositor thread after
   // this task has been processed.
   CompositorLoop()->PostTask(FROM_HERE, NewRunnableFunction(&AddCompositor,
                                                           this, &mCompositorID));
 
   CompositorLoop()->PostTask(FROM_HERE, NewRunnableFunction(SetThreadPriority));
 
   mRootLayerTreeID = AllocateLayerTreeId();
   sIndirectLayerTrees[mRootLayerTreeID].mParent = this;
 
   mApzcTreeManager = new APZCTreeManager();
-  ++sCompositorThreadRefCount;
 }
 
 bool
 CompositorParent::IsInCompositorThread()
 {
-  return sCompositorThread && sCompositorThread->thread_id() == PlatformThread::CurrentId();
+  return CompositorThread() && CompositorThread()->thread_id() == PlatformThread::CurrentId();
 }
 
 uint64_t
 CompositorParent::RootLayerTreeId()
 {
   return mRootLayerTreeID;
 }
 
 CompositorParent::~CompositorParent()
 {
   MOZ_COUNT_DTOR(CompositorParent);
-
-  ReleaseCompositorThread();
 }
 
 void
 CompositorParent::Destroy()
 {
   NS_ABORT_IF_FALSE(ManagedPLayerTransactionParent().Length() == 0,
                     "CompositorParent destroyed before managed PLayerTransactionParent");
 
@@ -914,37 +957,16 @@ CompositorParent::AllocPLayerTransaction
 
 bool
 CompositorParent::DeallocPLayerTransactionParent(PLayerTransactionParent* actor)
 {
   static_cast<LayerTransactionParent*>(actor)->ReleaseIPDLReference();
   return true;
 }
 
-
-typedef map<uint64_t,CompositorParent*> CompositorMap;
-static CompositorMap* sCompositorMap;
-
-void CompositorParent::CreateCompositorMap()
-{
-  if (sCompositorMap == nullptr) {
-    sCompositorMap = new CompositorMap;
-  }
-}
-
-void CompositorParent::DestroyCompositorMap()
-{
-  if (sCompositorMap != nullptr) {
-    NS_ASSERTION(sCompositorMap->empty(),
-                 "The Compositor map should be empty when destroyed>");
-    delete sCompositorMap;
-    sCompositorMap = nullptr;
-  }
-}
-
 CompositorParent* CompositorParent::GetCompositor(uint64_t id)
 {
   CompositorMap::iterator it = sCompositorMap->find(id);
   return it != sCompositorMap->end() ? it->second : nullptr;
 }
 
 void CompositorParent::AddCompositor(CompositorParent* compositor, uint64_t* outID)
 {
--- a/gfx/layers/ipc/CompositorParent.h
+++ b/gfx/layers/ipc/CompositorParent.h
@@ -58,16 +58,18 @@ struct ScopedLayerTreeRegistration
                               Layer* aRoot,
                               GeckoContentController* aController);
   ~ScopedLayerTreeRegistration();
 
 private:
   uint64_t mLayersId;
 };
 
+class CompositorThreadHolder;
+
 class CompositorParent : public PCompositorParent,
                          public ShadowLayersManager
 {
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(CompositorParent)
 
 public:
   CompositorParent(nsIWidget* aWidget,
                    bool aUseExternalSurfaceSize = false,
@@ -158,22 +160,24 @@ public:
    *
    * This message loop is used by CompositorParent and ImageBridgeParent.
    */
   static MessageLoop* CompositorLoop();
 
   /**
    * Creates the compositor thread and the global compositor map.
    */
-  static void StartUp();
+  static void StartUpCompositorThread();
 
   /**
-   * Destroys the compositor thread and the global compositor map.
+   * Drops the static reference to the compositor thread holder,
+   * allowing its shutdown (also destroying the compositor map) when it
+   * will no longer be referenced by any CompositorParent.
    */
-  static void ShutDown();
+  static void AllowShutdownOfCompositorThreadWhenUnreferenced();
 
   /**
    * Allocate an ID that can be used to refer to a layer tree and
    * associated resources that live only on the compositor thread.
    *
    * Must run on the content main thread.
    */
   static uint64_t AllocateLayerTreeId();
@@ -255,46 +259,16 @@ protected:
   void InitializeLayerManager(const nsTArray<LayersBackend>& aBackendHints);
   void PauseComposition();
   void ResumeComposition();
   void ResumeCompositionAndResize(int width, int height);
   void ForceComposition();
   void CancelCurrentCompositeTask();
 
   /**
-   * Creates a global map referencing each compositor by ID.
-   *
-   * This map is used by the ImageBridge protocol to trigger
-   * compositions without having to keep references to the
-   * compositor
-   */
-  static void CreateCompositorMap();
-  static void DestroyCompositorMap();
-
-  /**
-   * Creates the compositor thread.
-   *
-   * All compositors live on the same thread.
-   * The thread is not lazily created on first access to avoid dealing with
-   * thread safety. Therefore it's best to create and destroy the thread when
-   * we know we areb't using it (So creating/destroying along with gfxPlatform
-   * looks like a good place).
-   */
-  static bool CreateThread();
-
-  /**
-   * Destroys the compositor thread.
-   *
-   * It is safe to call this fucntion more than once, although the second call
-   * will have no effect.
-   * This function is not thread-safe.
-   */
-  static void DestroyThread();
-
-  /**
    * Add a compositor to the global compositor map.
    */
   static void AddCompositor(CompositorParent* compositor, uint64_t* id);
   /**
    * Remove a compositor from the global compositor map.
    */
   static CompositorParent* RemoveCompositor(uint64_t id);
 
@@ -331,15 +305,17 @@ protected:
   uint64_t mCompositorID;
   uint64_t mRootLayerTreeID;
 
   bool mOverrideComposeReadiness;
   CancelableTask* mForceCompositionTask;
 
   nsRefPtr<APZCTreeManager> mApzcTreeManager;
 
+  const nsRefPtr<CompositorThreadHolder> mCompositorThreadHolder;
+
   DISALLOW_EVIL_CONSTRUCTORS(CompositorParent);
 };
 
 } // layers
 } // mozilla
 
 #endif // mozilla_layers_CompositorParent_h
--- a/gfx/thebes/gfxPlatform.cpp
+++ b/gfx/thebes/gfxPlatform.cpp
@@ -365,17 +365,17 @@ gfxPlatform::Init()
 
 #ifdef DEBUG
     mozilla::gl::GLContext::StaticInit();
 #endif
 
     if (UsesOffMainThreadCompositing() &&
         XRE_GetProcessType() == GeckoProcessType_Default)
     {
-        mozilla::layers::CompositorParent::StartUp();
+        mozilla::layers::CompositorParent::StartUpCompositorThread();
         if (gfxPrefs::AsyncVideoEnabled()) {
             mozilla::layers::ImageBridgeChild::StartUp();
         }
 #ifdef MOZ_WIDGET_GONK
         SharedBufferManagerChild::StartUp();
 #endif
     }
 
@@ -516,17 +516,17 @@ gfxPlatform::ShutdownLayersIPC()
     {
         // This must happen after the shutdown of media and widgets, which
         // are triggered by the NS_XPCOM_SHUTDOWN_OBSERVER_ID notification.
         layers::ImageBridgeChild::ShutDown();
 #ifdef MOZ_WIDGET_GONK
         layers::SharedBufferManagerChild::ShutDown();
 #endif
 
-        layers::CompositorParent::ShutDown();
+        layers::CompositorParent::AllowShutdownOfCompositorThreadWhenUnreferenced();
     }
     gPlatform->mAlreadyShutDownLayersIPC = true;
 }
 
 gfxPlatform::~gfxPlatform()
 {
     mScreenReferenceSurface = nullptr;
     mScreenReferenceDrawTarget = nullptr;