Bug 1453801 - Part 2. Ensure shared surfaces are properly released from render texture cache. r=sotaro
authorAndrew Osmond <aosmond@mozilla.com>
Mon, 23 Apr 2018 07:57:15 -0400
changeset 468633 c9f4fd7170ad7602981791cb1db1288afed61472
parent 468632 f3eeeae62a5a723d500cd9d1bd92beeae363f2c1
child 468634 d1479b21a2848f1ed2b73fb54202b5ba5e042ef3
push id9165
push userasasaki@mozilla.com
push dateThu, 26 Apr 2018 21:04:54 +0000
treeherdermozilla-beta@064c3804de2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssotaro
bugs1453801
milestone61.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 1453801 - Part 2. Ensure shared surfaces are properly released from render texture cache. r=sotaro
gfx/layers/SourceSurfaceSharedData.h
gfx/layers/ipc/SharedSurfacesParent.cpp
gfx/layers/wr/AsyncImagePipelineManager.cpp
gfx/layers/wr/AsyncImagePipelineManager.h
gfx/layers/wr/WebRenderBridgeParent.cpp
gfx/layers/wr/WebRenderBridgeParent.h
--- a/gfx/layers/SourceSurfaceSharedData.h
+++ b/gfx/layers/SourceSurfaceSharedData.h
@@ -35,16 +35,17 @@ class SourceSurfaceSharedDataWrapper fin
 {
   typedef mozilla::ipc::SharedMemoryBasic SharedMemoryBasic;
 
 public:
   MOZ_DECLARE_REFCOUNTED_VIRTUAL_TYPENAME(SourceSurfaceSharedDataWrapper, override)
 
   SourceSurfaceSharedDataWrapper()
     : mStride(0)
+    , mConsumers(0)
     , mFormat(SurfaceFormat::UNKNOWN)
   { }
 
   bool Init(const IntSize& aSize,
             int32_t aStride,
             SurfaceFormat aFormat,
             const SharedMemoryBasic::Handle& aHandle,
             base::ProcessId aCreatorPid);
@@ -67,26 +68,16 @@ public:
     return static_cast<uint8_t*>(mBuf->memory());
   }
 
   bool OnHeap() const override
   {
     return false;
   }
 
-  bool Map(MapType, MappedSurface *aMappedSurface) override
-  {
-    aMappedSurface->mData = GetData();
-    aMappedSurface->mStride = mStride;
-    return true;
-  }
-
-  void Unmap() override
-  { }
-
   bool AddConsumer()
   {
     return ++mConsumers == 1;
   }
 
   bool RemoveConsumer()
   {
     MOZ_ASSERT(mConsumers > 0);
--- a/gfx/layers/ipc/SharedSurfacesParent.cpp
+++ b/gfx/layers/ipc/SharedSurfacesParent.cpp
@@ -83,16 +83,17 @@ SharedSurfacesParent::Release(const wr::
   uint64_t id = wr::AsUint64(aId);
   RefPtr<SourceSurfaceSharedDataWrapper> surface;
   sInstance->mSurfaces.Get(wr::AsUint64(aId), getter_AddRefs(surface));
   if (!surface) {
     return false;
   }
 
   if (surface->RemoveConsumer()) {
+    wr::RenderThread::Get()->UnregisterExternalImage(id);
     sInstance->mSurfaces.Remove(id);
   }
 
   return true;
 }
 
 /* static */ void
 SharedSurfacesParent::AddSameProcess(const wr::ExternalImageId& aId,
@@ -119,49 +120,51 @@ SharedSurfacesParent::AddSameProcess(con
       }
 
       MOZ_ASSERT(!sInstance->mSurfaces.Contains(id));
 
       RefPtr<wr::RenderSharedSurfaceTextureHost> texture =
         new wr::RenderSharedSurfaceTextureHost(surface);
       wr::RenderThread::Get()->RegisterExternalImage(id, texture.forget());
 
+      surface->AddConsumer();
       sInstance->mSurfaces.Put(id, surface);
     });
 
   CompositorThreadHolder::Loop()->PostTask(task.forget());
 }
 
 /* static */ void
 SharedSurfacesParent::RemoveSameProcess(const wr::ExternalImageId& aId)
 {
   MOZ_ASSERT(XRE_IsParentProcess());
   MOZ_ASSERT(NS_IsMainThread());
 
   const wr::ExternalImageId id(aId);
   RefPtr<Runnable> task = NS_NewRunnableFunction(
     "layers::SharedSurfacesParent::RemoveSameProcess",
     [id]() -> void {
-      Remove(id);
+      Release(id);
     });
 
   CompositorThreadHolder::Loop()->PostTask(task.forget());
 }
 
 /* static */ void
 SharedSurfacesParent::DestroyProcess(base::ProcessId aPid)
 {
   if (!sInstance) {
     return;
   }
 
   // Note that the destruction of a parent may not be cheap if it still has a
   // lot of surfaces still bound that require unmapping.
   for (auto i = sInstance->mSurfaces.Iter(); !i.Done(); i.Next()) {
-    if (i.Data()->GetCreatorPid() == aPid) {
+    SourceSurfaceSharedDataWrapper* surface = i.Data();
+    if (surface->GetCreatorPid() == aPid && surface->RemoveConsumer()) {
       wr::RenderThread::Get()->UnregisterExternalImage(i.Key());
       i.Remove();
     }
   }
 }
 
 /* static */ void
 SharedSurfacesParent::Add(const wr::ExternalImageId& aId,
@@ -185,16 +188,17 @@ SharedSurfacesParent::Add(const wr::Exte
 
   uint64_t id = wr::AsUint64(aId);
   MOZ_ASSERT(!sInstance->mSurfaces.Contains(id));
 
   RefPtr<wr::RenderSharedSurfaceTextureHost> texture =
     new wr::RenderSharedSurfaceTextureHost(surface);
   wr::RenderThread::Get()->RegisterExternalImage(id, texture.forget());
 
+  surface->AddConsumer();
   sInstance->mSurfaces.Put(id, surface.forget());
 }
 
 /* static */ void
 SharedSurfacesParent::Remove(const wr::ExternalImageId& aId)
 {
   //MOZ_ASSERT(CompositorThreadHolder::IsInCompositorThread());
   DebugOnly<bool> rv = Release(aId);
--- a/gfx/layers/wr/AsyncImagePipelineManager.cpp
+++ b/gfx/layers/wr/AsyncImagePipelineManager.cpp
@@ -5,16 +5,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "AsyncImagePipelineManager.h"
 
 #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/WebRenderAPI.h"
 #include "mozilla/webrender/WebRenderTypes.h"
 
 namespace mozilla {
 namespace layers {
 
@@ -378,30 +379,57 @@ AsyncImagePipelineManager::HoldExternalI
   if (!holder) {
     return;
   }
   // Hold WebRenderTextureHost until end of its usage on RenderThread
   holder->mTextureHosts.push(ForwardingTextureHost(aEpoch, aTexture));
 }
 
 void
+AsyncImagePipelineManager::HoldExternalImage(const wr::PipelineId& aPipelineId, const wr::Epoch& aEpoch, const wr::ExternalImageId& aImageId)
+{
+  if (mDestroyed) {
+    SharedSurfacesParent::Release(aImageId);
+    return;
+  }
+
+  PipelineTexturesHolder* holder = mPipelineTexturesHolders.Get(wr::AsUint64(aPipelineId));
+  MOZ_ASSERT(holder);
+  if (!holder) {
+    SharedSurfacesParent::Release(aImageId);
+    return;
+  }
+
+  holder->mExternalImages.push(ForwardingExternalImage(aEpoch, aImageId));
+}
+
+void
 AsyncImagePipelineManager::PipelineRendered(const wr::PipelineId& aPipelineId, const wr::Epoch& aEpoch)
 {
   if (mDestroyed) {
     return;
   }
   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();
     }
+    while (!holder->mExternalImages.empty()) {
+      if (aEpoch <= holder->mExternalImages.front().mEpoch) {
+        break;
+      }
+      DebugOnly<bool> released =
+        SharedSurfacesParent::Release(holder->mExternalImages.front().mImageId);
+      MOZ_ASSERT(released);
+      holder->mExternalImages.pop();
+    }
   }
 }
 
 void
 AsyncImagePipelineManager::PipelineRemoved(const wr::PipelineId& aPipelineId)
 {
   if (mDestroyed) {
     return;
--- a/gfx/layers/wr/AsyncImagePipelineManager.h
+++ b/gfx/layers/wr/AsyncImagePipelineManager.h
@@ -43,16 +43,17 @@ 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);
 
   TimeStamp GetCompositionTime() const {
     return mCompositionTime;
   }
   void SetCompositionTime(TimeStamp aTimeStamp) {
     mCompositionTime = aTimeStamp;
@@ -112,19 +113,29 @@ private:
     ForwardingTextureHost(const wr::Epoch& aEpoch, TextureHost* aTexture)
       : mEpoch(aEpoch)
       , mTexture(aTexture)
     {}
     wr::Epoch mEpoch;
     CompositableTextureHostRef mTexture;
   };
 
+  struct ForwardingExternalImage {
+    ForwardingExternalImage(const wr::Epoch& aEpoch, const wr::ExternalImageId& aImageId)
+      : mEpoch(aEpoch)
+      , mImageId(aImageId)
+    {}
+    wr::Epoch mEpoch;
+    wr::ExternalImageId mImageId;
+  };
+
   struct PipelineTexturesHolder {
     // Holds forwarding WebRenderTextureHosts.
     std::queue<ForwardingTextureHost> mTextureHosts;
+    std::queue<ForwardingExternalImage> mExternalImages;
     Maybe<wr::Epoch> mDestroyedEpoch;
   };
 
   struct AsyncImagePipeline {
     AsyncImagePipeline();
     void Update(const LayoutDeviceRect& aScBounds,
                 const gfx::Matrix4x4& aScTransform,
                 const gfx::MaybeIntSize& aScaleToSize,
--- a/gfx/layers/wr/WebRenderBridgeParent.cpp
+++ b/gfx/layers/wr/WebRenderBridgeParent.cpp
@@ -379,18 +379,26 @@ WebRenderBridgeParent::AddExternalImage(
                                         wr::TransactionBuilder& aResources)
 {
   Range<wr::ImageKey> keys(&aKey, 1);
   // Check if key is obsoleted.
   if (keys[0].mNamespace != mIdNamespace) {
     return true;
   }
 
-  RefPtr<DataSourceSurface> dSurf = SharedSurfacesParent::Get(aExtId);
+  RefPtr<DataSourceSurface> dSurf = SharedSurfacesParent::Acquire(aExtId);
   if (dSurf) {
+    bool inserted = mSharedSurfaceIds.EnsureInserted(wr::AsUint64(aExtId));
+    if (!inserted) {
+      // We already have a mapping for this image, so decrement the ownership
+      // counter just increased unnecessarily. This can happen when an image is
+      // slow to decode and we need to invalidate it by updating its image key.
+      SharedSurfacesParent::Release(aExtId);
+    }
+
     if (!gfxEnv::EnableWebRenderRecording()) {
       wr::ImageDescriptor descriptor(dSurf->GetSize(), dSurf->Stride(),
                                      dSurf->GetFormat());
       aResources.AddExternalImage(aKey, descriptor, aExtId,
                                   wr::WrExternalImageBufferType::ExternalBuffer,
                                   0);
       return true;
     }
@@ -938,23 +946,29 @@ WebRenderBridgeParent::AddExternalImageI
 
 void
 WebRenderBridgeParent::RemoveExternalImageId(const ExternalImageId& aImageId)
 {
   if (mDestroyed) {
     return;
   }
 
-  WebRenderImageHost* wrHost = mExternalImageIds.Get(wr::AsUint64(aImageId)).get();
+  uint64_t imageId = wr::AsUint64(aImageId);
+  if (mSharedSurfaceIds.EnsureRemoved(imageId)) {
+    mAsyncImageManager->HoldExternalImage(mPipelineId, mWrEpoch, aImageId);
+    return;
+  }
+
+  WebRenderImageHost* wrHost = mExternalImageIds.Get(imageId).get();
   if (!wrHost) {
     return;
   }
 
   wrHost->ClearWrBridge();
-  mExternalImageIds.Remove(wr::AsUint64(aImageId));
+  mExternalImageIds.Remove(imageId);
 
   return;
 }
 
 mozilla::ipc::IPCResult
 WebRenderBridgeParent::RecvSetLayerObserverEpoch(const uint64_t& aLayerObserverEpoch)
 {
   if (mDestroyed) {
@@ -1427,16 +1441,21 @@ WebRenderBridgeParent::ClearResources()
   mExternalImageIds.Clear();
   for (auto iter = mAsyncCompositables.Iter(); !iter.Done(); iter.Next()) {
     wr::PipelineId pipelineId = wr::AsPipelineId(iter.Key());
     RefPtr<WebRenderImageHost> host = iter.Data();
     host->ClearWrBridge();
     mAsyncImageManager->RemoveAsyncImagePipeline(pipelineId, txn);
   }
   mAsyncCompositables.Clear();
+  for (auto iter = mSharedSurfaceIds.Iter(); !iter.Done(); iter.Next()) {
+    wr::ExternalImageId id = wr::ToExternalImageId(iter.Get()->GetKey());
+    mAsyncImageManager->HoldExternalImage(mPipelineId, mWrEpoch, id);
+  }
+  mSharedSurfaceIds.Clear();
 
   mAsyncImageManager->RemovePipeline(mPipelineId, wrEpoch);
   txn.RemovePipeline(mPipelineId);
 
   mApi->SendTransaction(txn);
 
   for (std::unordered_set<uint64_t>::iterator iter = mActiveAnimations.begin(); iter != mActiveAnimations.end(); iter++) {
     mAnimStorage->ClearById(*iter);
--- a/gfx/layers/wr/WebRenderBridgeParent.h
+++ b/gfx/layers/wr/WebRenderBridgeParent.h
@@ -246,16 +246,17 @@ private:
   RefPtr<AsyncImagePipelineManager> mAsyncImageManager;
   RefPtr<CompositorVsyncScheduler> mCompositorScheduler;
   RefPtr<CompositorAnimationStorage> mAnimStorage;
   // mActiveAnimations is used to avoid leaking animations when WebRenderBridgeParent is
   // destroyed abnormally and Tab move between different windows.
   std::unordered_set<uint64_t> mActiveAnimations;
   nsDataHashtable<nsUint64HashKey, RefPtr<WebRenderImageHost>> mAsyncCompositables;
   nsDataHashtable<nsUint64HashKey, RefPtr<WebRenderImageHost>> mExternalImageIds;
+  nsTHashtable<nsUint64HashKey> mSharedSurfaceIds;
 
   TimeStamp mPreviousFrameTimeStamp;
   // These fields keep track of the latest layer observer epoch values in the child and the
   // parent. mChildLayerObserverEpoch is the latest epoch value received from the child.
   // mParentLayerObserverEpoch is the latest epoch value that we have told TabParent about
   // (via ObserveLayerUpdate).
   uint64_t mChildLayerObserverEpoch;
   uint64_t mParentLayerObserverEpoch;