Have Queue timeline tasks resolve in order Use QueueBase to track fences in flight and map requests so that they can be resolved in the order they were added. Before these tasks were separately tracked in FenceSignalTracker and MapRequestTracker, so tasks would be resolving out of order. Bug: dawn:404 Change-Id: I8b58fb72c99f43bc4593f56e08920d48ac506157 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/29441 Reviewed-by: Austin Eng <enga@chromium.org> Commit-Queue: Natasha Lee <natlee@microsoft.com>
diff --git a/src/dawn_native/BUILD.gn b/src/dawn_native/BUILD.gn index 0cfe3a7..5387494 100644 --- a/src/dawn_native/BUILD.gn +++ b/src/dawn_native/BUILD.gn
@@ -205,16 +205,12 @@ "Extensions.h", "Fence.cpp", "Fence.h", - "FenceSignalTracker.cpp", - "FenceSignalTracker.h", "Format.cpp", "Format.h", "Forward.h", "Instance.cpp", "Instance.h", "IntegerTypes.h", - "MapRequestTracker.cpp", - "MapRequestTracker.h", "ObjectBase.cpp", "ObjectBase.h", "PassResourceUsage.h",
diff --git a/src/dawn_native/Buffer.cpp b/src/dawn_native/Buffer.cpp index 519b0e7..b4605dc 100644 --- a/src/dawn_native/Buffer.cpp +++ b/src/dawn_native/Buffer.cpp
@@ -19,7 +19,6 @@ #include "dawn_native/Device.h" #include "dawn_native/DynamicUploader.h" #include "dawn_native/ErrorData.h" -#include "dawn_native/MapRequestTracker.h" #include "dawn_native/Queue.h" #include "dawn_native/ValidationUtils_autogen.h" @@ -30,6 +29,19 @@ namespace dawn_native { namespace { + struct MapRequestTask : QueueBase::TaskInFlight { + MapRequestTask(Ref<BufferBase> buffer, MapRequestID id) + : buffer(std::move(buffer)), id(id) { + } + void Finish() override { + buffer->OnMapRequestCompleted(id); + } + ~MapRequestTask() override = default; + + private: + Ref<BufferBase> buffer; + MapRequestID id; + }; class ErrorBuffer final : public BufferBase { public: @@ -261,9 +273,10 @@ CallMapCallback(mLastMapID, WGPUBufferMapAsyncStatus_DeviceLost); return; } - - MapRequestTracker* tracker = GetDevice()->GetMapRequestTracker(); - tracker->Track(this, mLastMapID); + std::unique_ptr<MapRequestTask> request = + std::make_unique<MapRequestTask>(this, mLastMapID); + GetDevice()->GetDefaultQueue()->TrackTask(std::move(request), + GetDevice()->GetPendingCommandSerial()); } void* BufferBase::GetMappedRange(size_t offset, size_t size) {
diff --git a/src/dawn_native/CMakeLists.txt b/src/dawn_native/CMakeLists.txt index 7aba5b1..ba0c94f 100644 --- a/src/dawn_native/CMakeLists.txt +++ b/src/dawn_native/CMakeLists.txt
@@ -88,16 +88,12 @@ "Extensions.h" "Fence.cpp" "Fence.h" - "FenceSignalTracker.cpp" - "FenceSignalTracker.h" "Format.cpp" "Format.h" "Forward.h" "Instance.cpp" "Instance.h" "IntegerTypes.h" - "MapRequestTracker.cpp" - "MapRequestTracker.h" "ObjectBase.cpp" "ObjectBase.h" "PassResourceUsage.h"
diff --git a/src/dawn_native/Device.cpp b/src/dawn_native/Device.cpp index 39e88df..41725fd 100644 --- a/src/dawn_native/Device.cpp +++ b/src/dawn_native/Device.cpp
@@ -28,9 +28,7 @@ #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" -#include "dawn_native/MapRequestTracker.h" #include "dawn_native/PipelineLayout.h" #include "dawn_native/QuerySet.h" #include "dawn_native/Queue.h" @@ -103,8 +101,6 @@ mCaches = std::make_unique<DeviceBase::Caches>(); mErrorScopeTracker = std::make_unique<ErrorScopeTracker>(this); - mFenceSignalTracker = std::make_unique<FenceSignalTracker>(this); - mMapRequestTracker = std::make_unique<MapRequestTracker>(this); mDynamicUploader = std::make_unique<DynamicUploader>(this); mDeprecationWarnings = std::make_unique<DeprecationWarnings>(); @@ -152,8 +148,7 @@ // freed by backends in the ShutDownImpl() call. Still tick the ones that might have // pending callbacks. mErrorScopeTracker->Tick(GetCompletedCommandSerial()); - mFenceSignalTracker->Tick(GetCompletedCommandSerial()); - mMapRequestTracker->Tick(GetCompletedCommandSerial()); + GetDefaultQueue()->Tick(GetCompletedCommandSerial()); // call TickImpl once last time to clean up resources // Ignore errors so that we can continue with destruction IgnoreErrors(TickImpl()); @@ -167,9 +162,7 @@ mCurrentErrorScope->UnlinkForShutdown(); } mErrorScopeTracker = nullptr; - mFenceSignalTracker = nullptr; mDynamicUploader = nullptr; - mMapRequestTracker = nullptr; mEmptyBindGroupLayout = nullptr; @@ -318,14 +311,6 @@ return mErrorScopeTracker.get(); } - FenceSignalTracker* DeviceBase::GetFenceSignalTracker() const { - return mFenceSignalTracker.get(); - } - - MapRequestTracker* DeviceBase::GetMapRequestTracker() const { - return mMapRequestTracker.get(); - } - ExecutionSerial DeviceBase::GetCompletedCommandSerial() const { return mCompletedSerial; } @@ -755,8 +740,7 @@ // reclaiming resources one tick earlier. mDynamicUploader->Deallocate(mCompletedSerial); mErrorScopeTracker->Tick(mCompletedSerial); - mFenceSignalTracker->Tick(mCompletedSerial); - mMapRequestTracker->Tick(mCompletedSerial); + GetDefaultQueue()->Tick(mCompletedSerial); } }
diff --git a/src/dawn_native/Device.h b/src/dawn_native/Device.h index 97d0b77..a2f9c4b 100644 --- a/src/dawn_native/Device.h +++ b/src/dawn_native/Device.h
@@ -36,8 +36,6 @@ class DynamicUploader; class ErrorScope; class ErrorScopeTracker; - class FenceSignalTracker; - class MapRequestTracker; class StagingBufferBase; class DeviceBase { @@ -71,8 +69,6 @@ dawn_platform::Platform* GetPlatform() const; ErrorScopeTracker* GetErrorScopeTracker() const; - FenceSignalTracker* GetFenceSignalTracker() const; - MapRequestTracker* GetMapRequestTracker() const; // Returns the Format corresponding to the wgpu::TextureFormat or an error if the format // isn't a valid wgpu::TextureFormat or isn't supported by this device. @@ -225,6 +221,13 @@ 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); virtual uint32_t GetOptimalBytesPerRowAlignment() const = 0; @@ -356,8 +359,6 @@ std::unique_ptr<DynamicUploader> mDynamicUploader; std::unique_ptr<ErrorScopeTracker> mErrorScopeTracker; - std::unique_ptr<FenceSignalTracker> mFenceSignalTracker; - std::unique_ptr<MapRequestTracker> mMapRequestTracker; Ref<QueueBase> mDefaultQueue; struct DeprecationWarnings;
diff --git a/src/dawn_native/Fence.cpp b/src/dawn_native/Fence.cpp index 5c2a64f..c1ce24b 100644 --- a/src/dawn_native/Fence.cpp +++ b/src/dawn_native/Fence.cpp
@@ -23,6 +23,20 @@ namespace dawn_native { + struct FenceInFlight : QueueBase::TaskInFlight { + FenceInFlight(Ref<Fence> fence, FenceAPISerial value) + : fence(std::move(fence)), value(value) { + } + void Finish() override { + fence->SetCompletedValue(value); + } + ~FenceInFlight() override = default; + + private: + Ref<Fence> fence; + FenceAPISerial value; + }; + MaybeError ValidateFenceDescriptor(const FenceDescriptor* descriptor) { if (descriptor->nextInChain != nullptr) { return DAWN_VALIDATION_ERROR("nextInChain must be nullptr"); @@ -118,6 +132,15 @@ mRequests.ClearUpTo(mCompletedValue); } + void Fence::UpdateFenceOnComplete(Fence* fence, FenceAPISerial value) { + std::unique_ptr<FenceInFlight> fenceInFlight = + std::make_unique<FenceInFlight>(fence, value); + + // TODO: use GetLastSubmittedCommandSerial in the future for perforamnce + GetDevice()->GetDefaultQueue()->TrackTask(std::move(fenceInFlight), + GetDevice()->GetPendingCommandSerial()); + } + MaybeError Fence::ValidateOnCompletion(FenceAPISerial value, WGPUFenceCompletionStatus* status) const { *status = WGPUFenceCompletionStatus_DeviceLost;
diff --git a/src/dawn_native/Fence.h b/src/dawn_native/Fence.h index f5f4be6..6c1cbcb 100644 --- a/src/dawn_native/Fence.h +++ b/src/dawn_native/Fence.h
@@ -44,9 +44,10 @@ protected: friend class QueueBase; - friend class FenceSignalTracker; + friend struct FenceInFlight; void SetSignaledValue(FenceAPISerial signalValue); void SetCompletedValue(FenceAPISerial completedValue); + void UpdateFenceOnComplete(Fence* fence, FenceAPISerial value); private: Fence(DeviceBase* device, ObjectBase::ErrorTag tag);
diff --git a/src/dawn_native/FenceSignalTracker.cpp b/src/dawn_native/FenceSignalTracker.cpp deleted file mode 100644 index c29a9c6..0000000 --- a/src/dawn_native/FenceSignalTracker.cpp +++ /dev/null
@@ -1,44 +0,0 @@ -// Copyright 2018 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/FenceSignalTracker.h" - -#include "dawn_native/Device.h" -#include "dawn_native/Fence.h" - -namespace dawn_native { - - FenceSignalTracker::FenceSignalTracker(DeviceBase* device) : mDevice(device) { - } - - FenceSignalTracker::~FenceSignalTracker() { - ASSERT(mFencesInFlight.Empty()); - } - - void FenceSignalTracker::UpdateFenceOnComplete(Fence* fence, FenceAPISerial value) { - // Because we currently only have a single queue, we can simply update - // the fence completed value once the last submitted serial has passed. - mFencesInFlight.Enqueue(FenceInFlight{fence, value}, - mDevice->GetLastSubmittedCommandSerial()); - mDevice->AddFutureCallbackSerial(mDevice->GetPendingCommandSerial()); - } - - void FenceSignalTracker::Tick(ExecutionSerial finishedSerial) { - for (auto& fenceInFlight : mFencesInFlight.IterateUpTo(finishedSerial)) { - fenceInFlight.fence->SetCompletedValue(fenceInFlight.value); - } - mFencesInFlight.ClearUpTo(finishedSerial); - } - -} // namespace dawn_native
diff --git a/src/dawn_native/FenceSignalTracker.h b/src/dawn_native/FenceSignalTracker.h deleted file mode 100644 index 6136156..0000000 --- a/src/dawn_native/FenceSignalTracker.h +++ /dev/null
@@ -1,48 +0,0 @@ -// Copyright 2018 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_FENCESIGNALTRACKER_H_ -#define DAWNNATIVE_FENCESIGNALTRACKER_H_ - -#include "common/RefCounted.h" -#include "common/SerialQueue.h" -#include "dawn_native/IntegerTypes.h" - -namespace dawn_native { - - class DeviceBase; - class Fence; - - class FenceSignalTracker { - struct FenceInFlight { - Ref<Fence> fence; - FenceAPISerial value; - }; - - public: - FenceSignalTracker(DeviceBase* device); - ~FenceSignalTracker(); - - void UpdateFenceOnComplete(Fence* fence, FenceAPISerial value); - - void Tick(ExecutionSerial finishedSerial); - - private: - DeviceBase* mDevice; - SerialQueue<ExecutionSerial, FenceInFlight> mFencesInFlight; - }; - -} // namespace dawn_native - -#endif // DAWNNATIVE_FENCESIGNALTRACKER_H_
diff --git a/src/dawn_native/MapRequestTracker.cpp b/src/dawn_native/MapRequestTracker.cpp deleted file mode 100644 index 999657d..0000000 --- a/src/dawn_native/MapRequestTracker.cpp +++ /dev/null
@@ -1,44 +0,0 @@ -// Copyright 2020 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/MapRequestTracker.h" - -#include "dawn_native/Buffer.h" -#include "dawn_native/Device.h" - -namespace dawn_native { - - MapRequestTracker::MapRequestTracker(DeviceBase* device) : mDevice(device) { - } - - MapRequestTracker::~MapRequestTracker() { - ASSERT(mInflightRequests.Empty()); - } - - void MapRequestTracker::Track(BufferBase* buffer, MapRequestID mapID) { - Request request; - request.buffer = buffer; - request.id = mapID; - - mInflightRequests.Enqueue(std::move(request), mDevice->GetPendingCommandSerial()); - mDevice->AddFutureCallbackSerial(mDevice->GetPendingCommandSerial()); - } - - void MapRequestTracker::Tick(ExecutionSerial finishedSerial) { - for (auto& request : mInflightRequests.IterateUpTo(finishedSerial)) { - request.buffer->OnMapRequestCompleted(request.id); - } - mInflightRequests.ClearUpTo(finishedSerial); - } -} // namespace dawn_native
diff --git a/src/dawn_native/MapRequestTracker.h b/src/dawn_native/MapRequestTracker.h deleted file mode 100644 index 08dda23..0000000 --- a/src/dawn_native/MapRequestTracker.h +++ /dev/null
@@ -1,45 +0,0 @@ -// Copyright 2020 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_MAPREQUESTTRACKER_H_ -#define DAWNNATIVE_MAPREQUESTTRACKER_H_ - -#include "common/SerialQueue.h" -#include "dawn_native/Buffer.h" -#include "dawn_native/Forward.h" -#include "dawn_native/IntegerTypes.h" - -namespace dawn_native { - - class MapRequestTracker { - public: - MapRequestTracker(DeviceBase* device); - ~MapRequestTracker(); - - void Track(BufferBase* buffer, MapRequestID mapID); - void Tick(ExecutionSerial finishedSerial); - - private: - DeviceBase* mDevice; - - struct Request { - Ref<BufferBase> buffer; - MapRequestID id; - }; - SerialQueue<ExecutionSerial, Request> mInflightRequests; - }; - -} // namespace dawn_native - -#endif // DAWNNATIVE_MAPREQUESTTRACKER_H
diff --git a/src/dawn_native/Queue.cpp b/src/dawn_native/Queue.cpp index c655139..82a37d1 100644 --- a/src/dawn_native/Queue.cpp +++ b/src/dawn_native/Queue.cpp
@@ -24,7 +24,6 @@ #include "dawn_native/ErrorScope.h" #include "dawn_native/ErrorScopeTracker.h" #include "dawn_native/Fence.h" -#include "dawn_native/FenceSignalTracker.h" #include "dawn_native/QuerySet.h" #include "dawn_native/Texture.h" #include "dawn_platform/DawnPlatform.h" @@ -34,6 +33,7 @@ namespace dawn_native { namespace { + void CopyTextureData(uint8_t* dstPointer, const uint8_t* srcPointer, uint32_t depth, @@ -136,12 +136,19 @@ // QueueBase + QueueBase::TaskInFlight::~TaskInFlight() { + } + QueueBase::QueueBase(DeviceBase* device) : ObjectBase(device) { } QueueBase::QueueBase(DeviceBase* device, ObjectBase::ErrorTag tag) : ObjectBase(device, tag) { } + QueueBase::~QueueBase() { + ASSERT(mTasksInFlight.Empty()); + } + // static QueueBase* QueueBase::MakeError(DeviceBase* device) { return new ErrorQueue(device); @@ -165,11 +172,23 @@ ASSERT(!IsError()); fence->SetSignaledValue(signalValue); - device->GetFenceSignalTracker()->UpdateFenceOnComplete(fence, signalValue); + fence->UpdateFenceOnComplete(fence, signalValue); device->GetErrorScopeTracker()->TrackUntilLastSubmitComplete( device->GetCurrentErrorScope()); } + void QueueBase::TrackTask(std::unique_ptr<TaskInFlight> task, ExecutionSerial serial) { + mTasksInFlight.Enqueue(std::move(task), serial); + GetDevice()->AddFutureCallbackSerial(serial); + } + + void QueueBase::Tick(ExecutionSerial finishedSerial) { + for (auto& task : mTasksInFlight.IterateUpTo(finishedSerial)) { + task->Finish(); + } + mTasksInFlight.ClearUpTo(finishedSerial); + } + Fence* QueueBase::CreateFence(const FenceDescriptor* descriptor) { if (GetDevice()->ConsumedError(ValidateCreateFence(descriptor))) { return Fence::MakeError(GetDevice());
diff --git a/src/dawn_native/Queue.h b/src/dawn_native/Queue.h index cf1d384..9e0d6c2 100644 --- a/src/dawn_native/Queue.h +++ b/src/dawn_native/Queue.h
@@ -15,6 +15,7 @@ #ifndef DAWNNATIVE_QUEUE_H_ #define DAWNNATIVE_QUEUE_H_ +#include "common/SerialQueue.h" #include "dawn_native/Error.h" #include "dawn_native/Forward.h" #include "dawn_native/IntegerTypes.h" @@ -26,7 +27,13 @@ class QueueBase : public ObjectBase { public: + struct TaskInFlight { + virtual ~TaskInFlight(); + virtual void Finish() = 0; + }; + static QueueBase* MakeError(DeviceBase* device); + ~QueueBase() override; // Dawn API void Submit(uint32_t commandCount, CommandBufferBase* const* commands); @@ -39,6 +46,9 @@ const TextureDataLayout* dataLayout, const Extent3D* writeSize); + void TrackTask(std::unique_ptr<TaskInFlight> task, ExecutionSerial serial); + void Tick(ExecutionSerial finishedSerial); + protected: QueueBase(DeviceBase* device); QueueBase(DeviceBase* device, ObjectBase::ErrorTag tag); @@ -77,6 +87,8 @@ const Extent3D* writeSize) const; void SubmitInternal(uint32_t commandCount, CommandBufferBase* const* commands); + + SerialQueue<ExecutionSerial, std::unique_ptr<TaskInFlight>> mTasksInFlight; }; } // namespace dawn_native