From f93791ab62965964f413744bb73884dd6b9abc42 Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Fri, 17 Apr 2020 17:09:07 +0000 Subject: [PATCH] Special-case GetDefaultQueue in the wire This makes it so calling GetDefaultQueue always returns the same object. It required updating various WireTests to account for the additional wire calls. Bug: dawn:22 Change-Id: I8c74374b7c732b8bb7d0490bbc740dee0d2dface Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/19726 Commit-Queue: Corentin Wallez Reviewed-by: Austin Eng Reviewed-by: Kai Ninomiya --- dawn_wire.json | 1 + src/dawn_wire/client/ApiProcs.cpp | 5 +++ src/dawn_wire/client/Client.cpp | 3 +- src/dawn_wire/client/Device.cpp | 31 +++++++++++++++++++ src/dawn_wire/client/Device.h | 9 ++++-- src/tests/end2end/QueueTests.cpp | 2 +- .../unittests/wire/WireArgumentTests.cpp | 5 --- src/tests/unittests/wire/WireFenceTests.cpp | 15 +++------ .../wire/WireMultipleDeviceTests.cpp | 30 +++++++++++------- src/tests/unittests/wire/WireTest.cpp | 7 +++++ src/tests/unittests/wire/WireTest.h | 2 ++ 11 files changed, 79 insertions(+), 31 deletions(-) diff --git a/dawn_wire.json b/dawn_wire.json index b43098fa0a..cdd0d97d34 100644 --- a/dawn_wire.json +++ b/dawn_wire.json @@ -101,6 +101,7 @@ "BufferUnmap", "DeviceCreateBuffer", "DeviceCreateBufferMapped", + "DeviceGetDefaultQueue", "DevicePushErrorScope", "QueueCreateFence", "QueueSignal" diff --git a/src/dawn_wire/client/ApiProcs.cpp b/src/dawn_wire/client/ApiProcs.cpp index 1d73a7ebb7..fddbbdec97 100644 --- a/src/dawn_wire/client/ApiProcs.cpp +++ b/src/dawn_wire/client/ApiProcs.cpp @@ -370,6 +370,11 @@ namespace dawn_wire { namespace client { void ClientDeviceRelease(WGPUDevice) { } + WGPUQueue ClientHandwrittenDeviceGetDefaultQueue(WGPUDevice cSelf) { + Device* device = reinterpret_cast(cSelf); + return device->GetDefaultQueue(); + } + void ClientHandwrittenDeviceSetUncapturedErrorCallback(WGPUDevice cSelf, WGPUErrorCallback callback, void* userdata) { diff --git a/src/dawn_wire/client/Client.cpp b/src/dawn_wire/client/Client.cpp index 1214e3824f..7024559b81 100644 --- a/src/dawn_wire/client/Client.cpp +++ b/src/dawn_wire/client/Client.cpp @@ -21,7 +21,6 @@ namespace dawn_wire { namespace client { Client::Client(CommandSerializer* serializer, MemoryTransferService* memoryTransferService) : ClientBase(), - mDevice(DeviceAllocator().New(this)->object.get()), mSerializer(serializer), mMemoryTransferService(memoryTransferService) { if (mMemoryTransferService == nullptr) { @@ -29,6 +28,8 @@ namespace dawn_wire { namespace client { mOwnedMemoryTransferService = CreateInlineMemoryTransferService(); mMemoryTransferService = mOwnedMemoryTransferService.get(); } + + mDevice = DeviceAllocator().New(this)->object.get(); } Client::~Client() { diff --git a/src/dawn_wire/client/Device.cpp b/src/dawn_wire/client/Device.cpp index 67021ef046..c69983ae00 100644 --- a/src/dawn_wire/client/Device.cpp +++ b/src/dawn_wire/client/Device.cpp @@ -16,20 +16,46 @@ #include "common/Assert.h" #include "dawn_wire/WireCmd_autogen.h" +#include "dawn_wire/client/ApiObjects_autogen.h" #include "dawn_wire/client/Client.h" +#include "dawn_wire/client/ObjectAllocator.h" namespace dawn_wire { namespace client { Device::Device(Client* client, uint32_t initialRefcount, uint32_t initialId) : ObjectBase(this, initialRefcount, initialId), mClient(client) { this->device = this; + + // Get the default queue for this device. + ObjectAllocator::ObjectAndSerial* allocation = mClient->QueueAllocator().New(this); + mDefaultQueue = allocation->object.get(); + + DeviceGetDefaultQueueCmd cmd; + cmd.self = reinterpret_cast(this); + cmd.result = ObjectHandle{allocation->object->id, allocation->generation}; + + size_t requiredSize = cmd.GetRequiredSize(); + char* allocatedBuffer = static_cast(mClient->GetCmdSpace(requiredSize)); + cmd.Serialize(allocatedBuffer, *mClient); } Device::~Device() { + // Fire pending error scopes auto errorScopes = std::move(mErrorScopes); for (const auto& it : errorScopes) { it.second.callback(WGPUErrorType_Unknown, "Device destroyed", it.second.userdata); } + + // Destroy the default queue + DestroyObjectCmd cmd; + cmd.objectType = ObjectType::Queue; + cmd.objectId = mDefaultQueue->id; + + size_t requiredSize = cmd.GetRequiredSize(); + char* allocatedBuffer = static_cast(mClient->GetCmdSpace(requiredSize)); + cmd.Serialize(allocatedBuffer); + + mClient->QueueAllocator().Free(mDefaultQueue); } Client* Device::GetClient() { @@ -119,4 +145,9 @@ namespace dawn_wire { namespace client { return true; } + WGPUQueue Device::GetDefaultQueue() { + mDefaultQueue->refcount++; + return reinterpret_cast(mDefaultQueue); + } + }} // namespace dawn_wire::client diff --git a/src/dawn_wire/client/Device.h b/src/dawn_wire/client/Device.h index e70cae2b11..f32259a76f 100644 --- a/src/dawn_wire/client/Device.h +++ b/src/dawn_wire/client/Device.h @@ -24,6 +24,7 @@ namespace dawn_wire { namespace client { class Client; + struct Queue; class Device : public ObjectBase { public: @@ -40,6 +41,8 @@ namespace dawn_wire { namespace client { bool RequestPopErrorScope(WGPUErrorCallback callback, void* userdata); bool PopErrorScope(uint64_t requestSerial, WGPUErrorType type, const char* message); + WGPUQueue GetDefaultQueue(); + private: struct ErrorScopeData { WGPUErrorCallback callback = nullptr; @@ -53,8 +56,10 @@ namespace dawn_wire { namespace client { WGPUErrorCallback mErrorCallback = nullptr; WGPUDeviceLostCallback mDeviceLostCallback = nullptr; bool mDidRunLostCallback = false; - void* mErrorUserdata; - void* mDeviceLostUserdata; + void* mErrorUserdata = nullptr; + void* mDeviceLostUserdata = nullptr; + + Queue* mDefaultQueue = nullptr; }; }} // namespace dawn_wire::client diff --git a/src/tests/end2end/QueueTests.cpp b/src/tests/end2end/QueueTests.cpp index 242503b0f0..710caec988 100644 --- a/src/tests/end2end/QueueTests.cpp +++ b/src/tests/end2end/QueueTests.cpp @@ -25,7 +25,7 @@ class QueueTests : public DawnTest {}; TEST_P(QueueTests, GetDefaultQueueSameObject) { wgpu::Queue q1 = device.GetDefaultQueue(); wgpu::Queue q2 = device.GetDefaultQueue(); - EXPECT_EQ(q1.Get() == q2.Get(), !UsesWire()); + EXPECT_EQ(q1.Get(), q2.Get()); } DAWN_INSTANTIATE_TEST(QueueTests, diff --git a/src/tests/unittests/wire/WireArgumentTests.cpp b/src/tests/unittests/wire/WireArgumentTests.cpp index 457625a56e..8664424441 100644 --- a/src/tests/unittests/wire/WireArgumentTests.cpp +++ b/src/tests/unittests/wire/WireArgumentTests.cpp @@ -232,11 +232,6 @@ TEST_F(WireArgumentTests, ObjectsAsPointerArgument) { .WillOnce(Return(apiCmdBufs[i])); } - // Create queue - WGPUQueue queue = wgpuDeviceGetDefaultQueue(device); - WGPUQueue apiQueue = api.GetNewQueue(); - EXPECT_CALL(api, DeviceGetDefaultQueue(apiDevice)).WillOnce(Return(apiQueue)); - // Submit command buffer and check we got a call with both API-side command buffers wgpuQueueSubmit(queue, 2, cmdBufs); diff --git a/src/tests/unittests/wire/WireFenceTests.cpp b/src/tests/unittests/wire/WireFenceTests.cpp index ffff49f374..1cd57708c4 100644 --- a/src/tests/unittests/wire/WireFenceTests.cpp +++ b/src/tests/unittests/wire/WireFenceTests.cpp @@ -43,12 +43,6 @@ class WireFenceTests : public WireTest { mockFenceOnCompletionCallback = std::make_unique>(); - { - queue = wgpuDeviceGetDefaultQueue(device); - apiQueue = api.GetNewQueue(); - EXPECT_CALL(api, DeviceGetDefaultQueue(apiDevice)).WillOnce(Return(apiQueue)); - FlushClient(); - } { WGPUFenceDescriptor descriptor = {}; descriptor.initialValue = 1; @@ -89,9 +83,6 @@ class WireFenceTests : public WireTest { // A successfully created fence WGPUFence fence; WGPUFence apiFence; - - WGPUQueue queue; - WGPUQueue apiQueue; }; // Check that signaling a fence succeeds @@ -227,7 +218,8 @@ TEST_F(WireFenceTests, DestroyBeforeOnCompletionEnd) { } // Test that signaling a fence on a wrong queue is invalid -TEST_F(WireFenceTests, SignalWrongQueue) { +// DISABLED until we have support for multiple queues. +TEST_F(WireFenceTests, DISABLED_SignalWrongQueue) { WGPUQueue queue2 = wgpuDeviceGetDefaultQueue(device); WGPUQueue apiQueue2 = api.GetNewQueue(); EXPECT_CALL(api, DeviceGetDefaultQueue(apiDevice)).WillOnce(Return(apiQueue2)); @@ -240,7 +232,8 @@ TEST_F(WireFenceTests, SignalWrongQueue) { } // Test that signaling a fence on a wrong queue does not update fence signaled value -TEST_F(WireFenceTests, SignalWrongQueueDoesNotUpdateValue) { +// DISABLED until we have support for multiple queues. +TEST_F(WireFenceTests, DISABLED_SignalWrongQueueDoesNotUpdateValue) { WGPUQueue queue2 = wgpuDeviceGetDefaultQueue(device); WGPUQueue apiQueue2 = api.GetNewQueue(); EXPECT_CALL(api, DeviceGetDefaultQueue(apiDevice)).WillOnce(Return(apiQueue2)); diff --git a/src/tests/unittests/wire/WireMultipleDeviceTests.cpp b/src/tests/unittests/wire/WireMultipleDeviceTests.cpp index da6f1e0d2f..e3aef3a568 100644 --- a/src/tests/unittests/wire/WireMultipleDeviceTests.cpp +++ b/src/tests/unittests/wire/WireMultipleDeviceTests.cpp @@ -69,6 +69,12 @@ class WireMultipleDeviceTests : public testing::Test { mS2cBuf->SetHandler(mWireClient.get()); mClientDevice = mWireClient->GetDevice(); + + // The GetDefaultQueue is done on WireClient startup so we expect it now. + mClientQueue = wgpuDeviceGetDefaultQueue(mClientDevice); + mServerQueue = mApi.GetNewQueue(); + EXPECT_CALL(mApi, DeviceGetDefaultQueue(mServerDevice)).WillOnce(Return(mServerQueue)); + FlushClient(); } ~WireHolder() { @@ -97,6 +103,14 @@ class WireMultipleDeviceTests : public testing::Test { return mServerDevice; } + WGPUQueue ClientQueue() { + return mClientQueue; + } + + WGPUQueue ServerQueue() { + return mServerQueue; + } + private: testing::StrictMock mApi; std::unique_ptr mWireServer; @@ -105,6 +119,8 @@ class WireMultipleDeviceTests : public testing::Test { std::unique_ptr mC2sBuf; WGPUDevice mServerDevice; WGPUDevice mClientDevice; + WGPUQueue mServerQueue; + WGPUQueue mClientQueue; }; void ExpectInjectedError(WireHolder* wire) { @@ -134,20 +150,12 @@ TEST_F(WireMultipleDeviceTests, ValidatesSameDevice) { WireHolder wireA; WireHolder wireB; - // Create the objects - WGPUQueue queueA = wgpuDeviceGetDefaultQueue(wireA.ClientDevice()); - WGPUQueue queueB = wgpuDeviceGetDefaultQueue(wireB.ClientDevice()); - + // Create the fence WGPUFenceDescriptor desc = {}; - WGPUFence fenceA = wgpuQueueCreateFence(queueA, &desc); - - // Flush on wire B. We should see the queue created. - EXPECT_CALL(*wireB.Api(), DeviceGetDefaultQueue(wireB.ServerDevice())) - .WillOnce(Return(wireB.Api()->GetNewQueue())); - wireB.FlushClient(); + WGPUFence fenceA = wgpuQueueCreateFence(wireA.ClientQueue(), &desc); // Signal with a fence from a different wire. - wgpuQueueSignal(queueB, fenceA, 1u); + wgpuQueueSignal(wireB.ClientQueue(), fenceA, 1u); // We should inject an error into the server. ExpectInjectedError(&wireB); diff --git a/src/tests/unittests/wire/WireTest.cpp b/src/tests/unittests/wire/WireTest.cpp index ee4a62b0c0..bdac99f2f7 100644 --- a/src/tests/unittests/wire/WireTest.cpp +++ b/src/tests/unittests/wire/WireTest.cpp @@ -70,6 +70,12 @@ void WireTest::SetUp() { dawnProcSetProcs(&clientProcs); apiDevice = mockDevice; + + // The GetDefaultQueue is done on WireClient startup so we expect it now. + queue = wgpuDeviceGetDefaultQueue(device); + apiQueue = api.GetNewQueue(); + EXPECT_CALL(api, DeviceGetDefaultQueue(apiDevice)).WillOnce(Return(apiQueue)); + FlushClient(); } void WireTest::TearDown() { @@ -104,6 +110,7 @@ dawn_wire::WireClient* WireTest::GetWireClient() { } void WireTest::DeleteServer() { + EXPECT_CALL(api, QueueRelease(apiQueue)).Times(1); mWireServer = nullptr; } diff --git a/src/tests/unittests/wire/WireTest.h b/src/tests/unittests/wire/WireTest.h index 7a6c234a29..7a8ed5a6ed 100644 --- a/src/tests/unittests/wire/WireTest.h +++ b/src/tests/unittests/wire/WireTest.h @@ -123,7 +123,9 @@ class WireTest : public testing::Test { testing::StrictMock api; WGPUDevice apiDevice; + WGPUQueue apiQueue; WGPUDevice device; + WGPUQueue queue; dawn_wire::WireServer* GetWireServer(); dawn_wire::WireClient* GetWireClient();