Metal: Handle failure to allocate an MTLCommandBuffer
This requires restructuring the logic around MTLCommandBuffer allocation
so that GetPendingCommandContext is guaranteed to never fail. Logic in
the Metal backend is now similar to the Vulkan backend: the
MTLCommandBuffer is prepared at device initialization time, or after a
submission, such that it is always valid.
A new mUsed boolean is added to CommandRecordingContext to say whether
any commands have been recording. Previously mCommandBuffer was used for
that purpose, but it is now always non-null.
Bug: dawn:801
Change-Id: I5dc6747d1e6d538054010cc50533a03a49af921a
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/58720
Auto-Submit: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Stephen White <senorblanco@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
diff --git a/src/common/NonCopyable.h b/src/common/NonCopyable.h
index e711f71..61f15ca 100644
--- a/src/common/NonCopyable.h
+++ b/src/common/NonCopyable.h
@@ -15,10 +15,7 @@
#ifndef COMMON_NONCOPYABLE_H_
#define COMMON_NONCOPYABLE_H_
-// NonCopyable:
-// the base class for the classes that are not copyable.
-//
-
+// A base class to make a class non-copyable.
class NonCopyable {
protected:
constexpr NonCopyable() = default;
@@ -29,4 +26,15 @@
void operator=(const NonCopyable&) = delete;
};
+// A base class to make a class non-movable.
+class NonMovable : NonCopyable {
+ protected:
+ constexpr NonMovable() = default;
+ ~NonMovable() = default;
+
+ private:
+ NonMovable(NonMovable&&) = delete;
+ void operator=(NonMovable&&) = delete;
+};
+
#endif
diff --git a/src/dawn_native/metal/CommandRecordingContext.h b/src/dawn_native/metal/CommandRecordingContext.h
index a0047a8..5189a53 100644
--- a/src/dawn_native/metal/CommandRecordingContext.h
+++ b/src/dawn_native/metal/CommandRecordingContext.h
@@ -15,6 +15,8 @@
#define DAWNNATIVE_METAL_COMMANDRECORDINGCONTEXT_H_
#include "common/NSRef.h"
+#include "common/NonCopyable.h"
+#include "dawn_native/Error.h"
#import <Metal/Metal.h>
@@ -22,21 +24,16 @@
// This class wraps a MTLCommandBuffer and tracks which Metal encoder is open.
// Only one encoder may be open at a time.
- class CommandRecordingContext {
+ class CommandRecordingContext : NonMovable {
public:
CommandRecordingContext();
- CommandRecordingContext(NSPRef<id<MTLCommandBuffer>> commands);
-
- CommandRecordingContext(const CommandRecordingContext& rhs) = delete;
- CommandRecordingContext& operator=(const CommandRecordingContext& rhs) = delete;
-
- CommandRecordingContext(CommandRecordingContext&& rhs);
- CommandRecordingContext& operator=(CommandRecordingContext&& rhs);
-
~CommandRecordingContext();
id<MTLCommandBuffer> GetCommands();
+ void MarkUsed();
+ bool WasUsed() const;
+ MaybeError PrepareNextCommandBuffer(id<MTLCommandQueue> queue);
NSPRef<id<MTLCommandBuffer>> AcquireCommands();
id<MTLBlitCommandEncoder> EnsureBlit();
@@ -54,6 +51,7 @@
NSPRef<id<MTLComputeCommandEncoder>> mCompute;
NSPRef<id<MTLRenderCommandEncoder>> mRender;
bool mInEncoder = false;
+ bool mUsed = false;
};
}} // namespace dawn_native::metal
diff --git a/src/dawn_native/metal/CommandRecordingContext.mm b/src/dawn_native/metal/CommandRecordingContext.mm
index decb650..f07c48c 100644
--- a/src/dawn_native/metal/CommandRecordingContext.mm
+++ b/src/dawn_native/metal/CommandRecordingContext.mm
@@ -20,19 +20,6 @@
CommandRecordingContext::CommandRecordingContext() = default;
- CommandRecordingContext::CommandRecordingContext(NSPRef<id<MTLCommandBuffer>> commands)
- : mCommands(std::move(commands)) {
- }
-
- CommandRecordingContext::CommandRecordingContext(CommandRecordingContext&& rhs)
- : mCommands(rhs.AcquireCommands()) {
- }
-
- CommandRecordingContext& CommandRecordingContext::operator=(CommandRecordingContext&& rhs) {
- mCommands = rhs.AcquireCommands();
- return *this;
- }
-
CommandRecordingContext::~CommandRecordingContext() {
// Commands must be acquired.
ASSERT(mCommands == nullptr);
@@ -42,6 +29,28 @@
return mCommands.Get();
}
+ void CommandRecordingContext::MarkUsed() {
+ mUsed = true;
+ }
+ bool CommandRecordingContext::WasUsed() const {
+ return mUsed;
+ }
+
+ MaybeError CommandRecordingContext::PrepareNextCommandBuffer(id<MTLCommandQueue> queue) {
+ ASSERT(mCommands == nil);
+ ASSERT(!mUsed);
+
+ // The MTLCommandBuffer will be autoreleased by default.
+ // The autorelease pool may drain before the command buffer is submitted. Retain so it stays
+ // alive.
+ mCommands = AcquireNSPRef([[queue commandBuffer] retain]);
+ if (mCommands == nil) {
+ return DAWN_INTERNAL_ERROR("Failed to allocate an MTLCommandBuffer");
+ }
+
+ return {};
+ }
+
NSPRef<id<MTLCommandBuffer>> CommandRecordingContext::AcquireCommands() {
// A blit encoder can be left open from WriteBuffer, make sure we close it.
if (mCommands != nullptr) {
@@ -49,6 +58,7 @@
}
ASSERT(!mInEncoder);
+ mUsed = false;
return std::move(mCommands);
}
diff --git a/src/dawn_native/metal/DeviceMTL.h b/src/dawn_native/metal/DeviceMTL.h
index 4427418..d1881e5 100644
--- a/src/dawn_native/metal/DeviceMTL.h
+++ b/src/dawn_native/metal/DeviceMTL.h
@@ -51,7 +51,7 @@
id<MTLCommandQueue> GetMTLQueue();
CommandRecordingContext* GetPendingCommandContext();
- void SubmitPendingCommandBuffer();
+ MaybeError SubmitPendingCommandBuffer();
Ref<Texture> CreateTextureWrappingIOSurface(const ExternalImageDescriptor* descriptor,
IOSurfaceRef ioSurface,
diff --git a/src/dawn_native/metal/DeviceMTL.mm b/src/dawn_native/metal/DeviceMTL.mm
index 0359698..359dfda 100644
--- a/src/dawn_native/metal/DeviceMTL.mm
+++ b/src/dawn_native/metal/DeviceMTL.mm
@@ -128,6 +128,11 @@
InitTogglesFromDriver();
mCommandQueue.Acquire([*mMtlDevice newCommandQueue]);
+ if (mCommandQueue == nil) {
+ return DAWN_INTERNAL_ERROR("Failed to allocate MTLCommandQueue.");
+ }
+
+ DAWN_TRY(mCommandContext.PrepareNextCommandBuffer(*mCommandQueue));
if (GetAdapter()->GetSupportedExtensions().IsEnabled(Extension::TimestampQuery)) {
// Make a best guess of timestamp period based on device vendor info, and converge it to
@@ -281,9 +286,7 @@
}
MaybeError Device::TickImpl() {
- if (mCommandContext.GetCommands() != nullptr) {
- SubmitPendingCommandBuffer();
- }
+ DAWN_TRY(SubmitPendingCommandBuffer());
// Just run timestamp period calculation when timestamp extension is enabled.
if (IsExtensionEnabled(Extension::TimestampQuery)) {
@@ -305,20 +308,13 @@
}
CommandRecordingContext* Device::GetPendingCommandContext() {
- if (mCommandContext.GetCommands() == nullptr) {
- TRACE_EVENT0(GetPlatform(), General, "[MTLCommandQueue commandBuffer]");
- // The MTLCommandBuffer will be autoreleased by default.
- // The autorelease pool may drain before the command buffer is submitted. Retain so it
- // stays alive.
- mCommandContext =
- CommandRecordingContext(AcquireNSPRef([[*mCommandQueue commandBuffer] retain]));
- }
+ mCommandContext.MarkUsed();
return &mCommandContext;
}
- void Device::SubmitPendingCommandBuffer() {
- if (mCommandContext.GetCommands() == nullptr) {
- return;
+ MaybeError Device::SubmitPendingCommandBuffer() {
+ if (!mCommandContext.WasUsed()) {
+ return {};
}
IncrementLastSubmittedCommandSerial();
@@ -359,6 +355,8 @@
TRACE_EVENT_ASYNC_BEGIN0(GetPlatform(), GPUWork, "DeviceMTL::SubmitPendingCommandBuffer",
uint64_t(pendingSerial));
[*pendingCommands commit];
+
+ return mCommandContext.PrepareNextCommandBuffer(*mCommandQueue);
}
ResultOrError<std::unique_ptr<StagingBufferBase>> Device::CreateStagingBuffer(size_t size) {
@@ -432,7 +430,9 @@
}
void Device::WaitForCommandsToBeScheduled() {
- SubmitPendingCommandBuffer();
+ if (ConsumedError(SubmitPendingCommandBuffer())) {
+ return;
+ }
// Only lock the object while we take a reference to it, otherwise we could block further
// progress if the driver calls the scheduled handler (which also acquires the lock) before
diff --git a/src/dawn_native/metal/QueueMTL.mm b/src/dawn_native/metal/QueueMTL.mm
index bfa33ac..ad1fad6 100644
--- a/src/dawn_native/metal/QueueMTL.mm
+++ b/src/dawn_native/metal/QueueMTL.mm
@@ -42,8 +42,7 @@
}
TRACE_EVENT_END0(GetDevice()->GetPlatform(), Recording, "CommandBufferMTL::FillCommands");
- device->SubmitPendingCommandBuffer();
- return {};
+ return device->SubmitPendingCommandBuffer();
}
}} // namespace dawn_native::metal
diff --git a/src/tests/white_box/MetalAutoreleasePoolTests.mm b/src/tests/white_box/MetalAutoreleasePoolTests.mm
index 4763239..1202307 100644
--- a/src/tests/white_box/MetalAutoreleasePoolTests.mm
+++ b/src/tests/white_box/MetalAutoreleasePoolTests.mm
@@ -41,7 +41,7 @@
}
// Submitting the command buffer should succeed.
- mMtlDevice->SubmitPendingCommandBuffer();
+ ASSERT_TRUE(mMtlDevice->SubmitPendingCommandBuffer().IsSuccess());
}
// Test that the MTLBlitCommandEncoder owned by the pending command context
@@ -56,7 +56,7 @@
// Submitting the command buffer should succeed.
mMtlDevice->GetPendingCommandContext()->EndBlit();
- mMtlDevice->SubmitPendingCommandBuffer();
+ ASSERT_TRUE(mMtlDevice->SubmitPendingCommandBuffer().IsSuccess());
}
DAWN_INSTANTIATE_TEST(MetalAutoreleasePoolTests, MetalBackend());
