Bug 1328797 - Part 1: Don't use a sync transaction for RemoveTexture. r=nical
authorMatt Woodrow <mwoodrow@mozilla.com>
Thu, 16 Feb 2017 11:28:24 +1300
changeset 485031 82c34cf618e8a3ba422e35f13d5041b0544893da
parent 485030 2e78a0da72687a894a87dcb742d8cdd78c85c21c
child 485032 6e42452c2616386464dbdc1cdd08deafe31387e4
push id45611
push userbmo:gasolin@mozilla.com
push dateThu, 16 Feb 2017 01:39:57 +0000
reviewersnical
bugs1328797
milestone54.0a1
Bug 1328797 - Part 1: Don't use a sync transaction for RemoveTexture. r=nical
gfx/layers/client/TextureClient.cpp
gfx/layers/client/TextureClient.h
gfx/layers/composite/TextureHost.cpp
gfx/layers/ipc/CompositableForwarder.h
gfx/layers/ipc/ImageBridgeChild.cpp
gfx/layers/ipc/ImageBridgeChild.h
gfx/layers/ipc/PTexture.ipdl
gfx/layers/ipc/ShadowLayers.cpp
gfx/layers/ipc/ShadowLayers.h
gfx/layers/wr/WebRenderBridgeChild.cpp
gfx/layers/wr/WebRenderBridgeChild.h
--- a/gfx/layers/client/TextureClient.cpp
+++ b/gfx/layers/client/TextureClient.cpp
@@ -142,23 +142,16 @@ private:
   }
 
   /// The normal way to destroy the actor.
   ///
   /// This will asynchronously send a Destroy message to the parent actor, whom
   /// will send the delete message.
   void Destroy(const TextureDeallocParams& aParams);
 
-  /// The ugly and slow way to destroy the actor.
-  ///
-  /// This will block until the Parent actor has handled the Destroy message,
-  /// and then start the asynchronous handshake (and destruction will already
-  /// be done on the parent side, when the async part happens).
-  void DestroySynchronously(const TextureDeallocParams& aParams);
-
   // This lock is used order to prevent several threads to access the
   // TextureClient's data concurrently. In particular, it prevents shutdown
   // code to destroy a texture while another thread is reading or writing into
   // it.
   // In most places, the lock is held in short and bounded scopes in which we
   // don't block on any other resource. There are few exceptions to this, which
   // are discussed below.
   //
@@ -274,57 +267,36 @@ TextureChild::Destroy(const TextureDeall
 {
   MOZ_ASSERT(!mOwnerCalledDestroy);
   if (mOwnerCalledDestroy) {
     return;
   }
 
   mOwnerCalledDestroy = true;
 
+  if (!IPCOpen()) {
+    DestroyTextureData(
+      aParams.data,
+      aParams.allocator,
+      aParams.clientDeallocation,
+      mMainThreadOnly);
+    return;
+  }
+
   // DestroyTextureData will be called by TextureChild::ActorDestroy
   mTextureData = aParams.data;
   mOwnsTextureData = aParams.clientDeallocation;
 
   if (!mCompositableForwarder ||
-      !mCompositableForwarder->DestroyInTransaction(this, false))
+      !mCompositableForwarder->DestroyInTransaction(this))
   {
     this->SendDestroy();
   }
 }
 
-void
-TextureChild::DestroySynchronously(const TextureDeallocParams& aParams)
-{
-  MOZ_PERFORMANCE_WARNING("gfx", "TextureClient/Host pair requires synchronous deallocation");
-
-  MOZ_ASSERT(!mOwnerCalledDestroy);
-  if (mOwnerCalledDestroy) {
-    return;
-  }
-
-  mOwnerCalledDestroy = true;
-
-  DestroyTextureData(
-    aParams.data,
-    aParams.allocator,
-    aParams.clientDeallocation,
-    mMainThreadOnly);
-
-  if (!IPCOpen()) {
-    return;
-  }
-
-  if (!mCompositableForwarder ||
-      !mCompositableForwarder->DestroyInTransaction(this, true))
-  {
-    this->SendDestroySync();
-    this->SendDestroy();
-  }
-}
-
 /* static */ Atomic<uint64_t> TextureClient::sSerialCounter(0);
 
 void DeallocateTextureClientSyncProxy(TextureDeallocParams params,
                                         ReentrantMonitor* aBarrier, bool* aDone)
 {
   DeallocateTextureClient(params);
   ReentrantMonitorAutoEnter autoMon(*aBarrier);
   *aDone = true;
@@ -396,24 +368,20 @@ DeallocateTextureClient(TextureDeallocPa
     // our data.
     bool shouldDeallocate = !params.workAroundSharedSurfaceOwnershipIssue;
     DestroyTextureData(params.data, params.allocator,
                        shouldDeallocate,
                        false);  // main-thread deallocation
     return;
   }
 
-  if (params.syncDeallocation || !actor->IPCOpen()) {
-    actor->DestroySynchronously(params);
-  } else {
-    actor->Destroy(params);
-  }
+  actor->Destroy(params);
 }
 
-void TextureClient::Destroy(bool aForceSync)
+void TextureClient::Destroy()
 {
   if (mActor && !mIsLocked) {
     mActor->Lock();
   }
 
   mBorrowedDrawTarget = nullptr;
   mReadLock = nullptr;
 
@@ -439,17 +407,17 @@ void TextureClient::Destroy(bool aForceS
     if (mWorkaroundAnnoyingSharedSurfaceLifetimeIssues) {
       params.data = nullptr;
     } else {
       params.data = data;
     }
     // At the moment we always deallocate synchronously when deallocating on the
     // client side, but having asynchronous deallocate in some of the cases will
     // be a worthwhile optimization.
-    params.syncDeallocation = !!(mFlags & TextureFlags::DEALLOCATE_CLIENT) || aForceSync;
+    params.syncDeallocation = !!(mFlags & TextureFlags::DEALLOCATE_CLIENT);
 
     // Release the lock before calling DeallocateTextureClient because the latter
     // may wait for the main thread which could create a dead-lock.
 
     if (actor) {
       actor->Unlock();
     }
 
@@ -626,17 +594,17 @@ TextureClient::SerializeReadLock(ReadLoc
   } else {
     aDescriptor = null_t();
   }
 }
 
 TextureClient::~TextureClient()
 {
   mReadLock = nullptr;
-  Destroy(false);
+  Destroy();
 }
 
 void
 TextureClient::UpdateFromSurface(gfx::SourceSurface* aSurface)
 {
   MOZ_ASSERT(IsValid());
   MOZ_ASSERT(mIsLocked);
   MOZ_ASSERT(aSurface);
--- a/gfx/layers/client/TextureClient.h
+++ b/gfx/layers/client/TextureClient.h
@@ -583,20 +583,18 @@ public:
   PTextureChild* GetIPDLActor();
 
   /**
    * Triggers the destruction of the shared data and the corresponding TextureHost.
    *
    * If the texture flags contain TextureFlags::DEALLOCATE_CLIENT, the destruction
    * will be synchronously coordinated with the compositor side, otherwise it
    * will be done asynchronously.
-   * If sync is true, the destruction will be synchronous regardless of the
-   * texture's flags (bad for performance, use with care).
    */
-  void Destroy(bool sync = false);
+  void Destroy();
 
   /**
    * Track how much of this texture is wasted.
    * For example we might allocate a 256x256 tile but only use 10x10.
    */
   void SetWaste(int aWasteArea) {
     mWasteTracker.Update(aWasteArea, BytesPerPixel(GetFormat()));
   }
--- a/gfx/layers/composite/TextureHost.cpp
+++ b/gfx/layers/composite/TextureHost.cpp
@@ -76,21 +76,16 @@ public:
   virtual mozilla::ipc::IPCResult RecvRecycleTexture(const TextureFlags& aTextureFlags) override;
 
   TextureHost* GetTextureHost() { return mTextureHost; }
 
   virtual void Destroy() override;
 
   uint64_t GetSerial() const { return mSerial; }
 
-  virtual mozilla::ipc::IPCResult RecvDestroySync() override {
-    DestroyIfNeeded();
-    return IPC_OK();
-  }
-
   HostIPCAllocator* mSurfaceAllocator;
   RefPtr<TextureHost> mTextureHost;
   // mSerial is unique in TextureClient's process.
   const uint64_t mSerial;
 };
 
 ////////////////////////////////////////////////////////////////////////////////
 PTextureParent*
--- a/gfx/layers/ipc/CompositableForwarder.h
+++ b/gfx/layers/ipc/CompositableForwarder.h
@@ -67,17 +67,17 @@ public:
    * Communicate to the compositor that aRegion in the texture identified by
    * aCompositable and aIdentifier has been updated to aThebesBuffer.
    */
   virtual void UpdateTextureRegion(CompositableClient* aCompositable,
                                    const ThebesBufferData& aThebesBufferData,
                                    const nsIntRegion& aUpdatedRegion) = 0;
 
   virtual void ReleaseCompositable(const CompositableHandle& aHandle) = 0;
-  virtual bool DestroyInTransaction(PTextureChild* aTexture, bool synchronously) = 0;
+  virtual bool DestroyInTransaction(PTextureChild* aTexture) = 0;
 
   /**
    * Tell the CompositableHost on the compositor side to remove the texture
    * from the CompositableHost.
    * This function does not delete the TextureHost corresponding to the
    * TextureClient passed in parameter.
    * When the TextureClient has TEXTURE_DEALLOCATE_CLIENT flag,
    * the transaction becomes synchronous.
--- a/gfx/layers/ipc/ImageBridgeChild.cpp
+++ b/gfx/layers/ipc/ImageBridgeChild.cpp
@@ -1060,41 +1060,36 @@ ImageBridgeChild::CreateTexture(const Su
                                 TextureFlags aFlags,
                                 uint64_t aSerial)
 {
   MOZ_ASSERT(CanSend());
   return SendPTextureConstructor(aSharedData, aLayersBackend, aFlags, aSerial);
 }
 
 static bool
-IBCAddOpDestroy(CompositableTransaction* aTxn, const OpDestroy& op, bool synchronously)
+IBCAddOpDestroy(CompositableTransaction* aTxn, const OpDestroy& op)
 {
   if (aTxn->Finished()) {
     return false;
   }
 
   aTxn->mDestroyedActors.AppendElement(op);
-
-  if (synchronously) {
-    aTxn->MarkSyncTransaction();
-  }
-
   return true;
 }
 
 bool
-ImageBridgeChild::DestroyInTransaction(PTextureChild* aTexture, bool synchronously)
+ImageBridgeChild::DestroyInTransaction(PTextureChild* aTexture)
 {
-  return IBCAddOpDestroy(mTxn, OpDestroy(aTexture), synchronously);
+  return IBCAddOpDestroy(mTxn, OpDestroy(aTexture));
 }
 
 bool
 ImageBridgeChild::DestroyInTransaction(const CompositableHandle& aHandle)
 {
-  return IBCAddOpDestroy(mTxn, OpDestroy(aHandle), false);
+  return IBCAddOpDestroy(mTxn, OpDestroy(aHandle));
 }
 
 void
 ImageBridgeChild::RemoveTextureFromCompositable(CompositableClient* aCompositable,
                                                 TextureClient* aTexture)
 {
   MOZ_ASSERT(CanSend());
   MOZ_ASSERT(aTexture);
--- a/gfx/layers/ipc/ImageBridgeChild.h
+++ b/gfx/layers/ipc/ImageBridgeChild.h
@@ -277,17 +277,17 @@ public:
   /**
    * Notify id of Texture When host side end its use. Transaction id is used to
    * make sure if there is no newer usage.
    */
   void NotifyNotUsed(uint64_t aTextureId, uint64_t aFwdTransactionId);
 
   virtual void CancelWaitForRecycle(uint64_t aTextureId) override;
 
-  virtual bool DestroyInTransaction(PTextureChild* aTexture, bool synchronously) override;
+  virtual bool DestroyInTransaction(PTextureChild* aTexture) override;
   bool DestroyInTransaction(const CompositableHandle& aHandle);
 
   virtual void RemoveTextureFromCompositable(CompositableClient* aCompositable,
                                              TextureClient* aTexture) override;
 
   virtual void UseTiledLayerBuffer(CompositableClient* aCompositable,
                                    const SurfaceDescriptorTiles& aTileLayerDescriptor) override
   {
--- a/gfx/layers/ipc/PTexture.ipdl
+++ b/gfx/layers/ipc/PTexture.ipdl
@@ -29,18 +29,13 @@ child:
     async __delete__();
 
 parent:
     /**
      * Asynchronously tell the compositor side to remove the texture.
      */
     async Destroy();
 
-    /**
-     * Synchronously tell the compositor side to remove the texture.
-     */
-    sync DestroySync();
-
     async RecycleTexture(TextureFlags aTextureFlags);
 };
 
 } // layers
 } // mozilla
--- a/gfx/layers/ipc/ShadowLayers.cpp
+++ b/gfx/layers/ipc/ShadowLayers.cpp
@@ -478,40 +478,36 @@ ShadowLayerForwarder::UseComponentAlphaT
         nullptr, aTextureOnBlack->GetIPDLActor(),
         nullptr, aTextureOnWhite->GetIPDLActor(),
         readLockB, readLockW)
       )
     );
 }
 
 static bool
-AddOpDestroy(Transaction* aTxn, const OpDestroy& op, bool synchronously)
+AddOpDestroy(Transaction* aTxn, const OpDestroy& op)
 {
   if (!aTxn->Opened()) {
     return false;
   }
 
   aTxn->mDestroyedActors.AppendElement(op);
-  if (synchronously) {
-    aTxn->MarkSyncTransaction();
-  }
-
   return true;
 }
 
 bool
-ShadowLayerForwarder::DestroyInTransaction(PTextureChild* aTexture, bool synchronously)
+ShadowLayerForwarder::DestroyInTransaction(PTextureChild* aTexture)
 {
-  return AddOpDestroy(mTxn, OpDestroy(aTexture), synchronously);
+  return AddOpDestroy(mTxn, OpDestroy(aTexture));
 }
 
 bool
 ShadowLayerForwarder::DestroyInTransaction(const CompositableHandle& aHandle)
 {
-  return AddOpDestroy(mTxn, OpDestroy(aHandle), false);
+  return AddOpDestroy(mTxn, OpDestroy(aHandle));
 }
 
 void
 ShadowLayerForwarder::RemoveTextureFromCompositable(CompositableClient* aCompositable,
                                                     TextureClient* aTexture)
 {
   MOZ_ASSERT(aCompositable);
   MOZ_ASSERT(aTexture);
--- a/gfx/layers/ipc/ShadowLayers.h
+++ b/gfx/layers/ipc/ShadowLayers.h
@@ -247,17 +247,17 @@ public:
 
   /**
    * See CompositableForwarder::UseTiledLayerBuffer
    */
   void UseTiledLayerBuffer(CompositableClient* aCompositable,
                                    const SurfaceDescriptorTiles& aTileLayerDescriptor) override;
 
   void ReleaseCompositable(const CompositableHandle& aHandle) override;
-  bool DestroyInTransaction(PTextureChild* aTexture, bool synchronously) override;
+  bool DestroyInTransaction(PTextureChild* aTexture) override;
   bool DestroyInTransaction(const CompositableHandle& aHandle);
 
   virtual void RemoveTextureFromCompositable(CompositableClient* aCompositable,
                                              TextureClient* aTexture) override;
 
   /**
    * Communicate to the compositor that aRegion in the texture identified by aLayer
    * and aIdentifier has been updated to aThebesBuffer.
--- a/gfx/layers/wr/WebRenderBridgeChild.cpp
+++ b/gfx/layers/wr/WebRenderBridgeChild.cpp
@@ -171,48 +171,45 @@ void
 WebRenderBridgeChild::UpdateTextureRegion(CompositableClient* aCompositable,
                                           const ThebesBufferData& aThebesBufferData,
                                           const nsIntRegion& aUpdatedRegion)
 {
 
 }
 
 bool
-WebRenderBridgeChild::AddOpDestroy(const OpDestroy& aOp, bool aSynchronously)
+WebRenderBridgeChild::AddOpDestroy(const OpDestroy& aOp)
 {
   if (!mIsInTransaction) {
     return false;
   }
 
   mDestroyedActors.AppendElement(aOp);
-  if (aSynchronously) {
-    MarkSyncTransaction();
-  }
   return true;
 }
 
 void
 WebRenderBridgeChild::ReleaseCompositable(const CompositableHandle& aHandle)
 {
   if (!DestroyInTransaction(aHandle)) {
     SendReleaseCompositable(aHandle);
   }
   mCompositables.Remove(aHandle.Value());
 }
 
 bool
-WebRenderBridgeChild::DestroyInTransaction(PTextureChild* aTexture, bool aSynchronously)
+WebRenderBridgeChild::DestroyInTransaction(PTextureChild* aTexture)
 {
-  return AddOpDestroy(OpDestroy(aTexture), aSynchronously);
+  return AddOpDestroy(OpDestroy(aTexture));
 }
 
 bool
 WebRenderBridgeChild::DestroyInTransaction(const CompositableHandle& aHandle)
 {
-  return AddOpDestroy(OpDestroy(aHandle), false);
+  return AddOpDestroy(OpDestroy(aHandle));
 }
 
 void
 WebRenderBridgeChild::RemoveTextureFromCompositable(CompositableClient* aCompositable,
                                                     TextureClient* aTexture)
 {
   MOZ_ASSERT(aCompositable);
   MOZ_ASSERT(aTexture);
--- a/gfx/layers/wr/WebRenderBridgeChild.h
+++ b/gfx/layers/wr/WebRenderBridgeChild.h
@@ -69,17 +69,17 @@ private:
   void Connect(CompositableClient* aCompositable,
                ImageContainer* aImageContainer = nullptr) override;
   void UseTiledLayerBuffer(CompositableClient* aCompositable,
                            const SurfaceDescriptorTiles& aTiledDescriptor) override;
   void UpdateTextureRegion(CompositableClient* aCompositable,
                            const ThebesBufferData& aThebesBufferData,
                            const nsIntRegion& aUpdatedRegion) override;
   void ReleaseCompositable(const CompositableHandle& aHandle) override;
-  bool DestroyInTransaction(PTextureChild* aTexture, bool aSynchronously) override;
+  bool DestroyInTransaction(PTextureChild* aTexture) override;
   bool DestroyInTransaction(const CompositableHandle& aHandle);
   void RemoveTextureFromCompositable(CompositableClient* aCompositable,
                                      TextureClient* aTexture) override;
   void UseTextures(CompositableClient* aCompositable,
                    const nsTArray<TimedTextureClient>& aTextures) override;
   void UseComponentAlphaTextures(CompositableClient* aCompositable,
                                  TextureClient* aClientOnBlack,
                                  TextureClient* aClientOnWhite) override;
@@ -95,17 +95,17 @@ private:
     AddRef();
   }
   void ReleaseIPDLReference() {
     MOZ_ASSERT(mIPCOpen == true);
     mIPCOpen = false;
     Release();
   }
 
-  bool AddOpDestroy(const OpDestroy& aOp, bool aSynchronously);
+  bool AddOpDestroy(const OpDestroy& aOp);
 
   nsTArray<WebRenderCommand> mCommands;
   nsTArray<OpDestroy> mDestroyedActors;
   nsDataHashtable<nsUint64HashKey, CompositableClient*> mCompositables;
   bool mIsInTransaction;
   bool mSyncTransaction;
 
   bool mIPCOpen;