Bug 1328797 - Part 1: Don't use a sync transaction for RemoveTexture. r=nical
--- 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;