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 343210 82c34cf618e8a3ba422e35f13d5041b0544893da
parent 343209 2e78a0da72687a894a87dcb742d8cdd78c85c21c
child 343211 6e42452c2616386464dbdc1cdd08deafe31387e4
push id31372
push usercbook@mozilla.com
push dateThu, 16 Feb 2017 12:16:10 +0000
treeherdermozilla-central@2737f66ad6ac [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnical
bugs1328797
milestone54.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 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;