Bug 1417784 - Shift how the AsyncImagePipelineManager is notified of updates. r?sotaro draft
authorKartikaya Gupta <kgupta@mozilla.com>
Mon, 28 May 2018 11:29:41 -0400
changeset 800593 087cf27350b4d3e0c72309bb66783ea7832963e0
parent 800592 a1d9bb2ed5843138e1ad43fa8a3f6ba436da1ffc
child 800594 0b1178b29bb4ab37853ed46c8b35a17b746302ae
push id111414
push userkgupta@mozilla.com
push dateMon, 28 May 2018 15:30:18 +0000
reviewerssotaro
bugs1417784
milestone62.0a1
Bug 1417784 - Shift how the AsyncImagePipelineManager is notified of updates. r?sotaro Instead of notifying the AsyncImagePipelineManager on the compositor thread via the CompositorBridgeParent, we can send it the new pipeline info directly from the RenderThread after the update happens. This effectively splits the AsyncImagePipelineManager update-processing into two parts: one that takes in the new pipeline info and one that process it. This allows us to invoke the processing step from other code running on the compositor thread, which we will need to do in the next patch. MozReview-Commit-ID: 7xhm8I7bY4C
gfx/layers/ipc/CompositorBridgeParent.cpp
gfx/layers/ipc/CompositorBridgeParent.h
gfx/layers/wr/AsyncImagePipelineManager.cpp
gfx/layers/wr/AsyncImagePipelineManager.h
gfx/webrender_bindings/RenderThread.cpp
--- a/gfx/layers/ipc/CompositorBridgeParent.cpp
+++ b/gfx/layers/ipc/CompositorBridgeParent.cpp
@@ -2136,33 +2136,21 @@ CompositorBridgeParent::DidComposite(Tim
     mTxnStartTime = TimeStamp();
     mFwdTime = TimeStamp();
 #endif
     mPendingTransaction = TransactionId{0};
   }
 }
 
 void
-CompositorBridgeParent::NotifyPipelineRemoved(const wr::PipelineId& aPipelineId)
-{
-  if (mAsyncImageManager) {
-    mAsyncImageManager->PipelineRemoved(aPipelineId);
-  }
-}
-
-void
 CompositorBridgeParent::NotifyPipelineRendered(const wr::PipelineId& aPipelineId,
                                                const wr::Epoch& aEpoch,
                                                TimeStamp& aCompositeStart,
                                                TimeStamp& aCompositeEnd)
 {
-  if (mAsyncImageManager) {
-    mAsyncImageManager->PipelineRendered(aPipelineId, aEpoch);
-  }
-
   if (!mWrBridge) {
     return;
   }
 
   if (mWrBridge->PipelineId() == aPipelineId) {
     mWrBridge->RemoveEpochDataPriorTo(aEpoch);
 
     if (!mPaused) {
@@ -2190,16 +2178,22 @@ CompositorBridgeParent::NotifyPipelineRe
         CrossProcessCompositorBridgeParent* cpcp = lts->mCrossProcessParent;
         TransactionId transactionId = lts->mWrBridge->FlushTransactionIdsForEpoch(aEpoch, aCompositeEnd);
         Unused << cpcp->SendDidComposite(aLayersId, transactionId, aCompositeStart, aCompositeEnd);
       }
     }
   });
 }
 
+RefPtr<AsyncImagePipelineManager>
+CompositorBridgeParent::GetAsyncImagePipelineManager() const
+{
+  return mAsyncImageManager;
+}
+
 void
 CompositorBridgeParent::NotifyDidComposite(TransactionId aTransactionId, TimeStamp& aCompositeStart, TimeStamp& aCompositeEnd)
 {
   Unused << SendDidComposite(LayersId{0}, aTransactionId, aCompositeStart, aCompositeEnd);
 
   if (mLayerManager) {
     nsTArray<ImageCompositeNotificationInfo> notifications;
     mLayerManager->ExtractImageCompositeNotifications(&notifications);
--- a/gfx/layers/ipc/CompositorBridgeParent.h
+++ b/gfx/layers/ipc/CompositorBridgeParent.h
@@ -264,18 +264,17 @@ public:
 
   bool IsSameProcess() const override;
 
   void NotifyWebRenderError(wr::WebRenderError aError);
   void NotifyPipelineRendered(const wr::PipelineId& aPipelineId,
                               const wr::Epoch& aEpoch,
                               TimeStamp& aCompositeStart,
                               TimeStamp& aCompositeEnd);
-  void NotifyPipelineRemoved(const wr::PipelineId& aPipelineId);
-
+  RefPtr<AsyncImagePipelineManager> GetAsyncImagePipelineManager() const;
 
   PCompositorWidgetParent* AllocPCompositorWidgetParent(const CompositorWidgetInitData& aInitData) override;
   bool DeallocPCompositorWidgetParent(PCompositorWidgetParent* aActor) override;
 
   void ObserveLayerUpdate(LayersId aLayersId, uint64_t aEpoch, bool aActive) override { }
 
   /**
    * This forces the is-first-paint flag to true. This is intended to
--- a/gfx/layers/wr/AsyncImagePipelineManager.cpp
+++ b/gfx/layers/wr/AsyncImagePipelineManager.cpp
@@ -8,16 +8,17 @@
 
 #include "CompositableHost.h"
 #include "gfxEnv.h"
 #include "mozilla/gfx/gfxVars.h"
 #include "mozilla/layers/CompositorThread.h"
 #include "mozilla/layers/SharedSurfacesParent.h"
 #include "mozilla/layers/WebRenderImageHost.h"
 #include "mozilla/layers/WebRenderTextureHost.h"
+#include "mozilla/webrender/RenderThread.h"
 #include "mozilla/webrender/WebRenderAPI.h"
 #include "mozilla/webrender/WebRenderTypes.h"
 
 namespace mozilla {
 namespace layers {
 
 AsyncImagePipelineManager::AsyncImagePipeline::AsyncImagePipeline()
  : mInitialised(false)
@@ -30,16 +31,17 @@ AsyncImagePipelineManager::AsyncImagePip
 
 AsyncImagePipelineManager::AsyncImagePipelineManager(already_AddRefed<wr::WebRenderAPI>&& aApi)
  : mApi(aApi)
  , mIdNamespace(mApi->GetNamespace())
  , mResourceId(0)
  , mAsyncImageEpoch{0}
  , mWillGenerateFrame(false)
  , mDestroyed(false)
+ , mUpdatesLock("UpdatesLock")
 {
   MOZ_COUNT_CTOR(AsyncImagePipelineManager);
 }
 
 AsyncImagePipelineManager::~AsyncImagePipelineManager()
 {
   MOZ_COUNT_DTOR(AsyncImagePipelineManager);
 }
@@ -400,21 +402,74 @@ AsyncImagePipelineManager::HoldExternalI
     SharedSurfacesParent::Release(aImageId);
     return;
   }
 
   holder->mExternalImages.push(ForwardingExternalImage(aEpoch, aImageId));
 }
 
 void
-AsyncImagePipelineManager::PipelineRendered(const wr::PipelineId& aPipelineId, const wr::Epoch& aEpoch)
+AsyncImagePipelineManager::NotifyPipelinesUpdated(wr::WrPipelineInfo aInfo)
 {
+  // This is called on the render thread, so we just stash the data into
+  // mUpdatesQueue and process it later on the compositor thread.
+  MOZ_ASSERT(wr::RenderThread::IsInRenderThread());
+
+  MutexAutoLock lock(mUpdatesLock);
+  for (uintptr_t i = 0; i < aInfo.epochs.length; i++) {
+    mUpdatesQueue.push(std::make_pair(
+        aInfo.epochs.data[i].pipeline_id,
+        Some(aInfo.epochs.data[i].epoch)));
+  }
+  for (uintptr_t i = 0; i < aInfo.removed_pipelines.length; i++) {
+    mUpdatesQueue.push(std::make_pair(
+        aInfo.removed_pipelines.data[i],
+        Nothing()));
+  }
+  // Queue a runnable on the compositor thread to process the queue
+  layers::CompositorThreadHolder::Loop()->PostTask(
+      NewRunnableMethod("ProcessPipelineUpdates",
+                        this,
+                        &AsyncImagePipelineManager::ProcessPipelineUpdates));
+}
+
+void
+AsyncImagePipelineManager::ProcessPipelineUpdates()
+{
+  MOZ_ASSERT(CompositorThreadHolder::IsInCompositorThread());
+
   if (mDestroyed) {
     return;
   }
+
+  while (true) {
+    wr::PipelineId pipelineId;
+    Maybe<wr::Epoch> epoch;
+
+    { // scope lock to extract one item from the queue
+      MutexAutoLock lock(mUpdatesLock);
+      if (mUpdatesQueue.empty()) {
+        break;
+      }
+      pipelineId = mUpdatesQueue.front().first;
+      epoch = mUpdatesQueue.front().second;
+      mUpdatesQueue.pop();
+    }
+
+    if (epoch.isSome()) {
+      ProcessPipelineRendered(pipelineId, *epoch);
+    } else {
+      ProcessPipelineRemoved(pipelineId);
+    }
+  }
+}
+
+void
+AsyncImagePipelineManager::ProcessPipelineRendered(const wr::PipelineId& aPipelineId, const wr::Epoch& aEpoch)
+{
   if (auto entry = mPipelineTexturesHolders.Lookup(wr::AsUint64(aPipelineId))) {
     PipelineTexturesHolder* holder = entry.Data();
     // Release TextureHosts based on Epoch
     while (!holder->mTextureHosts.empty()) {
       if (aEpoch <= holder->mTextureHosts.front().mEpoch) {
         break;
       }
       holder->mTextureHosts.pop();
@@ -427,17 +482,17 @@ AsyncImagePipelineManager::PipelineRende
         SharedSurfacesParent::Release(holder->mExternalImages.front().mImageId);
       MOZ_ASSERT(released);
       holder->mExternalImages.pop();
     }
   }
 }
 
 void
-AsyncImagePipelineManager::PipelineRemoved(const wr::PipelineId& aPipelineId)
+AsyncImagePipelineManager::ProcessPipelineRemoved(const wr::PipelineId& aPipelineId)
 {
   if (mDestroyed) {
     return;
   }
   if (auto entry = mPipelineTexturesHolders.Lookup(wr::AsUint64(aPipelineId))) {
     if (entry.Data()->mDestroyedEpoch.isSome()) {
       // Remove Pipeline
       entry.Remove();
--- a/gfx/layers/wr/AsyncImagePipelineManager.h
+++ b/gfx/layers/wr/AsyncImagePipelineManager.h
@@ -44,18 +44,25 @@ protected:
 public:
   void Destroy();
 
   void AddPipeline(const wr::PipelineId& aPipelineId);
   void RemovePipeline(const wr::PipelineId& aPipelineId, const wr::Epoch& aEpoch);
 
   void HoldExternalImage(const wr::PipelineId& aPipelineId, const wr::Epoch& aEpoch, WebRenderTextureHost* aTexture);
   void HoldExternalImage(const wr::PipelineId& aPipelineId, const wr::Epoch& aEpoch, const wr::ExternalImageId& aImageId);
-  void PipelineRendered(const wr::PipelineId& aPipelineId, const wr::Epoch& aEpoch);
-  void PipelineRemoved(const wr::PipelineId& aPipelineId);
+
+  // This is called from the Renderer thread to notify this class about the
+  // pipelines in the most recently completed render. A copy of the update
+  // information is put into mUpdatesQueue.
+  void NotifyPipelinesUpdated(wr::WrPipelineInfo aInfo);
+
+  // This is run on the compositor thread to process mUpdatesQueue. We make
+  // this a public entry point because we need to invoke it from other places.
+  void ProcessPipelineUpdates();
 
   TimeStamp GetCompositionTime() const {
     return mCompositionTime;
   }
   void SetCompositionTime(TimeStamp aTimeStamp) {
     mCompositionTime = aTimeStamp;
     if (!mCompositionTime.IsNull() && !mCompositeUntilTime.IsNull() &&
         mCompositionTime >= mCompositeUntilTime) {
@@ -92,16 +99,18 @@ public:
   {
     aNotifications->AppendElements(Move(mImageCompositeNotifications));
   }
 
   void SetWillGenerateFrame();
   bool GetAndResetWillGenerateFrame();
 
 private:
+  void ProcessPipelineRendered(const wr::PipelineId& aPipelineId, const wr::Epoch& aEpoch);
+  void ProcessPipelineRemoved(const wr::PipelineId& aPipelineId);
 
   wr::Epoch GetNextImageEpoch();
   uint32_t GetNextResourceId() { return ++mResourceId; }
   wr::IdNamespace GetNamespace() { return mIdNamespace; }
   wr::ImageKey GenerateImageKey()
   {
     wr::ImageKey key;
     key.mNamespace = GetNamespace();
@@ -192,14 +201,23 @@ private:
   TimeStamp mCompositionTime;
 
   // When nonnull, during rendering, some compositable indicated that it will
   // change its rendering at this time. In order not to miss it, we composite
   // on every vsync until this time occurs (this is the latest such time).
   TimeStamp mCompositeUntilTime;
 
   nsTArray<ImageCompositeNotificationInfo> mImageCompositeNotifications;
+
+  // The lock that protects mUpdatesQueue
+  Mutex mUpdatesLock;
+  // Queue to store rendered pipeline epoch information. This is populated from
+  // the Renderer thread after a render, and is read from the compositor thread
+  // to free resources (e.g. textures) that are no longer needed. Each entry
+  // in the queue is a pair that holds the pipeline id and Some(x) for
+  // a render of epoch x, or Nothing() for a removed pipeline.
+  std::queue<std::pair<wr::PipelineId, Maybe<wr::Epoch>>> mUpdatesQueue;
 };
 
 } // namespace layers
 } // namespace mozilla
 
 #endif /* MOZILLA_GFX_WEBRENDERCOMPOSITABLE_HOLDER_H */
--- a/gfx/webrender_bindings/RenderThread.cpp
+++ b/gfx/webrender_bindings/RenderThread.cpp
@@ -4,16 +4,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "base/task.h"
 #include "GeckoProfiler.h"
 #include "RenderThread.h"
 #include "nsThreadUtils.h"
 #include "mtransport/runnable_utils.h"
+#include "mozilla/layers/AsyncImagePipelineManager.h"
 #include "mozilla/layers/CompositorThread.h"
 #include "mozilla/layers/CompositorBridgeParent.h"
 #include "mozilla/layers/SharedSurfacesParent.h"
 #include "mozilla/StaticPtr.h"
 #include "mozilla/webrender/RendererOGL.h"
 #include "mozilla/webrender/RenderTextureHost.h"
 #include "mozilla/widget/CompositorWidget.h"
 
@@ -248,19 +249,16 @@ NotifyDidRender(layers::CompositorBridge
 {
   for (uintptr_t i = 0; i < aInfo.epochs.length; i++) {
     aBridge->NotifyPipelineRendered(
         aInfo.epochs.data[i].pipeline_id,
         aInfo.epochs.data[i].epoch,
         aStart,
         aEnd);
   }
-  for (uintptr_t i = 0; i < aInfo.removed_pipelines.length; i++) {
-    aBridge->NotifyPipelineRemoved(aInfo.removed_pipelines.data[i]);
-  }
 
   wr_pipeline_info_delete(aInfo);
 }
 
 void
 RenderThread::UpdateAndRender(wr::WindowId aWindowId, bool aReadback)
 {
   AUTO_PROFILER_TRACING("Paint", "Composite");
@@ -279,16 +277,26 @@ RenderThread::UpdateAndRender(wr::Window
   if (!ret) {
     // Render did not happen, do not call NotifyDidRender.
     return;
   }
 
   TimeStamp end = TimeStamp::Now();
 
   auto info = renderer->FlushPipelineInfo();
+  RefPtr<layers::AsyncImagePipelineManager> pipelineMgr =
+      renderer->GetCompositorBridge()->GetAsyncImagePipelineManager();
+  // pipelineMgr should always be non-null here because it is only nulled out
+  // after the WebRenderAPI instance for the CompositorBridgeParent is
+  // destroyed, and that destruction blocks until the renderer thread has
+  // removed the relevant renderer. And after that happens we should never reach
+  // this code at all; it would bail out at the mRenderers.find check above.
+  MOZ_ASSERT(pipelineMgr);
+  pipelineMgr->NotifyPipelinesUpdated(info);
+
   layers::CompositorThreadHolder::Loop()->PostTask(NewRunnableFunction(
     "NotifyDidRenderRunnable",
     &NotifyDidRender,
     renderer->GetCompositorBridge(),
     info,
     start, end
   ));
 }