Revert "Add a per-thread proc table using thread local storage"
This reverts commit b04a92f01be0e7dc2faf91e0f6f974f990e8b2f2.
Reason for revert: Build failed on win-archive-dbg; see:
https://chromium-review.googlesource.com/c/chromium/src/+/2450792
Original change's description:
> Add a per-thread proc table using thread local storage
>
> In situations where both dawn_wire and dawn_native are used on separate
> threads (Chrome with --single-process or --in-process-gpu), it's
> desirable to have a per-thread proc table so that the WebGPU C++ API can
> still be used. This eliminates classes of bugs with manual
> reference/release errors.
>
> This also changes many of the GetProcs functions to return const
> references to the static proc tables known at compile time, instead of a
> copy.
>
> Bug: none
> Change-Id: I8775bb715b312dd9476a1903fbd797d4b1302614
> Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/29240
> Reviewed-by: Stephen White <senorblanco@chromium.org>
> Reviewed-by: Corentin Wallez <cwallez@chromium.org>
> Commit-Queue: Austin Eng <enga@chromium.org>
TBR=cwallez@chromium.org,senorblanco@chromium.org,enga@chromium.org
Change-Id: I4587b457b7b2dd5d3c7457065bf4e77b95af59d7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: none
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/29442
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Kai Ninomiya <kainino@chromium.org>
diff --git a/generator/dawn_json_generator.py b/generator/dawn_json_generator.py
index 02c5723..b7d9003 100644
--- a/generator/dawn_json_generator.py
+++ b/generator/dawn_json_generator.py
@@ -687,10 +687,6 @@
renders.append(
FileRender('dawn_proc.c', 'src/dawn/dawn_proc.c',
[base_params, api_params]))
- renders.append(
- FileRender('dawn_thread_dispatch_proc.cpp',
- 'src/dawn/dawn_thread_dispatch_proc.cpp',
- [base_params, api_params]))
if 'dawncpp' in targets:
renders.append(
diff --git a/generator/templates/dawn_native/ProcTable.cpp b/generator/templates/dawn_native/ProcTable.cpp
index defae2d..88b780c 100644
--- a/generator/templates/dawn_native/ProcTable.cpp
+++ b/generator/templates/dawn_native/ProcTable.cpp
@@ -135,17 +135,16 @@
return result;
}
- static DawnProcTable gProcTable = {
- NativeGetProcAddress,
- NativeCreateInstance,
+ DawnProcTable GetProcsAutogen() {
+ DawnProcTable table;
+ table.getProcAddress = NativeGetProcAddress;
+ table.createInstance = NativeCreateInstance;
{% for type in by_category["object"] %}
{% for method in c_methods(type) %}
- Native{{as_MethodSuffix(type.name, method.name)}},
+ table.{{as_varName(type.name, method.name)}} = Native{{as_MethodSuffix(type.name, method.name)}};
{% endfor %}
{% endfor %}
- };
-
- const DawnProcTable& GetProcsAutogen() {
- return gProcTable;
+ return table;
}
+
}
diff --git a/generator/templates/dawn_proc_table.h b/generator/templates/dawn_proc_table.h
index 1da1f73..197f300 100644
--- a/generator/templates/dawn_proc_table.h
+++ b/generator/templates/dawn_proc_table.h
@@ -17,7 +17,6 @@
#include "dawn/webgpu.h"
-// Note: Often allocated as a static global. Do not add a complex constructor.
typedef struct DawnProcTable {
WGPUProcGetProcAddress getProcAddress;
WGPUProcCreateInstance createInstance;
diff --git a/generator/templates/dawn_thread_dispatch_proc.cpp b/generator/templates/dawn_thread_dispatch_proc.cpp
deleted file mode 100644
index bfc7794..0000000
--- a/generator/templates/dawn_thread_dispatch_proc.cpp
+++ /dev/null
@@ -1,52 +0,0 @@
-#include "dawn/dawn_thread_dispatch_proc.h"
-
-#include <thread>
-
-static DawnProcTable nullProcs;
-thread_local DawnProcTable perThreadProcs;
-
-void dawnProcSetPerThreadProcs(const DawnProcTable* procs) {
- if (procs) {
- perThreadProcs = *procs;
- } else {
- perThreadProcs = nullProcs;
- }
-}
-
-static WGPUProc ThreadDispatchGetProcAddress(WGPUDevice device, const char* procName) {
- return perThreadProcs.getProcAddress(device, procName);
-}
-
-static WGPUInstance ThreadDispatchCreateInstance(WGPUInstanceDescriptor const * descriptor) {
- return perThreadProcs.createInstance(descriptor);
-}
-
-{% for type in by_category["object"] %}
- {% for method in c_methods(type) %}
- static {{as_cType(method.return_type.name)}} ThreadDispatch{{as_MethodSuffix(type.name, method.name)}}(
- {{-as_cType(type.name)}} {{as_varName(type.name)}}
- {%- for arg in method.arguments -%}
- , {{as_annotated_cType(arg)}}
- {%- endfor -%}
- ) {
- {% if method.return_type.name.canonical_case() != "void" %}return {% endif %}
- perThreadProcs.{{as_varName(type.name, method.name)}}({{as_varName(type.name)}}
- {%- for arg in method.arguments -%}
- , {{as_varName(arg.name)}}
- {%- endfor -%}
- );
- }
- {% endfor %}
-{% endfor %}
-
-extern "C" {
- DawnProcTable dawnThreadDispatchProcTable = {
- ThreadDispatchGetProcAddress,
- ThreadDispatchCreateInstance,
-{% for type in by_category["object"] %}
- {% for method in c_methods(type) %}
- ThreadDispatch{{as_MethodSuffix(type.name, method.name)}},
- {% endfor %}
-{% endfor %}
- };
-}
diff --git a/generator/templates/dawn_wire/client/ApiProcs.cpp b/generator/templates/dawn_wire/client/ApiProcs.cpp
index 0ad9a77..3edba1a 100644
--- a/generator/templates/dawn_wire/client/ApiProcs.cpp
+++ b/generator/templates/dawn_wire/client/ApiProcs.cpp
@@ -293,16 +293,21 @@
return result;
}
- static DawnProcTable gProcTable = {
- ClientGetProcAddress,
- ClientCreateInstance,
+ //* Some commands don't have a custom wire format, but need to be handled manually to update
+ //* some client-side state tracking. For these we have two functions:
+ //* - An autogenerated Client{{suffix}} method that sends the command on the wire
+ //* - A manual ProxyClient{{suffix}} method that will be inserted in the proctable instead of
+ //* the autogenerated one, and that will have to call Client{{suffix}}
+ DawnProcTable GetProcs() {
+ DawnProcTable table;
+ table.getProcAddress = ClientGetProcAddress;
+ table.createInstance = ClientCreateInstance;
{% for type in by_category["object"] %}
{% for method in c_methods(type) %}
- Client{{as_MethodSuffix(type.name, method.name)}},
+ {% set suffix = as_MethodSuffix(type.name, method.name) %}
+ table.{{as_varName(type.name, method.name)}} = Client{{suffix}};
{% endfor %}
{% endfor %}
- };
- const DawnProcTable& GetProcs() {
- return gProcTable;
+ return table;
}
}} // namespace dawn_wire::client
diff --git a/src/dawn/BUILD.gn b/src/dawn/BUILD.gn
index ad48712..9034be4 100644
--- a/src/dawn/BUILD.gn
+++ b/src/dawn/BUILD.gn
@@ -87,10 +87,7 @@
dawn_json_generator("dawn_proc_gen") {
target = "dawn_proc"
- outputs = [
- "src/dawn/dawn_proc.c",
- "src/dawn/dawn_thread_dispatch_proc.cpp",
- ]
+ outputs = [ "src/dawn/dawn_proc.c" ]
}
dawn_component("dawn_proc") {
@@ -99,8 +96,5 @@
public_deps = [ ":dawn_headers" ]
deps = [ ":dawn_proc_gen" ]
sources = get_target_outputs(":dawn_proc_gen")
- sources += [
- "${dawn_root}/src/include/dawn/dawn_proc.h",
- "${dawn_root}/src/include/dawn/dawn_thread_dispatch_proc.h",
- ]
+ sources += [ "${dawn_root}/src/include/dawn/dawn_proc.h" ]
}
diff --git a/src/dawn_native/DawnNative.cpp b/src/dawn_native/DawnNative.cpp
index 52efa18..bfc47db 100644
--- a/src/dawn_native/DawnNative.cpp
+++ b/src/dawn_native/DawnNative.cpp
@@ -22,9 +22,9 @@
namespace dawn_native {
- const DawnProcTable& GetProcsAutogen();
+ DawnProcTable GetProcsAutogen();
- const DawnProcTable& GetProcs() {
+ DawnProcTable GetProcs() {
return GetProcsAutogen();
}
diff --git a/src/dawn_wire/WireClient.cpp b/src/dawn_wire/WireClient.cpp
index 430a55c..e6fe263 100644
--- a/src/dawn_wire/WireClient.cpp
+++ b/src/dawn_wire/WireClient.cpp
@@ -26,7 +26,7 @@
}
// static
- const DawnProcTable& WireClient::GetProcs() {
+ DawnProcTable WireClient::GetProcs() {
return client::GetProcs();
}
diff --git a/src/dawn_wire/client/Client.h b/src/dawn_wire/client/Client.h
index be70e75..d8df86d 100644
--- a/src/dawn_wire/client/Client.h
+++ b/src/dawn_wire/client/Client.h
@@ -68,7 +68,7 @@
bool mIsDisconnected = false;
};
- const DawnProcTable& GetProcs();
+ DawnProcTable GetProcs();
std::unique_ptr<MemoryTransferService> CreateInlineMemoryTransferService();
diff --git a/src/include/dawn/dawn_thread_dispatch_proc.h b/src/include/dawn/dawn_thread_dispatch_proc.h
deleted file mode 100644
index 4d08ba8..0000000
--- a/src/include/dawn/dawn_thread_dispatch_proc.h
+++ /dev/null
@@ -1,33 +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 DAWN_DAWN_THREAD_DISPATCH_PROC_H_
-#define DAWN_DAWN_THREAD_DISPATCH_PROC_H_
-
-#include "dawn/dawn_proc.h"
-
-#ifdef __cplusplus
-extern "C" {
-#endif
-
-// Call dawnProcSetProcs(&dawnThreadDispatchProcTable) and then use dawnProcSetPerThreadProcs
-// to set per-thread procs.
-WGPU_EXPORT extern DawnProcTable dawnThreadDispatchProcTable;
-WGPU_EXPORT void dawnProcSetPerThreadProcs(const DawnProcTable* procs);
-
-#ifdef __cplusplus
-} // extern "C"
-#endif
-
-#endif // DAWN_DAWN_THREAD_DISPATCH_PROC_H_
diff --git a/src/include/dawn_native/DawnNative.h b/src/include/dawn_native/DawnNative.h
index 6498646..2199efa 100644
--- a/src/include/dawn_native/DawnNative.h
+++ b/src/include/dawn_native/DawnNative.h
@@ -170,7 +170,7 @@
};
// Backend-agnostic API for dawn_native
- DAWN_NATIVE_EXPORT const DawnProcTable& GetProcs();
+ DAWN_NATIVE_EXPORT DawnProcTable GetProcs();
// Query the names of all the toggles that are enabled in device
DAWN_NATIVE_EXPORT std::vector<const char*> GetTogglesUsed(WGPUDevice device);
diff --git a/src/include/dawn_wire/WireClient.h b/src/include/dawn_wire/WireClient.h
index 50da913..815b66b 100644
--- a/src/include/dawn_wire/WireClient.h
+++ b/src/include/dawn_wire/WireClient.h
@@ -26,8 +26,6 @@
namespace client {
class Client;
class MemoryTransferService;
-
- DAWN_WIRE_EXPORT const DawnProcTable& GetProcs();
} // namespace client
struct ReservedTexture {
@@ -46,8 +44,7 @@
WireClient(const WireClientDescriptor& descriptor);
~WireClient() override;
- // TODO(enga): Remove this and use dawn_wire::client::GetProcs() instead
- static const DawnProcTable& GetProcs();
+ static DawnProcTable GetProcs();
WGPUDevice GetDevice() const;
const volatile char* HandleCommands(const volatile char* commands,
diff --git a/src/tests/BUILD.gn b/src/tests/BUILD.gn
index 9c998f5..0077695 100644
--- a/src/tests/BUILD.gn
+++ b/src/tests/BUILD.gn
@@ -168,7 +168,6 @@
"unittests/MathTests.cpp",
"unittests/ObjectBaseTests.cpp",
"unittests/PerStageTests.cpp",
- "unittests/PerThreadProcTests.cpp",
"unittests/PlacementAllocatedTests.cpp",
"unittests/RefCountedTests.cpp",
"unittests/ResultTests.cpp",
diff --git a/src/tests/DawnTest.cpp b/src/tests/DawnTest.cpp
index 945d9c1..5873da8 100644
--- a/src/tests/DawnTest.cpp
+++ b/src/tests/DawnTest.cpp
@@ -744,9 +744,12 @@
clientDesc.serializer = mC2sBuf.get();
mWireClient.reset(new dawn_wire::WireClient(clientDesc));
- cDevice = mWireClient->GetDevice();
- procs = dawn_wire::client::GetProcs();
+ WGPUDevice clientDevice = mWireClient->GetDevice();
+ DawnProcTable clientProcs = dawn_wire::WireClient::GetProcs();
mS2cBuf->SetHandler(mWireClient.get());
+
+ procs = clientProcs;
+ cDevice = clientDevice;
} else {
procs = backendProcs;
cDevice = backendDevice;
diff --git a/src/tests/end2end/WindowSurfaceTests.cpp b/src/tests/end2end/WindowSurfaceTests.cpp
index 03e954e..8aaa8cc 100644
--- a/src/tests/end2end/WindowSurfaceTests.cpp
+++ b/src/tests/end2end/WindowSurfaceTests.cpp
@@ -50,7 +50,8 @@
});
DAWN_SKIP_TEST_IF(!glfwInit());
- dawnProcSetProcs(&dawn_native::GetProcs());
+ DawnProcTable procs = dawn_native::GetProcs();
+ dawnProcSetProcs(&procs);
mInstance = wgpu::CreateInstance();
}
diff --git a/src/tests/unittests/GetProcAddressTests.cpp b/src/tests/unittests/GetProcAddressTests.cpp
index 10f9c5c..f5ac8c6 100644
--- a/src/tests/unittests/GetProcAddressTests.cpp
+++ b/src/tests/unittests/GetProcAddressTests.cpp
@@ -72,7 +72,7 @@
mWireClient = std::make_unique<dawn_wire::WireClient>(clientDesc);
mDevice = wgpu::Device::Acquire(mWireClient->GetDevice());
- mProcs = dawn_wire::client::GetProcs();
+ mProcs = dawn_wire::WireClient::GetProcs();
break;
}
diff --git a/src/tests/unittests/PerThreadProcTests.cpp b/src/tests/unittests/PerThreadProcTests.cpp
deleted file mode 100644
index 38ce981..0000000
--- a/src/tests/unittests/PerThreadProcTests.cpp
+++ /dev/null
@@ -1,118 +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/dawn_thread_dispatch_proc.h"
-#include "dawn/webgpu_cpp.h"
-#include "dawn_native/DawnNative.h"
-#include "dawn_native/Instance.h"
-#include "dawn_native/null/DeviceNull.h"
-
-#include <gtest/gtest.h>
-#include <atomic>
-#include <thread>
-
-class PerThreadProcTests : public testing::Test {
- public:
- PerThreadProcTests()
- : mNativeInstance(dawn_native::InstanceBase::Create()),
- mNativeAdapter(mNativeInstance.Get()) {
- }
- ~PerThreadProcTests() override = default;
-
- protected:
- Ref<dawn_native::InstanceBase> mNativeInstance;
- dawn_native::null::Adapter mNativeAdapter;
-};
-
-// Test that procs can be set per thread. This test overrides deviceCreateBuffer with a dummy proc
-// for each thread that increments a counter. Because each thread has their own proc and counter,
-// there should be no data races. The per-thread procs also check that the current thread id is
-// exactly equal to the expected thread id.
-TEST_F(PerThreadProcTests, DispatchesPerThread) {
- dawnProcSetProcs(&dawnThreadDispatchProcTable);
-
- // Threads will block on this atomic to be sure we set procs on both threads before
- // either thread calls the procs.
- std::atomic<bool> ready(false);
-
- static int threadACounter = 0;
- static int threadBCounter = 0;
-
- static std::atomic<std::thread::id> threadIdA;
- static std::atomic<std::thread::id> threadIdB;
-
- constexpr int kThreadATargetCount = 28347;
- constexpr int kThreadBTargetCount = 40420;
-
- // Note: Acquire doesn't call reference or release.
- wgpu::Device deviceA =
- wgpu::Device::Acquire(reinterpret_cast<WGPUDevice>(mNativeAdapter.CreateDevice(nullptr)));
-
- wgpu::Device deviceB =
- wgpu::Device::Acquire(reinterpret_cast<WGPUDevice>(mNativeAdapter.CreateDevice(nullptr)));
-
- std::thread threadA([&]() {
- DawnProcTable procs = dawn_native::GetProcs();
- procs.deviceCreateBuffer = [](WGPUDevice device,
- WGPUBufferDescriptor const* descriptor) -> WGPUBuffer {
- EXPECT_EQ(std::this_thread::get_id(), threadIdA);
- threadACounter++;
- return nullptr;
- };
- dawnProcSetPerThreadProcs(&procs);
-
- while (!ready) {
- } // Should be fast, so just spin.
-
- for (int i = 0; i < kThreadATargetCount; ++i) {
- deviceA.CreateBuffer(nullptr);
- }
-
- deviceA = nullptr;
- dawnProcSetPerThreadProcs(nullptr);
- });
-
- std::thread threadB([&]() {
- DawnProcTable procs = dawn_native::GetProcs();
- procs.deviceCreateBuffer = [](WGPUDevice device,
- WGPUBufferDescriptor const* bufferDesc) -> WGPUBuffer {
- EXPECT_EQ(std::this_thread::get_id(), threadIdB);
- threadBCounter++;
- return nullptr;
- };
- dawnProcSetPerThreadProcs(&procs);
-
- while (!ready) {
- } // Should be fast, so just spin.
-
- for (int i = 0; i < kThreadBTargetCount; ++i) {
- deviceB.CreateBuffer(nullptr);
- }
-
- deviceB = nullptr;
- dawnProcSetPerThreadProcs(nullptr);
- });
-
- threadIdA = threadA.get_id();
- threadIdB = threadB.get_id();
-
- ready = true;
- threadA.join();
- threadB.join();
-
- EXPECT_EQ(threadACounter, kThreadATargetCount);
- EXPECT_EQ(threadBCounter, kThreadBTargetCount);
-
- dawnProcSetProcs(nullptr);
-}
diff --git a/src/tests/unittests/validation/ValidationTest.cpp b/src/tests/unittests/validation/ValidationTest.cpp
index 1713171..058fd3b 100644
--- a/src/tests/unittests/validation/ValidationTest.cpp
+++ b/src/tests/unittests/validation/ValidationTest.cpp
@@ -40,7 +40,8 @@
ASSERT(foundNullAdapter);
- dawnProcSetProcs(&dawn_native::GetProcs());
+ DawnProcTable procs = dawn_native::GetProcs();
+ dawnProcSetProcs(&procs);
device = CreateDeviceFromAdapter(adapter, std::vector<const char*>());
}
diff --git a/src/tests/unittests/wire/WireMultipleDeviceTests.cpp b/src/tests/unittests/wire/WireMultipleDeviceTests.cpp
index b442794..3ba0da0 100644
--- a/src/tests/unittests/wire/WireMultipleDeviceTests.cpp
+++ b/src/tests/unittests/wire/WireMultipleDeviceTests.cpp
@@ -29,7 +29,8 @@
class WireMultipleDeviceTests : public testing::Test {
protected:
void SetUp() override {
- dawnProcSetProcs(&dawn_wire::client::GetProcs());
+ DawnProcTable procs = dawn_wire::WireClient::GetProcs();
+ dawnProcSetProcs(&procs);
}
void TearDown() override {
diff --git a/src/tests/unittests/wire/WireTest.cpp b/src/tests/unittests/wire/WireTest.cpp
index d23709c..bdac99f 100644
--- a/src/tests/unittests/wire/WireTest.cpp
+++ b/src/tests/unittests/wire/WireTest.cpp
@@ -66,7 +66,8 @@
mS2cBuf->SetHandler(mWireClient.get());
device = mWireClient->GetDevice();
- dawnProcSetProcs(&dawn_wire::client::GetProcs());
+ DawnProcTable clientProcs = dawn_wire::WireClient::GetProcs();
+ dawnProcSetProcs(&clientProcs);
apiDevice = mockDevice;
