commit 43ae770183e0d8922ba5b6d317e841a8b58aa246
parent baecbcec1afd9a692858ec84e3991b4e97655a51
Author: Lee Salzman <lsalzman@mozilla.com>
Date: Wed, 26 Nov 2025 06:24:54 +0000
Bug 2002342 - Reduce CanvasTranslator data shmem copies. r=aosmond
CanvasChild often creates data shmem transfers for snapshots, which CanvasTranslator
immediately copies into a separate memory buffer as soon as it receives the shmem.
Like CanvasChild, we can keep alive these shmems on the CanvasTranslator side for
a reasonable lifetime to directly use the shmem's memory rather than copy from it,
avoiding the sometimes large costs of the copy.
Unlike CanvasChild, this limits the amount of shmems on the CanvasTranslator side
by a pref beyond which the shmems are still copied to a memory buffer as usual.
Differential Revision: https://phabricator.services.mozilla.com/D274031
Diffstat:
7 files changed, 184 insertions(+), 48 deletions(-)
diff --git a/gfx/layers/RecordedCanvasEventImpl.h b/gfx/layers/RecordedCanvasEventImpl.h
@@ -295,8 +295,10 @@ RecordedCacheDataSurface::RecordedCacheDataSurface(S& aStream)
class RecordedGetDataForSurface final
: public RecordedEventDerived<RecordedGetDataForSurface> {
public:
- explicit RecordedGetDataForSurface(const gfx::SourceSurface* aSurface)
- : RecordedEventDerived(GET_DATA_FOR_SURFACE), mSurface(aSurface) {}
+ RecordedGetDataForSurface(uint32_t aId, const gfx::SourceSurface* aSurface)
+ : RecordedEventDerived(GET_DATA_FOR_SURFACE),
+ mId(aId),
+ mSurface(aSurface) {}
template <class S>
MOZ_IMPLICIT RecordedGetDataForSurface(S& aStream);
@@ -309,23 +311,26 @@ class RecordedGetDataForSurface final
std::string GetName() const final { return "RecordedGetDataForSurface"; }
private:
+ uint32_t mId = 0;
ReferencePtr mSurface;
};
inline bool RecordedGetDataForSurface::PlayCanvasEvent(
CanvasTranslator* aTranslator) const {
- aTranslator->GetDataSurface(mSurface.mLongPtr);
+ aTranslator->GetDataSurface(mId, mSurface.mLongPtr);
return true;
}
template <class S>
void RecordedGetDataForSurface::Record(S& aStream) const {
+ WriteElement(aStream, mId);
WriteElement(aStream, mSurface);
}
template <class S>
RecordedGetDataForSurface::RecordedGetDataForSurface(S& aStream)
: RecordedEventDerived(GET_DATA_FOR_SURFACE) {
+ ReadElement(aStream, mId);
ReadElement(aStream, mSurface);
}
diff --git a/gfx/layers/ipc/CanvasChild.cpp b/gfx/layers/ipc/CanvasChild.cpp
@@ -552,7 +552,12 @@ bool CanvasChild::EnsureDataSurfaceShmem(size_t aSizeRequired) {
return false;
}
- if (!SendSetDataSurfaceBuffer(std::move(shmemHandle))) {
+ auto id = ++mNextDataSurfaceShmemId;
+ if (!id) {
+ // If ids overflow, ensure that zero is reserved.
+ id = ++mNextDataSurfaceShmemId;
+ }
+ if (!SendSetDataSurfaceBuffer(id, std::move(shmemHandle))) {
return false;
}
@@ -648,19 +653,13 @@ already_AddRefed<gfx::DataSourceSurface> CanvasChild::GetDataSurface(
return nullptr;
}
- // If growing the data surface shmem, allocation may require significant time
- // in the content process, during which the GPU process may issue an earlier
- // readback while the content process is still busy. If the existing data
- // surface shmem is to be reused instead, then try to instead read the data
- // directly into the shmem to avoid a superfluous copy after readback.
- bool forceData = ShouldGrowDataSurfaceShmem(sizeRequired);
- RecordEvent(RecordedCacheDataSurface(aSurface, forceData));
+ RecordEvent(RecordedCacheDataSurface(aSurface));
if (!EnsureDataSurfaceShmem(sizeRequired)) {
return nullptr;
}
- RecordEvent(RecordedGetDataForSurface(aSurface));
+ RecordEvent(RecordedGetDataForSurface(mNextDataSurfaceShmemId, aSurface));
auto checkpoint = CreateCheckpoint();
if (NS_WARN_IF(!mRecorder->WaitForCheckpoint(checkpoint))) {
return nullptr;
diff --git a/gfx/layers/ipc/CanvasChild.h b/gfx/layers/ipc/CanvasChild.h
@@ -197,6 +197,7 @@ class CanvasChild final : public PCanvasChild, public SupportsWeakPtr {
std::shared_ptr<ipc::ReadOnlySharedMemoryMapping> mDataSurfaceShmem;
bool mDataSurfaceShmemAvailable = false;
+ uint32_t mNextDataSurfaceShmemId = 0;
int64_t mLastWriteLockCheckpoint = 0;
uint32_t mTransactionsSinceGetDataSurface = kCacheDataSurfaceThreshold;
struct TextureInfo {
diff --git a/gfx/layers/ipc/CanvasTranslator.cpp b/gfx/layers/ipc/CanvasTranslator.cpp
@@ -284,7 +284,7 @@ bool CanvasTranslator::AddBuffer(
}
ipc::IPCResult CanvasTranslator::RecvSetDataSurfaceBuffer(
- ipc::MutableSharedMemoryHandle&& aBufferHandle) {
+ uint32_t aId, ipc::MutableSharedMemoryHandle&& aBufferHandle) {
if (mDeactivated) {
// The other side might have sent a resume message before we deactivated.
return IPC_OK();
@@ -293,29 +293,77 @@ ipc::IPCResult CanvasTranslator::RecvSetDataSurfaceBuffer(
if (UsePendingCanvasTranslatorEvents()) {
MutexAutoLock lock(mCanvasTranslatorEventsLock);
mPendingCanvasTranslatorEvents.push_back(
- CanvasTranslatorEvent::SetDataSurfaceBuffer(std::move(aBufferHandle)));
+ CanvasTranslatorEvent::SetDataSurfaceBuffer(aId,
+ std::move(aBufferHandle)));
PostCanvasTranslatorEvents(lock);
} else {
- DispatchToTaskQueue(NewRunnableMethod<ipc::MutableSharedMemoryHandle&&>(
- "CanvasTranslator::SetDataSurfaceBuffer", this,
- &CanvasTranslator::SetDataSurfaceBuffer, std::move(aBufferHandle)));
+ DispatchToTaskQueue(
+ NewRunnableMethod<uint32_t, ipc::MutableSharedMemoryHandle&&>(
+ "CanvasTranslator::SetDataSurfaceBuffer", this,
+ &CanvasTranslator::SetDataSurfaceBuffer, aId,
+ std::move(aBufferHandle)));
}
return IPC_OK();
}
-void CanvasTranslator::DataSurfaceBufferWillChange() {
- RefPtr<gfx::DataSourceSurface> owner(mDataSurfaceShmemOwner);
- if (owner) {
- // Force copy-on-write of contained shmem if applicable
- gfx::DataSourceSurface::ScopedMap map(
- owner, gfx::DataSourceSurface::MapType::READ_WRITE);
- mDataSurfaceShmemOwner = nullptr;
+void CanvasTranslator::DataSurfaceBufferWillChange(uint32_t aId,
+ bool aKeepAlive,
+ size_t aLimit) {
+ if (aId) {
+ // If a specific id is changing, then attempt to copy it.
+ auto it = mDataSurfaceShmems.find(aId);
+ if (it != mDataSurfaceShmems.end()) {
+ RefPtr<gfx::DataSourceSurface> owner(it->second.mOwner);
+ if (owner) {
+ // Force copy-on-write of contained shmem if applicable
+ gfx::DataSourceSurface::ScopedMap map(
+ owner, gfx::DataSourceSurface::MapType::READ_WRITE);
+ it->second.mOwner = nullptr;
+ }
+ // Erase the shmem if it's not the last used id.
+ if (!aKeepAlive || aId != mLastDataSurfaceShmemId) {
+ mDataSurfaceShmems.erase(it);
+ }
+ }
+ } else {
+ // If no limit is requested, clear everything.
+ if (!aLimit) {
+ aLimit = mDataSurfaceShmems.size();
+ }
+ // Ensure all shmems still alive are copied.
+ DataSurfaceShmem lastShmem;
+ auto it = mDataSurfaceShmems.begin();
+ for (; aLimit > 0 && it != mDataSurfaceShmems.end(); ++it, --aLimit) {
+ // If the last shmem id needs to be kept alive, move it before clearing
+ // the hashtable.
+ if (aKeepAlive && it->first == mLastDataSurfaceShmemId) {
+ lastShmem = std::move(it->second);
+ continue;
+ }
+ RefPtr<gfx::DataSourceSurface> owner(it->second.mOwner);
+ if (owner) {
+ // Force copy-on-write of contained shmem if applicable
+ gfx::DataSourceSurface::ScopedMap map(
+ owner, gfx::DataSourceSurface::MapType::READ_WRITE);
+ it->second.mOwner = nullptr;
+ }
+ }
+ // Clear all iterated shmem mappings.
+ if (it == mDataSurfaceShmems.end()) {
+ mDataSurfaceShmems.clear();
+ } else if (it != mDataSurfaceShmems.begin()) {
+ mDataSurfaceShmems.erase(mDataSurfaceShmems.begin(), it);
+ }
+ // Restore the last shmem id if necessary.
+ if (lastShmem.mShmem.IsValid()) {
+ mDataSurfaceShmems[mLastDataSurfaceShmemId] = std::move(lastShmem);
+ }
}
}
bool CanvasTranslator::SetDataSurfaceBuffer(
- ipc::MutableSharedMemoryHandle&& aBufferHandle) {
+ uint32_t aId, ipc::MutableSharedMemoryHandle&& aBufferHandle) {
MOZ_ASSERT(IsInTaskQueue());
if (mHeader->readerState == State::Failed) {
// We failed before we got to the pause event.
@@ -332,16 +380,53 @@ bool CanvasTranslator::SetDataSurfaceBuffer(
return false;
}
- DataSurfaceBufferWillChange();
- mDataSurfaceShmem = aBufferHandle.Map();
- if (!mDataSurfaceShmem) {
+ if (!aId) {
return false;
}
+ if (aId < mLastDataSurfaceShmemId) {
+ // The id space has overflowed, so clear out all existing mapping.
+ DataSurfaceBufferWillChange(0, false);
+ } else if (mLastDataSurfaceShmemId != aId) {
+ // The last shmem id will be kept alive, even if there is no owner. The last
+ // shmem id is about to change, so if the current last id has no owner, it
+ // needs to be erased.
+ auto it = mDataSurfaceShmems.find(mLastDataSurfaceShmemId);
+ if (it != mDataSurfaceShmems.end() && it->second.mOwner.IsDead()) {
+ mDataSurfaceShmems.erase(it);
+ }
+ // Ensure the current shmems don't exceed the maximum allowed.
+ size_t maxShmems = StaticPrefs::gfx_canvas_accelerated_max_data_shmems();
+ if (maxShmems > 0 && mDataSurfaceShmems.size() >= maxShmems) {
+ DataSurfaceBufferWillChange(0, false,
+ (mDataSurfaceShmems.size() - maxShmems) + 1);
+ }
+ }
+ mLastDataSurfaceShmemId = aId;
+
+ // If the id is being reused, then ensure the old contents is copied before
+ // the buffer is changed.
+ DataSurfaceBufferWillChange(aId);
+
+ // Finally, change the shmem mapping.
+ {
+ auto& dataSurfaceShmem = mDataSurfaceShmems[aId];
+ dataSurfaceShmem.mShmem = aBufferHandle.Map();
+ if (!dataSurfaceShmem.mShmem) {
+ // Try clearing out old mappings to see if resource limits were reached.
+ DataSurfaceBufferWillChange(0, false);
+ // Try mapping one last time.
+ dataSurfaceShmem.mShmem = aBufferHandle.Map();
+ if (!dataSurfaceShmem.mShmem) {
+ return false;
+ }
+ }
+ }
+
return TranslateRecording();
}
-void CanvasTranslator::GetDataSurface(uint64_t aSurfaceRef) {
+void CanvasTranslator::GetDataSurface(uint32_t aId, uint64_t aSurfaceRef) {
MOZ_ASSERT(IsInTaskQueue());
ReferencePtr surfaceRef = reinterpret_cast<void*>(aSurfaceRef);
@@ -362,17 +447,31 @@ void CanvasTranslator::GetDataSurface(uint64_t aSurfaceRef) {
ImageDataSerializer::ComputeRGBStride(format, dstSize.width);
auto requiredSize =
ImageDataSerializer::ComputeRGBBufferSize(dstSize, format);
- if (requiredSize <= 0 || size_t(requiredSize) > mDataSurfaceShmem.Size()) {
+ if (requiredSize <= 0) {
return;
}
// Ensure any old references to the shmem are copied before modification.
- DataSurfaceBufferWillChange();
+ DataSurfaceBufferWillChange(aId);
+
+ // Ensure a shmem exists with the given id.
+ auto it = mDataSurfaceShmems.find(aId);
+ if (it == mDataSurfaceShmems.end()) {
+ return;
+ }
// Try directly reading the data surface into shmem to avoid further copies.
- uint8_t* dst = mDataSurfaceShmem.DataAs<uint8_t>();
+ if (size_t(requiredSize) > it->second.mShmem.Size()) {
+ return;
+ }
+
+ uint8_t* dst = it->second.mShmem.DataAs<uint8_t>();
if (dataSurface->ReadDataInto(dst, dstStride)) {
- mDataSurfaceShmemOwner = dataSurface;
+ // If reading directly into the shmem, then mark the data surface as the
+ // shmem's owner.
+ it->second.mOwner = dataSurface;
+ dataSurface->AddUserData(&mDataSurfaceShmemIdKey,
+ reinterpret_cast<void*>(aId), nullptr);
return;
}
@@ -784,8 +883,8 @@ void CanvasTranslator::HandleCanvasTranslatorEvents() {
dispatchTranslate = AddBuffer(event->TakeBufferHandle());
break;
case CanvasTranslatorEvent::Tag::SetDataSurfaceBuffer:
- dispatchTranslate =
- SetDataSurfaceBuffer(event->TakeDataSurfaceBufferHandle());
+ dispatchTranslate = SetDataSurfaceBuffer(
+ event->mId, event->TakeDataSurfaceBufferHandle());
break;
case CanvasTranslatorEvent::Tag::ClearCachedResources:
ClearCachedResources();
@@ -1382,7 +1481,7 @@ bool CanvasTranslator::PushRemoteTexture(
void CanvasTranslator::ClearTextureInfo() {
MOZ_ASSERT(mIPDLClosed);
- DataSurfaceBufferWillChange();
+ DataSurfaceBufferWillChange(0, false);
mUsedDataSurfaceForSurfaceDescriptor = nullptr;
mUsedWrapperForSurfaceDescriptor = nullptr;
@@ -1751,7 +1850,20 @@ void CanvasTranslator::AddDataSurface(
}
void CanvasTranslator::RemoveDataSurface(gfx::ReferencePtr aRefPtr) {
- mDataSurfaces.Remove(aRefPtr);
+ RefPtr<gfx::DataSourceSurface> surface;
+ if (mDataSurfaces.Remove(aRefPtr, getter_AddRefs(surface))) {
+ // If the data surface is the assigned owner of a shmem, and if the shmem
+ // is not the last shmem id used, then erase the shmem.
+ if (auto id = reinterpret_cast<uintptr_t>(
+ surface->GetUserData(&mDataSurfaceShmemIdKey))) {
+ if (id != mLastDataSurfaceShmemId) {
+ auto it = mDataSurfaceShmems.find(id);
+ if (it != mDataSurfaceShmems.end()) {
+ mDataSurfaceShmems.erase(it);
+ }
+ }
+ }
+ }
}
} // namespace layers
diff --git a/gfx/layers/ipc/CanvasTranslator.h b/gfx/layers/ipc/CanvasTranslator.h
@@ -8,6 +8,7 @@
#define mozilla_layers_CanvasTranslator_h
#include <deque>
+#include <map>
#include <memory>
#include <unordered_map>
@@ -112,7 +113,7 @@ class CanvasTranslator final : public gfx::InlineTranslator,
* Sets the shared memory to be used for readback.
*/
ipc::IPCResult RecvSetDataSurfaceBuffer(
- ipc::MutableSharedMemoryHandle&& aBufferHandle);
+ uint32_t aId, ipc::MutableSharedMemoryHandle&& aBufferHandle);
ipc::IPCResult RecvClearCachedResources();
@@ -291,7 +292,7 @@ class CanvasTranslator final : public gfx::InlineTranslator,
void NextBuffer();
- void GetDataSurface(uint64_t aSurfaceRef);
+ void GetDataSurface(uint32_t aId, uint64_t aSurfaceRef);
/**
* Wait for a canvas to produce the designated surface. If necessary,
@@ -352,8 +353,9 @@ class CanvasTranslator final : public gfx::InlineTranslator,
MOZ_ASSERT(mTag == Tag::AddBuffer);
}
CanvasTranslatorEvent(const Tag aTag,
- ipc::MutableSharedMemoryHandle&& aBufferHandle)
- : mTag(aTag), mBufferHandle(std::move(aBufferHandle)) {
+ ipc::MutableSharedMemoryHandle&& aBufferHandle,
+ uint32_t aId = 0)
+ : mTag(aTag), mBufferHandle(std::move(aBufferHandle)), mId(aId) {
MOZ_ASSERT(mTag == Tag::SetDataSurfaceBuffer);
}
@@ -368,9 +370,9 @@ class CanvasTranslator final : public gfx::InlineTranslator,
}
static UniquePtr<CanvasTranslatorEvent> SetDataSurfaceBuffer(
- ipc::MutableSharedMemoryHandle&& aBufferHandle) {
+ uint32_t aId, ipc::MutableSharedMemoryHandle&& aBufferHandle) {
return MakeUnique<CanvasTranslatorEvent>(Tag::SetDataSurfaceBuffer,
- std::move(aBufferHandle));
+ std::move(aBufferHandle), aId);
}
static UniquePtr<CanvasTranslatorEvent> ClearCachedResources() {
@@ -396,6 +398,8 @@ class CanvasTranslator final : public gfx::InlineTranslator,
MOZ_ASSERT_UNREACHABLE("unexpected to be called");
return nullptr;
}
+
+ uint32_t mId = 0;
};
/*
@@ -408,9 +412,11 @@ class CanvasTranslator final : public gfx::InlineTranslator,
* @returns true if next HandleCanvasTranslatorEvents() needs to call
* TranslateRecording().
*/
- bool SetDataSurfaceBuffer(ipc::MutableSharedMemoryHandle&& aBufferHandle);
+ bool SetDataSurfaceBuffer(uint32_t aId,
+ ipc::MutableSharedMemoryHandle&& aBufferHandle);
- void DataSurfaceBufferWillChange();
+ void DataSurfaceBufferWillChange(uint32_t aId = 0, bool aKeepAlive = true,
+ size_t aLimit = 0);
bool ReadNextEvent(EventType& aEventType);
@@ -526,8 +532,14 @@ class CanvasTranslator final : public gfx::InlineTranslator,
std::queue<CanvasShmem> mCanvasShmems;
CanvasShmem mCurrentShmem;
gfx::MemReader mCurrentMemReader{0, 0};
- ipc::SharedMemoryMapping mDataSurfaceShmem;
- ThreadSafeWeakPtr<gfx::DataSourceSurface> mDataSurfaceShmemOwner;
+ // Track any data surfaces pointing to a shmem mapping.
+ struct DataSurfaceShmem {
+ ipc::SharedMemoryMapping mShmem;
+ ThreadSafeWeakPtr<gfx::DataSourceSurface> mOwner;
+ };
+ std::map<uint32_t, DataSurfaceShmem> mDataSurfaceShmems;
+ // The last shmem id that was assigned to a mapping.
+ uint32_t mLastDataSurfaceShmemId = 0;
UniquePtr<CrossProcessSemaphore> mWriterSemaphore;
UniquePtr<CrossProcessSemaphore> mReaderSemaphore;
TextureType mTextureType = TextureType::Unknown;
@@ -575,6 +587,8 @@ class CanvasTranslator final : public gfx::InlineTranslator,
std::deque<UniquePtr<CanvasTranslatorEvent>> mPendingCanvasTranslatorEvents;
std::unordered_map<gfx::ReferencePtr, ExportSurface> mExportSurfaces;
+
+ gfx::UserDataKey mDataSurfaceShmemIdKey = {0};
};
} // namespace layers
diff --git a/gfx/layers/ipc/PCanvas.ipdl b/gfx/layers/ipc/PCanvas.ipdl
@@ -55,7 +55,7 @@ parent:
/**
* Sets the shared memory to be used for readback.
*/
- async SetDataSurfaceBuffer(MutableSharedMemoryHandle aBufferHandle);
+ async SetDataSurfaceBuffer(uint32_t aId, MutableSharedMemoryHandle aBufferHandle);
/**
* Notify CanvasTranslator it is about to be minimized.
diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml
@@ -6734,6 +6734,11 @@
value: 128
mirror: always
+- name: gfx.canvas.accelerated.max-data-shmems
+ type: RelaxedAtomicUint32
+ value: 400
+ mirror: always
+
- name: gfx.canvas.accelerated.max-export-surfaces
type: RelaxedAtomicUint32
value: 100
