From 4ad35865b03eefb435a2839ef2917696d1a58b34 Mon Sep 17 00:00:00 2001 From: Brandon Jones Date: Thu, 15 Oct 2020 16:21:03 +0000 Subject: [PATCH] Change Device::Tick To Return Bool Changes Device::Tick to return a boolean that denotes whether or not Tick needs to be called again. Bug: dawn:119 Change-Id: I9d4c7e291536d676b33fc61d652667c1fbff8c62 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/29980 Reviewed-by: Austin Eng Reviewed-by: Corentin Wallez Commit-Queue: Brandon Jones --- src/dawn_native/DawnNative.cpp | 5 ++++ src/dawn_native/Device.cpp | 37 +++++++++++++++++---------- src/dawn_native/Device.h | 29 +++++++++++---------- src/dawn_native/ErrorScopeTracker.cpp | 2 +- src/dawn_native/Queue.cpp | 12 ++++++--- src/include/dawn_native/DawnNative.h | 2 ++ 6 files changed, 57 insertions(+), 30 deletions(-) diff --git a/src/dawn_native/DawnNative.cpp b/src/dawn_native/DawnNative.cpp index 5627ff07f3..c3db0fd88f 100644 --- a/src/dawn_native/DawnNative.cpp +++ b/src/dawn_native/DawnNative.cpp @@ -196,6 +196,11 @@ namespace dawn_native { return GetProcMapNamesForTestingInternal(); } + DAWN_NATIVE_EXPORT bool DeviceTick(WGPUDevice device) { + dawn_native::DeviceBase* deviceBase = reinterpret_cast(device); + return deviceBase->Tick(); + } + // ExternalImageDescriptor ExternalImageDescriptor::ExternalImageDescriptor(ExternalImageType type) : type(type) { diff --git a/src/dawn_native/Device.cpp b/src/dawn_native/Device.cpp index 41725fd80f..b6dbed6dc3 100644 --- a/src/dawn_native/Device.cpp +++ b/src/dawn_native/Device.cpp @@ -139,7 +139,7 @@ namespace dawn_native { break; } ASSERT(mCompletedSerial == mLastSubmittedSerial); - ASSERT(mFutureCallbackSerial <= mCompletedSerial); + ASSERT(mFutureSerial <= mCompletedSerial); // Skip handling device facilities if they haven't even been created (or failed doing so) if (mState != State::BeingCreated) { @@ -189,7 +189,7 @@ namespace dawn_native { IgnoreErrors(WaitForIdleForDestruction()); IgnoreErrors(TickImpl()); AssumeCommandsComplete(); - ASSERT(mFutureCallbackSerial <= mCompletedSerial); + ASSERT(mFutureSerial <= mCompletedSerial); mState = State::Disconnected; // Now everything is as if the device was lost. @@ -319,8 +319,8 @@ namespace dawn_native { return mLastSubmittedSerial; } - ExecutionSerial DeviceBase::GetFutureCallbackSerial() const { - return mFutureCallbackSerial; + ExecutionSerial DeviceBase::GetFutureSerial() const { + return mFutureSerial; } void DeviceBase::IncrementLastSubmittedCommandSerial() { @@ -328,19 +328,27 @@ namespace dawn_native { } void DeviceBase::AssumeCommandsComplete() { - ExecutionSerial maxSerial = ExecutionSerial( - std::max(mLastSubmittedSerial + ExecutionSerial(1), mFutureCallbackSerial)); + ExecutionSerial maxSerial = + ExecutionSerial(std::max(mLastSubmittedSerial + ExecutionSerial(1), mFutureSerial)); mLastSubmittedSerial = maxSerial; mCompletedSerial = maxSerial; } + bool DeviceBase::IsDeviceIdle() { + ExecutionSerial maxSerial = std::max(mLastSubmittedSerial, mFutureSerial); + if (mCompletedSerial == maxSerial) { + return true; + } + return false; + } + ExecutionSerial DeviceBase::GetPendingCommandSerial() const { return mLastSubmittedSerial + ExecutionSerial(1); } - void DeviceBase::AddFutureCallbackSerial(ExecutionSerial serial) { - if (serial > mFutureCallbackSerial) { - mFutureCallbackSerial = serial; + void DeviceBase::AddFutureSerial(ExecutionSerial serial) { + if (serial > mFutureSerial) { + mFutureSerial = serial; } } @@ -713,18 +721,19 @@ namespace dawn_native { // Other Device API methods - void DeviceBase::Tick() { + // Returns true if future ticking is needed. + bool DeviceBase::Tick() { if (ConsumedError(ValidateIsAlive())) { - return; + return false; } // to avoid overly ticking, we only want to tick when: // 1. the last submitted serial has moved beyond the completed serial // 2. or the completed serial has not reached the future serial set by the trackers - if (mLastSubmittedSerial > mCompletedSerial || mCompletedSerial < mFutureCallbackSerial) { + if (mLastSubmittedSerial > mCompletedSerial || mCompletedSerial < mFutureSerial) { CheckPassedSerials(); if (ConsumedError(TickImpl())) { - return; + return false; } // There is no GPU work in flight, we need to move the serials forward so that @@ -742,6 +751,8 @@ namespace dawn_native { mErrorScopeTracker->Tick(mCompletedSerial); GetDefaultQueue()->Tick(mCompletedSerial); } + + return !IsDeviceIdle(); } void DeviceBase::Reference() { diff --git a/src/dawn_native/Device.h b/src/dawn_native/Device.h index a2f9c4baf2..bd76deea88 100644 --- a/src/dawn_native/Device.h +++ b/src/dawn_native/Device.h @@ -86,7 +86,7 @@ namespace dawn_native { ExecutionSerial GetCompletedCommandSerial() const; ExecutionSerial GetLastSubmittedCommandSerial() const; - ExecutionSerial GetFutureCallbackSerial() const; + ExecutionSerial GetFutureSerial() const; ExecutionSerial GetPendingCommandSerial() const; virtual MaybeError TickImpl() = 0; @@ -161,7 +161,7 @@ namespace dawn_native { QueueBase* GetDefaultQueue(); void InjectError(wgpu::ErrorType type, const char* message); - void Tick(); + bool Tick(); void SetDeviceLostCallback(wgpu::DeviceLostCallback callback, void* userdata); void SetUncapturedErrorCallback(wgpu::ErrorCallback callback, void* userdata); @@ -221,14 +221,15 @@ namespace dawn_native { size_t GetDeprecationWarningCountForTesting(); void EmitDeprecationWarning(const char* warning); void LoseForTesting(); - // AddFutureCallbackSerial is used to update the mFutureCallbackSerial with the max - // serial needed to be ticked in order to clean up all pending callback work. It should be - // given the serial that a callback is tracked with, so that once that serial is completed, - // it can be resolved and cleaned up. This is so that when there is no gpu work (the last - // submitted serial has not moved beyond the completed serial), Tick can still check if we - // have pending callback work to take care of, rather than hanging and never reaching the - // serial the callbacks are enqueued on. - void AddFutureCallbackSerial(ExecutionSerial serial); + + // AddFutureSerial is used to update the mFutureSerial with the max serial needed to be + // ticked in order to clean up all pending callback work or to execute asynchronous resource + // writes. It should be given the serial that a callback is tracked with, so that once that + // serial is completed, it can be resolved and cleaned up. This is so that when there is no + // gpu work (the last submitted serial has not moved beyond the completed serial), Tick can + // still check if we have pending work to take care of, rather than hanging and never + // reaching the serial the work will be executed on. + void AddFutureSerial(ExecutionSerial serial); virtual uint32_t GetOptimalBytesPerRowAlignment() const = 0; virtual uint64_t GetOptimalBufferToTextureCopyOffsetAlignment() const = 0; @@ -320,17 +321,19 @@ namespace dawn_native { // and waiting on a serial that doesn't have a corresponding fence enqueued. Fake serials to // make all commands look completed. void AssumeCommandsComplete(); + bool IsDeviceIdle(); + // mCompletedSerial tracks the last completed command serial that the fence has returned. // mLastSubmittedSerial tracks the last submitted command serial. // During device removal, the serials could be artificially incremented // to make it appear as if commands have been compeleted. They can also be artificially // incremented when no work is being done in the GPU so CPU operations don't have to wait on // stale serials. - // mFutureCallbackSerial tracks the largest serial we need to tick to for the callbacks to - // fire + // mFutureSerial tracks the largest serial we need to tick to for asynchronous commands or + // callbacks to fire ExecutionSerial mCompletedSerial = ExecutionSerial(0); ExecutionSerial mLastSubmittedSerial = ExecutionSerial(0); - ExecutionSerial mFutureCallbackSerial = ExecutionSerial(0); + ExecutionSerial mFutureSerial = ExecutionSerial(0); // ShutDownImpl is used to clean up and release resources used by device, does not wait for // GPU or check errors. diff --git a/src/dawn_native/ErrorScopeTracker.cpp b/src/dawn_native/ErrorScopeTracker.cpp index f950e37f88..8f1f657e7d 100644 --- a/src/dawn_native/ErrorScopeTracker.cpp +++ b/src/dawn_native/ErrorScopeTracker.cpp @@ -36,7 +36,7 @@ namespace dawn_native { void ErrorScopeTracker::TrackUntilLastSubmitComplete(ErrorScope* scope) { mScopesInFlight.Enqueue(scope, mDevice->GetLastSubmittedCommandSerial()); - mDevice->AddFutureCallbackSerial(mDevice->GetPendingCommandSerial()); + mDevice->AddFutureSerial(mDevice->GetPendingCommandSerial()); } void ErrorScopeTracker::Tick(ExecutionSerial completedSerial) { diff --git a/src/dawn_native/Queue.cpp b/src/dawn_native/Queue.cpp index ed6cbab4cd..519cf93383 100644 --- a/src/dawn_native/Queue.cpp +++ b/src/dawn_native/Queue.cpp @@ -179,7 +179,7 @@ namespace dawn_native { void QueueBase::TrackTask(std::unique_ptr task, ExecutionSerial serial) { mTasksInFlight.Enqueue(std::move(task), serial); - GetDevice()->AddFutureCallbackSerial(serial); + GetDevice()->AddFutureSerial(serial); } void QueueBase::Tick(ExecutionSerial finishedSerial) { @@ -234,6 +234,8 @@ namespace dawn_native { memcpy(uploadHandle.mappedBuffer, data, size); + device->AddFutureSerial(device->GetPendingCommandSerial()); + return device->CopyFromStagingToBuffer(uploadHandle.stagingBuffer, uploadHandle.startOffset, buffer, bufferOffset, size); } @@ -297,8 +299,12 @@ namespace dawn_native { textureCopy.origin = destination.origin; textureCopy.aspect = ConvertAspect(destination.texture->GetFormat(), destination.aspect); - return GetDevice()->CopyFromStagingToTexture(uploadHandle.stagingBuffer, passDataLayout, - &textureCopy, writeSizePixel); + DeviceBase* device = GetDevice(); + + device->AddFutureSerial(device->GetPendingCommandSerial()); + + return device->CopyFromStagingToTexture(uploadHandle.stagingBuffer, passDataLayout, + &textureCopy, writeSizePixel); } MaybeError QueueBase::ValidateSubmit(uint32_t commandCount, CommandBufferBase* const* commands) const { diff --git a/src/include/dawn_native/DawnNative.h b/src/include/dawn_native/DawnNative.h index 6498646e49..22a038047f 100644 --- a/src/include/dawn_native/DawnNative.h +++ b/src/include/dawn_native/DawnNative.h @@ -193,6 +193,8 @@ namespace dawn_native { // Backdoor to get the order of the ProcMap for testing DAWN_NATIVE_EXPORT std::vector GetProcMapNamesForTesting(); + DAWN_NATIVE_EXPORT bool DeviceTick(WGPUDevice device); + // ErrorInjector functions used for testing only. Defined in dawn_native/ErrorInjector.cpp DAWN_NATIVE_EXPORT void EnableErrorInjector(); DAWN_NATIVE_EXPORT void DisableErrorInjector();