From be990077f4906a63a257ab0804a36e5549a27412 Mon Sep 17 00:00:00 2001 From: Austin Eng Date: Tue, 17 Sep 2019 18:24:07 +0000 Subject: [PATCH] Support ErrorScopes for asynchronous GPU execution This changes updates ErrorScopes so that scopes enclosing a Queue::Submit or Queue::Signal resolve their callbacks asynchronously after GPU execution is complete. Bug: dawn:153 Change-Id: I0e0b8a9f19f3f29d1b6a3683938154b87f190a07 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/10701 Reviewed-by: Kai Ninomiya Reviewed-by: Corentin Wallez Commit-Queue: Austin Eng --- BUILD.gn | 3 +- src/dawn_native/Device.cpp | 12 ++++ src/dawn_native/Device.h | 4 ++ src/dawn_native/ErrorScope.cpp | 7 ++ src/dawn_native/ErrorScope.h | 2 + src/dawn_native/ErrorScopeTracker.cpp | 44 +++++++++++++ src/dawn_native/ErrorScopeTracker.h | 42 ++++++++++++ src/dawn_native/Queue.cpp | 17 +++-- .../validation/ErrorScopeValidationTests.cpp | 65 +++++++++++++++++++ 9 files changed, 190 insertions(+), 6 deletions(-) create mode 100644 src/dawn_native/ErrorScopeTracker.cpp create mode 100644 src/dawn_native/ErrorScopeTracker.h diff --git a/BUILD.gn b/BUILD.gn index e096cd3f31..d5364a1f7f 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -168,6 +168,8 @@ source_set("libdawn_native_sources") { "src/dawn_native/ErrorData.h", "src/dawn_native/ErrorScope.cpp", "src/dawn_native/ErrorScope.h", + "src/dawn_native/ErrorScopeTracker.cpp", + "src/dawn_native/ErrorScopeTracker.h", "src/dawn_native/Extensions.cpp", "src/dawn_native/Extensions.h", "src/dawn_native/Fence.cpp", @@ -595,7 +597,6 @@ if (is_win || (is_linux && !is_chromeos) || is_mac) { # Allow inclusion of include_dirs = [ "${dawn_glfw_dir}/include" ] - # The GLFW/glfw3.h header includes by default, but the latter # does not exist on Fuchsia. Defining GLFW_INCLUDE_NONE helps work around # the issue, but it needs to be defined for any file that includes the diff --git a/src/dawn_native/Device.cpp b/src/dawn_native/Device.cpp index 434391136c..fa450631f5 100644 --- a/src/dawn_native/Device.cpp +++ b/src/dawn_native/Device.cpp @@ -25,6 +25,7 @@ #include "dawn_native/DynamicUploader.h" #include "dawn_native/ErrorData.h" #include "dawn_native/ErrorScope.h" +#include "dawn_native/ErrorScopeTracker.h" #include "dawn_native/Fence.h" #include "dawn_native/FenceSignalTracker.h" #include "dawn_native/Instance.h" @@ -67,6 +68,7 @@ namespace dawn_native { mRootErrorScope(AcquireRef(new ErrorScope())), mCurrentErrorScope(mRootErrorScope.Get()) { mCaches = std::make_unique(); + mErrorScopeTracker = std::make_unique(this); mFenceSignalTracker = std::make_unique(this); mDynamicUploader = std::make_unique(this); SetDefaultToggles(); @@ -121,6 +123,11 @@ namespace dawn_native { return true; } + ErrorScope* DeviceBase::GetCurrentErrorScope() { + ASSERT(mCurrentErrorScope.Get() != nullptr); + return mCurrentErrorScope.Get(); + } + MaybeError DeviceBase::ValidateObject(const ObjectBase* object) const { if (DAWN_UNLIKELY(object->GetDevice() != this)) { return DAWN_VALIDATION_ERROR("Object from a different device."); @@ -139,6 +146,10 @@ namespace dawn_native { return GetAdapter()->GetInstance()->GetPlatform(); } + ErrorScopeTracker* DeviceBase::GetErrorScopeTracker() const { + return mErrorScopeTracker.get(); + } + FenceSignalTracker* DeviceBase::GetFenceSignalTracker() const { return mFenceSignalTracker.get(); } @@ -519,6 +530,7 @@ namespace dawn_native { deferred.callback(deferred.status, deferred.result, deferred.userdata); } } + mErrorScopeTracker->Tick(GetCompletedCommandSerial()); mFenceSignalTracker->Tick(GetCompletedCommandSerial()); } diff --git a/src/dawn_native/Device.h b/src/dawn_native/Device.h index fe52fd2748..1c3354f4c7 100644 --- a/src/dawn_native/Device.h +++ b/src/dawn_native/Device.h @@ -36,6 +36,7 @@ namespace dawn_native { class AttachmentState; class AttachmentStateBlueprint; class ErrorScope; + class ErrorScopeTracker; class FenceSignalTracker; class DynamicUploader; class StagingBufferBase; @@ -61,6 +62,7 @@ namespace dawn_native { AdapterBase* GetAdapter() const; dawn_platform::Platform* GetPlatform() const; + ErrorScopeTracker* GetErrorScopeTracker() const; FenceSignalTracker* GetFenceSignalTracker() const; // Returns the Format corresponding to the dawn::TextureFormat or an error if the format @@ -153,6 +155,7 @@ namespace dawn_native { void SetUncapturedErrorCallback(dawn::ErrorCallback callback, void* userdata); void PushErrorScope(dawn::ErrorFilter filter); bool PopErrorScope(dawn::ErrorCallback callback, void* userdata); + ErrorScope* GetCurrentErrorScope(); void Reference(); void Release(); @@ -251,6 +254,7 @@ namespace dawn_native { void* userdata; }; + std::unique_ptr mErrorScopeTracker; std::unique_ptr mFenceSignalTracker; std::vector mDeferredCreateBufferMappedAsyncResults; diff --git a/src/dawn_native/ErrorScope.cpp b/src/dawn_native/ErrorScope.cpp index 0b53216a62..812ab69433 100644 --- a/src/dawn_native/ErrorScope.cpp +++ b/src/dawn_native/ErrorScope.cpp @@ -111,4 +111,11 @@ namespace dawn_native { } } + void ErrorScope::Destroy() { + if (!IsRoot()) { + mErrorType = dawn::ErrorType::Unknown; + mErrorMessage = "Error scope destroyed"; + } + } + } // namespace dawn_native diff --git a/src/dawn_native/ErrorScope.h b/src/dawn_native/ErrorScope.h index 4fd57c9892..9bafcf9474 100644 --- a/src/dawn_native/ErrorScope.h +++ b/src/dawn_native/ErrorScope.h @@ -49,6 +49,8 @@ namespace dawn_native { void HandleError(dawn::ErrorType type, const char* message); void HandleError(ErrorData* error); + void Destroy(); + private: bool IsRoot() const; static void HandleErrorImpl(ErrorScope* scope, dawn::ErrorType type, const char* message); diff --git a/src/dawn_native/ErrorScopeTracker.cpp b/src/dawn_native/ErrorScopeTracker.cpp new file mode 100644 index 0000000000..e0a21aa4c8 --- /dev/null +++ b/src/dawn_native/ErrorScopeTracker.cpp @@ -0,0 +1,44 @@ +// Copyright 2019 The Dawn Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "dawn_native/ErrorScopeTracker.h" + +#include "dawn_native/Device.h" +#include "dawn_native/ErrorScope.h" + +namespace dawn_native { + + ErrorScopeTracker::ErrorScopeTracker(DeviceBase* device) : mDevice(device) { + } + + ErrorScopeTracker::~ErrorScopeTracker() { + // The tracker is destroyed when the Device is destroyed. We need to + // call Destroy on all in-flight error scopes so they resolve their callbacks + // with UNKNOWN. + constexpr Serial maxSerial = std::numeric_limits::max(); + for (Ref& scope : mScopesInFlight.IterateUpTo(maxSerial)) { + scope->Destroy(); + } + Tick(maxSerial); + } + + void ErrorScopeTracker::TrackUntilLastSubmitComplete(ErrorScope* scope) { + mScopesInFlight.Enqueue(scope, mDevice->GetLastSubmittedCommandSerial()); + } + + void ErrorScopeTracker::Tick(Serial completedSerial) { + mScopesInFlight.ClearUpTo(completedSerial); + } + +} // namespace dawn_native diff --git a/src/dawn_native/ErrorScopeTracker.h b/src/dawn_native/ErrorScopeTracker.h new file mode 100644 index 0000000000..7337eb6414 --- /dev/null +++ b/src/dawn_native/ErrorScopeTracker.h @@ -0,0 +1,42 @@ +// Copyright 2019 The Dawn Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef DAWNNATIVE_ERRORSCOPETRACKER_H_ +#define DAWNNATIVE_ERRORSCOPETRACKER_H_ + +#include "common/SerialQueue.h" +#include "dawn_native/RefCounted.h" + +namespace dawn_native { + + class DeviceBase; + class ErrorScope; + + class ErrorScopeTracker { + public: + ErrorScopeTracker(DeviceBase* device); + ~ErrorScopeTracker(); + + void TrackUntilLastSubmitComplete(ErrorScope* scope); + + void Tick(Serial completedSerial); + + protected: + DeviceBase* mDevice; + SerialQueue> mScopesInFlight; + }; + +} // namespace dawn_native + +#endif // DAWNNATIVE_ERRORSCOPETRACKER_H_ diff --git a/src/dawn_native/Queue.cpp b/src/dawn_native/Queue.cpp index 46ead11636..d3b2455aee 100644 --- a/src/dawn_native/Queue.cpp +++ b/src/dawn_native/Queue.cpp @@ -17,6 +17,8 @@ #include "dawn_native/Buffer.h" #include "dawn_native/CommandBuffer.h" #include "dawn_native/Device.h" +#include "dawn_native/ErrorScope.h" +#include "dawn_native/ErrorScopeTracker.h" #include "dawn_native/Fence.h" #include "dawn_native/FenceSignalTracker.h" #include "dawn_native/Texture.h" @@ -30,24 +32,29 @@ namespace dawn_native { } void QueueBase::Submit(uint32_t commandCount, CommandBufferBase* const* commands) { - TRACE_EVENT0(GetDevice()->GetPlatform(), TRACE_DISABLED_BY_DEFAULT("gpu.dawn"), - "Queue::Submit"); - if (GetDevice()->ConsumedError(ValidateSubmit(commandCount, commands))) { + DeviceBase* device = GetDevice(); + TRACE_EVENT0(device->GetPlatform(), TRACE_DISABLED_BY_DEFAULT("gpu.dawn"), "Queue::Submit"); + if (device->ConsumedError(ValidateSubmit(commandCount, commands))) { return; } ASSERT(!IsError()); SubmitImpl(commandCount, commands); + device->GetErrorScopeTracker()->TrackUntilLastSubmitComplete( + device->GetCurrentErrorScope()); } void QueueBase::Signal(FenceBase* fence, uint64_t signalValue) { - if (GetDevice()->ConsumedError(ValidateSignal(fence, signalValue))) { + DeviceBase* device = GetDevice(); + if (device->ConsumedError(ValidateSignal(fence, signalValue))) { return; } ASSERT(!IsError()); fence->SetSignaledValue(signalValue); - GetDevice()->GetFenceSignalTracker()->UpdateFenceOnComplete(fence, signalValue); + device->GetFenceSignalTracker()->UpdateFenceOnComplete(fence, signalValue); + device->GetErrorScopeTracker()->TrackUntilLastSubmitComplete( + device->GetCurrentErrorScope()); } FenceBase* QueueBase::CreateFence(const FenceDescriptor* descriptor) { diff --git a/src/tests/unittests/validation/ErrorScopeValidationTests.cpp b/src/tests/unittests/validation/ErrorScopeValidationTests.cpp index 4d9629eb5b..8d2c749234 100644 --- a/src/tests/unittests/validation/ErrorScopeValidationTests.cpp +++ b/src/tests/unittests/validation/ErrorScopeValidationTests.cpp @@ -132,3 +132,68 @@ TEST_F(ErrorScopeValidationTest, PushPopBalanced) { EXPECT_FALSE(device.PopErrorScope(ToMockDevicePopErrorScopeCallback, this + 2)); } } + +// Test that error scopes do not call their callbacks until after an enclosed Queue::Submit +// completes +TEST_F(ErrorScopeValidationTest, CallbackAfterQueueSubmit) { + dawn::Queue queue = device.CreateQueue(); + + device.PushErrorScope(dawn::ErrorFilter::OutOfMemory); + queue.Submit(0, nullptr); + device.PopErrorScope(ToMockDevicePopErrorScopeCallback, this); + + EXPECT_CALL(*mockDevicePopErrorScopeCallback, Call(DAWN_ERROR_TYPE_NO_ERROR, _, this)).Times(1); + + // Side effects of Queue::Submit only are seen after Tick() + device.Tick(); +} + +// Test that parent error scopes do not call their callbacks until after an enclosed Queue::Submit +// completes +TEST_F(ErrorScopeValidationTest, CallbackAfterQueueSubmitNested) { + dawn::Queue queue = device.CreateQueue(); + + device.PushErrorScope(dawn::ErrorFilter::OutOfMemory); + device.PushErrorScope(dawn::ErrorFilter::OutOfMemory); + queue.Submit(0, nullptr); + device.PopErrorScope(ToMockDevicePopErrorScopeCallback, this); + device.PopErrorScope(ToMockDevicePopErrorScopeCallback, this + 1); + + EXPECT_CALL(*mockDevicePopErrorScopeCallback, Call(DAWN_ERROR_TYPE_NO_ERROR, _, this)).Times(1); + EXPECT_CALL(*mockDevicePopErrorScopeCallback, Call(DAWN_ERROR_TYPE_NO_ERROR, _, this + 1)) + .Times(1); + + // Side effects of Queue::Submit only are seen after Tick() + device.Tick(); +} + +// Test a callback that returns asynchronously followed by a synchronous one +TEST_F(ErrorScopeValidationTest, AsynchronousThenSynchronous) { + dawn::Queue queue = device.CreateQueue(); + + device.PushErrorScope(dawn::ErrorFilter::OutOfMemory); + queue.Submit(0, nullptr); + device.PopErrorScope(ToMockDevicePopErrorScopeCallback, this); + + EXPECT_CALL(*mockDevicePopErrorScopeCallback, Call(DAWN_ERROR_TYPE_NO_ERROR, _, this + 1)) + .Times(1); + device.PushErrorScope(dawn::ErrorFilter::OutOfMemory); + device.PopErrorScope(ToMockDevicePopErrorScopeCallback, this + 1); + + EXPECT_CALL(*mockDevicePopErrorScopeCallback, Call(DAWN_ERROR_TYPE_NO_ERROR, _, this)).Times(1); + + // Side effects of Queue::Submit only are seen after Tick() + device.Tick(); +} + +// Test that if the device is destroyed before the callback occurs, it is called with UNKNOWN. +TEST_F(ErrorScopeValidationTest, DeviceDestroyedBeforeCallback) { + dawn::Queue queue = device.CreateQueue(); + + device.PushErrorScope(dawn::ErrorFilter::OutOfMemory); + queue.Submit(0, nullptr); + device.PopErrorScope(ToMockDevicePopErrorScopeCallback, this); + + EXPECT_CALL(*mockDevicePopErrorScopeCallback, Call(DAWN_ERROR_TYPE_UNKNOWN, _, this)).Times(1); + device = nullptr; +}