From 51af1b428f5f075091c421b282d489d526be4292 Mon Sep 17 00:00:00 2001 From: Natasha Lee Date: Mon, 12 Oct 2020 22:32:33 +0000 Subject: [PATCH] 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 Commit-Queue: Natasha Lee --- src/dawn_native/BUILD.gn | 4 - src/dawn_native/Buffer.cpp | 21 ++- src/dawn_native/CMakeLists.txt | 4 - src/dawn_native/Device.cpp | 20 +-- src/dawn_native/Device.h | 13 +- src/dawn_native/Fence.cpp | 23 +++ src/dawn_native/Fence.h | 3 +- src/dawn_native/FenceSignalTracker.cpp | 44 ----- src/dawn_native/FenceSignalTracker.h | 48 ----- src/dawn_native/MapRequestTracker.cpp | 44 ----- src/dawn_native/MapRequestTracker.h | 45 ----- src/dawn_native/Queue.cpp | 23 ++- src/dawn_native/Queue.h | 12 ++ src/tests/BUILD.gn | 1 + src/tests/DawnTest.cpp | 11 ++ src/tests/DawnTest.h | 1 + src/tests/end2end/DeviceLostTests.cpp | 2 +- src/tests/end2end/QueueTimelineTests.cpp | 164 ++++++++++++++++++ .../validation/FenceValidationTests.cpp | 18 +- 19 files changed, 269 insertions(+), 232 deletions(-) delete mode 100644 src/dawn_native/FenceSignalTracker.cpp delete mode 100644 src/dawn_native/FenceSignalTracker.h delete mode 100644 src/dawn_native/MapRequestTracker.cpp delete mode 100644 src/dawn_native/MapRequestTracker.h create mode 100644 src/tests/end2end/QueueTimelineTests.cpp diff --git a/src/dawn_native/BUILD.gn b/src/dawn_native/BUILD.gn index 0cfe3a745e..5387494dcb 100644 --- a/src/dawn_native/BUILD.gn +++ b/src/dawn_native/BUILD.gn @@ -205,16 +205,12 @@ source_set("dawn_native_sources") { "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 519b0e7d75..b4605dc7f5 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 buffer, MapRequestID id) + : buffer(std::move(buffer)), id(id) { + } + void Finish() override { + buffer->OnMapRequestCompleted(id); + } + ~MapRequestTask() override = default; + + private: + Ref buffer; + MapRequestID id; + }; class ErrorBuffer final : public BufferBase { public: @@ -261,9 +273,10 @@ namespace dawn_native { CallMapCallback(mLastMapID, WGPUBufferMapAsyncStatus_DeviceLost); return; } - - MapRequestTracker* tracker = GetDevice()->GetMapRequestTracker(); - tracker->Track(this, mLastMapID); + std::unique_ptr request = + std::make_unique(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 7aba5b1323..ba0c94f71e 100644 --- a/src/dawn_native/CMakeLists.txt +++ b/src/dawn_native/CMakeLists.txt @@ -88,16 +88,12 @@ target_sources(dawn_native PRIVATE "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 39e88dfb1e..41725fd80f 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 @@ namespace dawn_native { mCaches = std::make_unique(); mErrorScopeTracker = std::make_unique(this); - mFenceSignalTracker = std::make_unique(this); - mMapRequestTracker = std::make_unique(this); mDynamicUploader = std::make_unique(this); mDeprecationWarnings = std::make_unique(); @@ -152,8 +148,7 @@ namespace dawn_native { // 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 @@ namespace dawn_native { mCurrentErrorScope->UnlinkForShutdown(); } mErrorScopeTracker = nullptr; - mFenceSignalTracker = nullptr; mDynamicUploader = nullptr; - mMapRequestTracker = nullptr; mEmptyBindGroupLayout = nullptr; @@ -318,14 +311,6 @@ namespace dawn_native { 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 @@ namespace dawn_native { // 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 97d0b77205..a2f9c4baf2 100644 --- a/src/dawn_native/Device.h +++ b/src/dawn_native/Device.h @@ -36,8 +36,6 @@ namespace dawn_native { class DynamicUploader; class ErrorScope; class ErrorScopeTracker; - class FenceSignalTracker; - class MapRequestTracker; class StagingBufferBase; class DeviceBase { @@ -71,8 +69,6 @@ namespace dawn_native { 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 @@ 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); virtual uint32_t GetOptimalBytesPerRowAlignment() const = 0; @@ -356,8 +359,6 @@ namespace dawn_native { std::unique_ptr mDynamicUploader; std::unique_ptr mErrorScopeTracker; - std::unique_ptr mFenceSignalTracker; - std::unique_ptr mMapRequestTracker; Ref mDefaultQueue; struct DeprecationWarnings; diff --git a/src/dawn_native/Fence.cpp b/src/dawn_native/Fence.cpp index 5c2a64fe14..c1ce24bfcc 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, FenceAPISerial value) + : fence(std::move(fence)), value(value) { + } + void Finish() override { + fence->SetCompletedValue(value); + } + ~FenceInFlight() override = default; + + private: + Ref fence; + FenceAPISerial value; + }; + MaybeError ValidateFenceDescriptor(const FenceDescriptor* descriptor) { if (descriptor->nextInChain != nullptr) { return DAWN_VALIDATION_ERROR("nextInChain must be nullptr"); @@ -118,6 +132,15 @@ namespace dawn_native { mRequests.ClearUpTo(mCompletedValue); } + void Fence::UpdateFenceOnComplete(Fence* fence, FenceAPISerial value) { + std::unique_ptr fenceInFlight = + std::make_unique(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 f5f4be6e34..6c1cbcbd93 100644 --- a/src/dawn_native/Fence.h +++ b/src/dawn_native/Fence.h @@ -44,9 +44,10 @@ namespace dawn_native { 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 c29a9c690b..0000000000 --- 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 61361562af..0000000000 --- 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; - FenceAPISerial value; - }; - - public: - FenceSignalTracker(DeviceBase* device); - ~FenceSignalTracker(); - - void UpdateFenceOnComplete(Fence* fence, FenceAPISerial value); - - void Tick(ExecutionSerial finishedSerial); - - private: - DeviceBase* mDevice; - SerialQueue 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 999657df7a..0000000000 --- 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 08dda23989..0000000000 --- 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 buffer; - MapRequestID id; - }; - SerialQueue mInflightRequests; - }; - -} // namespace dawn_native - -#endif // DAWNNATIVE_MAPREQUESTTRACKER_H diff --git a/src/dawn_native/Queue.cpp b/src/dawn_native/Queue.cpp index c655139d4b..82a37d1270 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 @@ namespace dawn_native { // 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 @@ namespace dawn_native { 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 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 cf1d384082..9e0d6c2671 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 @@ namespace dawn_native { 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 @@ namespace dawn_native { const TextureDataLayout* dataLayout, const Extent3D* writeSize); + void TrackTask(std::unique_ptr task, ExecutionSerial serial); + void Tick(ExecutionSerial finishedSerial); + protected: QueueBase(DeviceBase* device); QueueBase(DeviceBase* device, ObjectBase::ErrorTag tag); @@ -77,6 +87,8 @@ namespace dawn_native { const Extent3D* writeSize) const; void SubmitInternal(uint32_t commandCount, CommandBufferBase* const* commands); + + SerialQueue> mTasksInFlight; }; } // namespace dawn_native diff --git a/src/tests/BUILD.gn b/src/tests/BUILD.gn index 299813c3b3..f50bf2d235 100644 --- a/src/tests/BUILD.gn +++ b/src/tests/BUILD.gn @@ -301,6 +301,7 @@ source_set("dawn_end2end_tests_sources") { "end2end/PrimitiveTopologyTests.cpp", "end2end/QueryTests.cpp", "end2end/QueueTests.cpp", + "end2end/QueueTimelineTests.cpp", "end2end/RenderBundleTests.cpp", "end2end/RenderPassLoadOpTests.cpp", "end2end/RenderPassTests.cpp", diff --git a/src/tests/DawnTest.cpp b/src/tests/DawnTest.cpp index 945d9c1668..d5adb8c155 100644 --- a/src/tests/DawnTest.cpp +++ b/src/tests/DawnTest.cpp @@ -901,6 +901,17 @@ void DawnTestBase::FlushWire() { } } +void DawnTestBase::WaitForAllOperations() { + wgpu::Queue queue = device.GetDefaultQueue(); + wgpu::Fence fence = queue.CreateFence(); + + // Force the currently submitted operations to completed. + queue.Signal(fence, 1); + while (fence.GetCompletedValue() < 1) { + WaitABit(); + } +} + DawnTestBase::ReadbackReservation DawnTestBase::ReserveReadback(uint64_t readbackSize) { // For now create a new MapRead buffer for each readback // TODO(cwallez@chromium.org): eventually make bigger buffers and allocate linearly? diff --git a/src/tests/DawnTest.h b/src/tests/DawnTest.h index 2f818fbd4a..8fea2a1845 100644 --- a/src/tests/DawnTest.h +++ b/src/tests/DawnTest.h @@ -313,6 +313,7 @@ class DawnTestBase { void WaitABit(); void FlushWire(); + void WaitForAllOperations(); bool SupportsExtensions(const std::vector& extensions); diff --git a/src/tests/end2end/DeviceLostTests.cpp b/src/tests/end2end/DeviceLostTests.cpp index 125d957a80..caaacd4f11 100644 --- a/src/tests/end2end/DeviceLostTests.cpp +++ b/src/tests/end2end/DeviceLostTests.cpp @@ -494,7 +494,7 @@ TEST_P(DeviceLostTest, FenceSignalTickOnCompletion) { wgpu::Fence fence = queue.CreateFence(&descriptor); queue.Signal(fence, 2); - device.Tick(); + WaitForAllOperations(); // callback should have device lost status EXPECT_CALL(*mockFenceOnCompletionCallback, Call(WGPUFenceCompletionStatus_Success, nullptr)) diff --git a/src/tests/end2end/QueueTimelineTests.cpp b/src/tests/end2end/QueueTimelineTests.cpp new file mode 100644 index 0000000000..4ff022533e --- /dev/null +++ b/src/tests/end2end/QueueTimelineTests.cpp @@ -0,0 +1,164 @@ +// 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 +#include "tests/DawnTest.h" + +using namespace testing; + +class MockMapCallback { + public: + MOCK_METHOD(void, Call, (WGPUBufferMapAsyncStatus status, void* userdata)); +}; + +static std::unique_ptr mockMapCallback; +static void ToMockMapCallback(WGPUBufferMapAsyncStatus status, void* userdata) { + EXPECT_EQ(status, WGPUBufferMapAsyncStatus_Success); + mockMapCallback->Call(status, userdata); +} + +class MockFenceOnCompletionCallback { + public: + MOCK_METHOD(void, Call, (WGPUFenceCompletionStatus status, void* userdata)); +}; + +static std::unique_ptr mockFenceOnCompletionCallback; +static void ToMockFenceOnCompletionCallback(WGPUFenceCompletionStatus status, void* userdata) { + EXPECT_EQ(status, WGPUFenceCompletionStatus_Success); + mockFenceOnCompletionCallback->Call(status, userdata); +} + +class QueueTimelineTests : public DawnTest { + protected: + void SetUp() override { + DawnTest::SetUp(); + + mockMapCallback = std::make_unique(); + mockFenceOnCompletionCallback = std::make_unique(); + + wgpu::BufferDescriptor descriptor; + descriptor.size = 4; + descriptor.usage = wgpu::BufferUsage::MapRead; + mMapReadBuffer = device.CreateBuffer(&descriptor); + } + + void TearDown() override { + mockFenceOnCompletionCallback = nullptr; + mockMapCallback = nullptr; + DawnTest::TearDown(); + } + + wgpu::Buffer mMapReadBuffer; +}; + +// Test that mMapReadBuffer.MapAsync callback happens before fence.OnCompletion callback +// when queue.Signal is called after mMapReadBuffer.MapAsync. The callback order should +// happen in the order the functions are called. +TEST_P(QueueTimelineTests, MapReadSignalOnComplete) { + testing::InSequence sequence; + EXPECT_CALL(*mockMapCallback, Call(WGPUBufferMapAsyncStatus_Success, this)).Times(1); + EXPECT_CALL(*mockFenceOnCompletionCallback, Call(WGPUFenceCompletionStatus_Success, this)) + .Times(1); + + mMapReadBuffer.MapAsync(wgpu::MapMode::Read, 0, 0, ToMockMapCallback, this); + + wgpu::Fence fence = queue.CreateFence(); + + queue.Signal(fence, 1); + fence.OnCompletion(1u, ToMockFenceOnCompletionCallback, this); + + WaitForAllOperations(); + mMapReadBuffer.Unmap(); +} + +// Test that fence.OnCompletion callback happens before mMapReadBuffer.MapAsync callback when +// queue.Signal is called before mMapReadBuffer.MapAsync. The callback order should +// happen in the order the functions are called. +TEST_P(QueueTimelineTests, SignalMapReadOnComplete) { + testing::InSequence sequence; + EXPECT_CALL(*mockFenceOnCompletionCallback, Call(WGPUFenceCompletionStatus_Success, this)) + .Times(1); + EXPECT_CALL(*mockMapCallback, Call(WGPUBufferMapAsyncStatus_Success, this)).Times(1); + + wgpu::Fence fence = queue.CreateFence(); + queue.Signal(fence, 2); + + mMapReadBuffer.MapAsync(wgpu::MapMode::Read, 0, 0, ToMockMapCallback, this); + + fence.OnCompletion(2u, ToMockFenceOnCompletionCallback, this); + WaitForAllOperations(); + mMapReadBuffer.Unmap(); +} + +// Test that fence.OnCompletion callback happens before mMapReadBuffer.MapAsync callback when +// queue.Signal is called before mMapReadBuffer.MapAsync. The callback order should +// happen in the order the functions are called +TEST_P(QueueTimelineTests, SignalOnCompleteMapRead) { + testing::InSequence sequence; + EXPECT_CALL(*mockFenceOnCompletionCallback, Call(WGPUFenceCompletionStatus_Success, this)) + .Times(1); + EXPECT_CALL(*mockMapCallback, Call(WGPUBufferMapAsyncStatus_Success, this)).Times(1); + + wgpu::Fence fence = queue.CreateFence(); + queue.Signal(fence, 2); + + fence.OnCompletion(2u, ToMockFenceOnCompletionCallback, this); + + mMapReadBuffer.MapAsync(wgpu::MapMode::Read, 0, 0, ToMockMapCallback, this); + + WaitForAllOperations(); + mMapReadBuffer.Unmap(); +} + +TEST_P(QueueTimelineTests, SurroundWithFenceSignals) { + testing::InSequence sequence; + EXPECT_CALL(*mockFenceOnCompletionCallback, Call(WGPUFenceCompletionStatus_Success, this + 0)) + .Times(1); + EXPECT_CALL(*mockFenceOnCompletionCallback, Call(WGPUFenceCompletionStatus_Success, this + 2)) + .Times(1); + EXPECT_CALL(*mockFenceOnCompletionCallback, Call(WGPUFenceCompletionStatus_Success, this + 3)) + .Times(1); + EXPECT_CALL(*mockMapCallback, Call(WGPUBufferMapAsyncStatus_Success, this)).Times(1); + EXPECT_CALL(*mockFenceOnCompletionCallback, Call(WGPUFenceCompletionStatus_Success, this + 5)) + .Times(1); + EXPECT_CALL(*mockFenceOnCompletionCallback, Call(WGPUFenceCompletionStatus_Success, this + 6)) + .Times(1); + EXPECT_CALL(*mockFenceOnCompletionCallback, Call(WGPUFenceCompletionStatus_Success, this + 8)) + .Times(1); + + wgpu::Fence fence = queue.CreateFence(); + queue.Signal(fence, 2); + queue.Signal(fence, 4); + + fence.OnCompletion(1u, ToMockFenceOnCompletionCallback, this + 0); + fence.OnCompletion(2u, ToMockFenceOnCompletionCallback, this + 2); + + mMapReadBuffer.MapAsync(wgpu::MapMode::Read, 0, 0, ToMockMapCallback, this); + queue.Signal(fence, 6); + fence.OnCompletion(3u, ToMockFenceOnCompletionCallback, this + 3); + fence.OnCompletion(5u, ToMockFenceOnCompletionCallback, this + 5); + fence.OnCompletion(6u, ToMockFenceOnCompletionCallback, this + 6); + + queue.Signal(fence, 8); + fence.OnCompletion(8u, ToMockFenceOnCompletionCallback, this + 8); + + WaitForAllOperations(); + mMapReadBuffer.Unmap(); +} + +DAWN_INSTANTIATE_TEST(QueueTimelineTests, + D3D12Backend(), + MetalBackend(), + OpenGLBackend(), + VulkanBackend()); diff --git a/src/tests/unittests/validation/FenceValidationTests.cpp b/src/tests/unittests/validation/FenceValidationTests.cpp index 127ea02fe8..c6646a7361 100644 --- a/src/tests/unittests/validation/FenceValidationTests.cpp +++ b/src/tests/unittests/validation/FenceValidationTests.cpp @@ -46,10 +46,6 @@ class FenceValidationTest : public ValidationTest { fence.OnCompletion(value, ToMockFenceOnCompletionCallback, expectation); } - void Flush() { - device.Tick(); - } - wgpu::Queue queue; private: @@ -127,7 +123,7 @@ TEST_F(FenceValidationTest, OnCompletionLargerThanSignaled) { .Times(1); fence.OnCompletion(2u, ToMockFenceOnCompletionCallback, nullptr); - Flush(); + WaitForAllOperations(device); } TEST_F(FenceValidationTest, GetCompletedValueInsideCallback) { @@ -142,7 +138,7 @@ TEST_F(FenceValidationTest, GetCompletedValueInsideCallback) { EXPECT_EQ(fence.GetCompletedValue(), 3u); })); - Flush(); + WaitForAllOperations(device); } TEST_F(FenceValidationTest, GetCompletedValueAfterCallback) { @@ -155,7 +151,7 @@ TEST_F(FenceValidationTest, GetCompletedValueAfterCallback) { EXPECT_CALL(*mockFenceOnCompletionCallback, Call(WGPUFenceCompletionStatus_Success, nullptr)) .Times(1); - Flush(); + WaitForAllOperations(device); EXPECT_EQ(fence.GetCompletedValue(), 2u); } @@ -178,12 +174,12 @@ TEST_F(FenceValidationTest, SignalSuccess) { // Success queue.Signal(fence, 2); - Flush(); + WaitForAllOperations(device); EXPECT_EQ(fence.GetCompletedValue(), 2u); // Success increasing fence value by more than 1 queue.Signal(fence, 6); - Flush(); + WaitForAllOperations(device); EXPECT_EQ(fence.GetCompletedValue(), 6u); } @@ -211,11 +207,11 @@ TEST_F(FenceValidationTest, DISABLED_SignalWrongQueueDoesNotUpdateValue) { ASSERT_DEVICE_ERROR(queue2.Signal(fence, 2)); // Fence value should be unchanged. - Flush(); + WaitForAllOperations(device); EXPECT_EQ(fence.GetCompletedValue(), 1u); // Signaling with 2 on the correct queue should succeed queue.Signal(fence, 2); - Flush(); + WaitForAllOperations(device); EXPECT_EQ(fence.GetCompletedValue(), 2u); }