Add Reserve/InjectBuffer to Wire Adds reserve and inject functionality for buffers to the wire. Includes basic tests. This is necessary for Chromium to utilize SharedBufferMemory. Bug: dawn:2382 Change-Id: I9599c1bff32815a0437c21e8810dc0cb5a6623bb Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/186825 Reviewed-by: Austin Eng <enga@chromium.org> Commit-Queue: Brandon1 Jones <brandon1.jones@intel.com> Reviewed-by: Corentin Wallez <cwallez@chromium.org>
diff --git a/include/dawn/wire/WireClient.h b/include/dawn/wire/WireClient.h index a1e268a..82cb2d7 100644 --- a/include/dawn/wire/WireClient.h +++ b/include/dawn/wire/WireClient.h
@@ -43,6 +43,12 @@ DAWN_WIRE_EXPORT const DawnProcTable& GetProcs(); } // namespace client +struct ReservedBuffer { + WGPUBuffer buffer; + Handle handle; + Handle deviceHandle; +}; + struct ReservedTexture { WGPUTexture texture; Handle handle; @@ -77,11 +83,13 @@ const volatile char* HandleCommands(const volatile char* commands, size_t size) override; + ReservedBuffer ReserveBuffer(WGPUDevice device, const WGPUBufferDescriptor* descriptor); ReservedTexture ReserveTexture(WGPUDevice device, const WGPUTextureDescriptor* descriptor); ReservedSwapChain ReserveSwapChain(WGPUDevice device, const WGPUSwapChainDescriptor* descriptor); ReservedInstance ReserveInstance(const WGPUInstanceDescriptor* descriptor = nullptr); + void ReclaimBufferReservation(const ReservedBuffer& reservation); void ReclaimTextureReservation(const ReservedTexture& reservation); void ReclaimSwapChainReservation(const ReservedSwapChain& reservation); void ReclaimDeviceReservation(const ReservedDevice& reservation);
diff --git a/include/dawn/wire/WireServer.h b/include/dawn/wire/WireServer.h index a3b9972..eb1a68a 100644 --- a/include/dawn/wire/WireServer.h +++ b/include/dawn/wire/WireServer.h
@@ -54,6 +54,7 @@ const volatile char* HandleCommands(const volatile char* commands, size_t size) override; + bool InjectBuffer(WGPUBuffer buffer, const Handle& handle, const Handle& deviceHandle); bool InjectTexture(WGPUTexture texture, const Handle& handle, const Handle& deviceHandle); bool InjectSwapChain(WGPUSwapChain swapchain, const Handle& handle, const Handle& deviceHandle); bool InjectInstance(WGPUInstance instance, const Handle& handle);
diff --git a/src/dawn/tests/BUILD.gn b/src/dawn/tests/BUILD.gn index e858d83..6158b5d 100644 --- a/src/dawn/tests/BUILD.gn +++ b/src/dawn/tests/BUILD.gn
@@ -437,6 +437,7 @@ "unittests/wire/WireExtensionTests.cpp", "unittests/wire/WireFutureTest.cpp", "unittests/wire/WireFutureTest.h", + "unittests/wire/WireInjectBufferTests.cpp", "unittests/wire/WireInjectInstanceTests.cpp", "unittests/wire/WireInjectSwapChainTests.cpp", "unittests/wire/WireInjectTextureTests.cpp",
diff --git a/src/dawn/tests/unittests/wire/WireInjectBufferTests.cpp b/src/dawn/tests/unittests/wire/WireInjectBufferTests.cpp new file mode 100644 index 0000000..f6abb29 --- /dev/null +++ b/src/dawn/tests/unittests/wire/WireInjectBufferTests.cpp
@@ -0,0 +1,181 @@ +// Copyright 2024 The Dawn & Tint Authors +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are met: +// +// 1. Redistributions of source code must retain the above copyright notice, this +// list of conditions and the following disclaimer. +// +// 2. Redistributions in binary form must reproduce the above copyright notice, +// this list of conditions and the following disclaimer in the documentation +// and/or other materials provided with the distribution. +// +// 3. Neither the name of the copyright holder nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +// AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +// DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE +// FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +// DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +// SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +// CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +// OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +#include "dawn/tests/unittests/wire/WireTest.h" + +#include "dawn/wire/WireClient.h" +#include "dawn/wire/WireServer.h" + +namespace dawn::wire { +namespace { + +using testing::Mock; +using testing::Return; + +class WireInjectBufferTests : public WireTest { + public: + WireInjectBufferTests() {} + ~WireInjectBufferTests() override = default; + + // A placeholder buffer format for ReserveBuffer. The data in it doesn't matter as long as + // we don't call buffer reflection methods. + WGPUBufferDescriptor placeholderDesc = {}; +}; + +// Test that reserving and injecting a buffer makes calls on the client object forward to the +// server object correctly. +TEST_F(WireInjectBufferTests, CallAfterReserveInject) { + auto reserved = GetWireClient()->ReserveBuffer(device, &placeholderDesc); + + WGPUBuffer apiBuffer = api.GetNewBuffer(); + EXPECT_CALL(api, BufferAddRef(apiBuffer)); + ASSERT_TRUE(GetWireServer()->InjectBuffer(apiBuffer, reserved.handle, reserved.deviceHandle)); + + wgpuBufferDestroy(reserved.buffer); + EXPECT_CALL(api, BufferDestroy(apiBuffer)); + FlushClient(); +} + +// Test that reserve correctly returns different IDs each time. +TEST_F(WireInjectBufferTests, ReserveDifferentIDs) { + auto reserved1 = GetWireClient()->ReserveBuffer(device, &placeholderDesc); + auto reserved2 = GetWireClient()->ReserveBuffer(device, &placeholderDesc); + + ASSERT_NE(reserved1.handle.id, reserved2.handle.id); + ASSERT_NE(reserved1.buffer, reserved2.buffer); +} + +// Test that injecting the same id without a destroy first fails. +TEST_F(WireInjectBufferTests, InjectExistingID) { + auto reserved = GetWireClient()->ReserveBuffer(device, &placeholderDesc); + + WGPUBuffer apiBuffer = api.GetNewBuffer(); + EXPECT_CALL(api, BufferAddRef(apiBuffer)); + ASSERT_TRUE(GetWireServer()->InjectBuffer(apiBuffer, reserved.handle, reserved.deviceHandle)); + + // ID already in use, call fails. + ASSERT_FALSE(GetWireServer()->InjectBuffer(apiBuffer, reserved.handle, reserved.deviceHandle)); +} + +// Test that injecting the same id without a destroy first fails. +TEST_F(WireInjectBufferTests, ReuseIDAndGeneration) { + // Do this loop multiple times since the first time, we can't test `generation - 1` since + // generation == 0. + ReservedBuffer reserved; + WGPUBuffer apiBuffer = nullptr; + for (int i = 0; i < 2; ++i) { + reserved = GetWireClient()->ReserveBuffer(device, &placeholderDesc); + + apiBuffer = api.GetNewBuffer(); + EXPECT_CALL(api, BufferAddRef(apiBuffer)); + ASSERT_TRUE( + GetWireServer()->InjectBuffer(apiBuffer, reserved.handle, reserved.deviceHandle)); + + // Release the buffer. It should be possible to reuse the ID now, but not the generation + wgpuBufferRelease(reserved.buffer); + EXPECT_CALL(api, BufferRelease(apiBuffer)); + FlushClient(); + + // Invalid to inject with the same ID and generation. + ASSERT_FALSE( + GetWireServer()->InjectBuffer(apiBuffer, reserved.handle, reserved.deviceHandle)); + if (i > 0) { + EXPECT_GE(reserved.handle.generation, 1u); + + // Invalid to inject with the same ID and lesser generation. + reserved.handle.generation -= 1; + ASSERT_FALSE( + GetWireServer()->InjectBuffer(apiBuffer, reserved.handle, reserved.deviceHandle)); + } + } + + // Valid to inject with the same ID and greater generation. + EXPECT_CALL(api, BufferAddRef(apiBuffer)); + reserved.handle.generation += 2; + ASSERT_TRUE(GetWireServer()->InjectBuffer(apiBuffer, reserved.handle, reserved.deviceHandle)); +} + +// Test that the server only borrows the buffer and does a single reference-release +TEST_F(WireInjectBufferTests, InjectedBufferLifetime) { + auto reserved = GetWireClient()->ReserveBuffer(device, &placeholderDesc); + + // Injecting the buffer adds a reference + WGPUBuffer apiBuffer = api.GetNewBuffer(); + EXPECT_CALL(api, BufferAddRef(apiBuffer)); + ASSERT_TRUE(GetWireServer()->InjectBuffer(apiBuffer, reserved.handle, reserved.deviceHandle)); + + // Releasing the buffer removes a single reference. + wgpuBufferRelease(reserved.buffer); + EXPECT_CALL(api, BufferRelease(apiBuffer)); + FlushClient(); + + // Deleting the server doesn't release a second reference. + DeleteServer(); + Mock::VerifyAndClearExpectations(&api); +} + +// Test that a buffer reservation can be reclaimed. This is necessary to +// avoid leaking ObjectIDs for reservations that are never injected. +TEST_F(WireInjectBufferTests, ReclaimBufferReservation) { + // Test that doing a reservation and full release is an error. + { + auto reserved = GetWireClient()->ReserveBuffer(device, &placeholderDesc); + wgpuBufferRelease(reserved.buffer); + FlushClient(false); + } + + // Test that doing a reservation and then reclaiming it recycles the ID. + { + auto reserved1 = GetWireClient()->ReserveBuffer(device, &placeholderDesc); + GetWireClient()->ReclaimBufferReservation(reserved1); + + auto reserved2 = GetWireClient()->ReserveBuffer(device, &placeholderDesc); + + // The ID is the same, but the generation is still different. + ASSERT_EQ(reserved1.handle.id, reserved2.handle.id); + ASSERT_NE(reserved1.handle.generation, reserved2.handle.generation); + + // No errors should occur. + FlushClient(); + } +} + +// Test the reflection of buffer creation parameters for reserved buffer. +TEST_F(WireInjectBufferTests, ReservedBufferReflection) { + WGPUBufferDescriptor desc = {}; + desc.size = 10; + desc.usage = WGPUBufferUsage_Storage; + + auto reserved = GetWireClient()->ReserveBuffer(device, &desc); + WGPUBuffer buffer = reserved.buffer; + + ASSERT_EQ(desc.size, wgpuBufferGetSize(buffer)); + ASSERT_EQ(desc.usage, wgpuBufferGetUsage(buffer)); +} + +} // anonymous namespace +} // namespace dawn::wire
diff --git a/src/dawn/wire/WireClient.cpp b/src/dawn/wire/WireClient.cpp index aa15aa2..e616232 100644 --- a/src/dawn/wire/WireClient.cpp +++ b/src/dawn/wire/WireClient.cpp
@@ -41,6 +41,11 @@ return mImpl->HandleCommands(commands, size); } +ReservedBuffer WireClient::ReserveBuffer(WGPUDevice device, + const WGPUBufferDescriptor* descriptor) { + return mImpl->ReserveBuffer(device, descriptor); +} + ReservedTexture WireClient::ReserveTexture(WGPUDevice device, const WGPUTextureDescriptor* descriptor) { return mImpl->ReserveTexture(device, descriptor); @@ -55,6 +60,10 @@ return mImpl->ReserveInstance(descriptor); } +void WireClient::ReclaimBufferReservation(const ReservedBuffer& reservation) { + mImpl->ReclaimBufferReservation(reservation); +} + void WireClient::ReclaimTextureReservation(const ReservedTexture& reservation) { mImpl->ReclaimTextureReservation(reservation); }
diff --git a/src/dawn/wire/WireServer.cpp b/src/dawn/wire/WireServer.cpp index 9437e54..9f34ae0 100644 --- a/src/dawn/wire/WireServer.cpp +++ b/src/dawn/wire/WireServer.cpp
@@ -43,6 +43,10 @@ return mImpl->HandleCommands(commands, size); } +bool WireServer::InjectBuffer(WGPUBuffer buffer, const Handle& handle, const Handle& deviceHandle) { + return mImpl->InjectBuffer(buffer, handle, deviceHandle) == WireResult::Success; +} + bool WireServer::InjectTexture(WGPUTexture texture, const Handle& handle, const Handle& deviceHandle) {
diff --git a/src/dawn/wire/client/Client.cpp b/src/dawn/wire/client/Client.cpp index 3d9aac3..3cc08d6 100644 --- a/src/dawn/wire/client/Client.cpp +++ b/src/dawn/wire/client/Client.cpp
@@ -99,6 +99,16 @@ } } +ReservedBuffer Client::ReserveBuffer(WGPUDevice device, const WGPUBufferDescriptor* descriptor) { + Buffer* buffer = Make<Buffer>(FromAPI(device)->GetEventManagerHandle(), descriptor); + + ReservedBuffer result; + result.buffer = ToAPI(buffer); + result.handle = buffer->GetWireHandle(); + result.deviceHandle = FromAPI(device)->GetWireHandle(); + return result; +} + ReservedTexture Client::ReserveTexture(WGPUDevice device, const WGPUTextureDescriptor* descriptor) { Texture* texture = Make<Texture>(descriptor); @@ -137,6 +147,10 @@ return result; } +void Client::ReclaimBufferReservation(const ReservedBuffer& reservation) { + Free(FromAPI(reservation.buffer)); +} + void Client::ReclaimTextureReservation(const ReservedTexture& reservation) { Free(FromAPI(reservation.texture)); }
diff --git a/src/dawn/wire/client/Client.h b/src/dawn/wire/client/Client.h index 0140680..d389bd5 100644 --- a/src/dawn/wire/client/Client.h +++ b/src/dawn/wire/client/Client.h
@@ -83,11 +83,13 @@ MemoryTransferService* GetMemoryTransferService() const { return mMemoryTransferService; } + ReservedBuffer ReserveBuffer(WGPUDevice device, const WGPUBufferDescriptor* descriptor); ReservedTexture ReserveTexture(WGPUDevice device, const WGPUTextureDescriptor* descriptor); ReservedSwapChain ReserveSwapChain(WGPUDevice device, const WGPUSwapChainDescriptor* descriptor); ReservedInstance ReserveInstance(const WGPUInstanceDescriptor* descriptor); + void ReclaimBufferReservation(const ReservedBuffer& reservation); void ReclaimTextureReservation(const ReservedTexture& reservation); void ReclaimSwapChainReservation(const ReservedSwapChain& reservation); void ReclaimDeviceReservation(const ReservedDevice& reservation);
diff --git a/src/dawn/wire/server/Server.cpp b/src/dawn/wire/server/Server.cpp index c835a3f..b363c19 100644 --- a/src/dawn/wire/server/Server.cpp +++ b/src/dawn/wire/server/Server.cpp
@@ -61,6 +61,30 @@ DestroyAllObjects(mProcs); } +WireResult Server::InjectBuffer(WGPUBuffer buffer, + const Handle& handle, + const Handle& deviceHandle) { + DAWN_ASSERT(buffer != nullptr); + Known<WGPUDevice> device; + WIRE_TRY(Objects<WGPUDevice>().Get(deviceHandle.id, &device)); + if (device->generation != deviceHandle.generation) { + return WireResult::FatalError; + } + + Reserved<WGPUBuffer> data; + WIRE_TRY(Objects<WGPUBuffer>().Allocate(&data, handle)); + + data->handle = buffer; + data->generation = handle.generation; + data->state = AllocationState::Allocated; + + // The Buffer is externally owned so it shouldn't be destroyed when we receive a destroy + // message from the client. Add a reference to counterbalance the eventual release. + mProcs.bufferAddRef(buffer); + + return WireResult::Success; +} + WireResult Server::InjectTexture(WGPUTexture texture, const Handle& handle, const Handle& deviceHandle) {
diff --git a/src/dawn/wire/server/Server.h b/src/dawn/wire/server/Server.h index ba1b07f..2d29945 100644 --- a/src/dawn/wire/server/Server.h +++ b/src/dawn/wire/server/Server.h
@@ -180,6 +180,7 @@ // ChunkedCommandHandler implementation const volatile char* HandleCommandsImpl(const volatile char* commands, size_t size) override; + WireResult InjectBuffer(WGPUBuffer buffer, const Handle& handle, const Handle& deviceHandle); WireResult InjectTexture(WGPUTexture texture, const Handle& handle, const Handle& deviceHandle); WireResult InjectSwapChain(WGPUSwapChain swapchain, const Handle& handle,