Revert "Add a per-thread proc table using thread local storage"

This reverts commit b04a92f01b.

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>

generator/dawn_json_generator.py

@@ -687,10 +687,6 @@ class MultiGeneratorFromDawnJSON(Generator):
             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(

generator/templates/dawn_native/ProcTable.cpp

@@ -135,17 +135,16 @@ namespace dawn_native {
         return result;
     }
 
-    static DawnProcTable gProcTable = {
+    DawnProcTable GetProcsAutogen() {
-        NativeGetProcAddress,
+        DawnProcTable table;
-        NativeCreateInstance,
+        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 %}
-    };
+        return table;
-
-    const DawnProcTable& GetProcsAutogen() {
-        return gProcTable;
     }
+
 }

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;

generator/templates/dawn_thread_dispatch_proc.cpp

@@ -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 %}
-    };
-}

generator/templates/dawn_wire/client/ApiProcs.cpp

@@ -293,16 +293,21 @@ namespace dawn_wire { namespace client {
         return result;
     }
 
-    static DawnProcTable gProcTable = {
+    //* Some commands don't have a custom wire format, but need to be handled manually to update
-        ClientGetProcAddress,
+    //* some client-side state tracking. For these we have two functions:
-        ClientCreateInstance,
+    //*  - 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 %}
-    };
+        return table;
-    const DawnProcTable& GetProcs() {
-        return gProcTable;
     }
 }}  // namespace dawn_wire::client

src/dawn/BUILD.gn

@@ -87,10 +87,7 @@ source_set("dawncpp") {
 
 dawn_json_generator("dawn_proc_gen") {
   target = "dawn_proc"
-  outputs = [
+  outputs = [ "src/dawn/dawn_proc.c" ]
-    "src/dawn/dawn_proc.c",
-    "src/dawn/dawn_thread_dispatch_proc.cpp",
-  ]
 }
 
 dawn_component("dawn_proc") {
@@ -99,8 +96,5 @@ dawn_component("dawn_proc") {
   public_deps = [ ":dawn_headers" ]
   deps = [ ":dawn_proc_gen" ]
   sources = get_target_outputs(":dawn_proc_gen")
-  sources += [
+  sources += [ "${dawn_root}/src/include/dawn/dawn_proc.h" ]
-    "${dawn_root}/src/include/dawn/dawn_proc.h",
-    "${dawn_root}/src/include/dawn/dawn_thread_dispatch_proc.h",
-  ]
 }

src/dawn_native/DawnNative.cpp

@@ -22,9 +22,9 @@
 
 namespace dawn_native {
 
-    const DawnProcTable& GetProcsAutogen();
+    DawnProcTable GetProcsAutogen();
 
-    const DawnProcTable& GetProcs() {
+    DawnProcTable GetProcs() {
         return GetProcsAutogen();
     }
 

src/dawn_wire/WireClient.cpp

@@ -26,7 +26,7 @@ namespace dawn_wire {
     }
 
     // static
-    const DawnProcTable& WireClient::GetProcs() {
+    DawnProcTable WireClient::GetProcs() {
         return client::GetProcs();
     }
 

src/dawn_wire/client/Client.h

@@ -68,7 +68,7 @@ namespace dawn_wire { namespace client {
         bool mIsDisconnected = false;
     };
 
-    const DawnProcTable& GetProcs();
+    DawnProcTable GetProcs();
 
     std::unique_ptr<MemoryTransferService> CreateInlineMemoryTransferService();
 

src/include/dawn/dawn_thread_dispatch_proc.h

@@ -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_

src/include/dawn_native/DawnNative.h

@@ -170,7 +170,7 @@ namespace dawn_native {
     };
 
     // 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);

src/include/dawn_wire/WireClient.h

@@ -26,8 +26,6 @@ namespace dawn_wire {
     namespace client {
         class Client;
         class MemoryTransferService;
-
-        DAWN_WIRE_EXPORT const DawnProcTable& GetProcs();
     }  // namespace client
 
     struct ReservedTexture {
@@ -46,8 +44,7 @@ namespace dawn_wire {
         WireClient(const WireClientDescriptor& descriptor);
         ~WireClient() override;
 
-        // TODO(enga): Remove this and use dawn_wire::client::GetProcs() instead
+        static DawnProcTable GetProcs();
-        static const DawnProcTable& GetProcs();
 
         WGPUDevice GetDevice() const;
         const volatile char* HandleCommands(const volatile char* commands,

src/tests/BUILD.gn

@@ -168,7 +168,6 @@ test("dawn_unittests") {
     "unittests/MathTests.cpp",
     "unittests/ObjectBaseTests.cpp",
     "unittests/PerStageTests.cpp",
-    "unittests/PerThreadProcTests.cpp",
     "unittests/PlacementAllocatedTests.cpp",
     "unittests/RefCountedTests.cpp",
     "unittests/ResultTests.cpp",

src/tests/DawnTest.cpp

@@ -744,9 +744,12 @@ void DawnTestBase::SetUp() {
         clientDesc.serializer = mC2sBuf.get();
 
         mWireClient.reset(new dawn_wire::WireClient(clientDesc));
-        cDevice = mWireClient->GetDevice();
+        WGPUDevice clientDevice = mWireClient->GetDevice();
-        procs = dawn_wire::client::GetProcs();
+        DawnProcTable clientProcs = dawn_wire::WireClient::GetProcs();
         mS2cBuf->SetHandler(mWireClient.get());
+
+        procs = clientProcs;
+        cDevice = clientDevice;
     } else {
         procs = backendProcs;
         cDevice = backendDevice;

src/tests/end2end/WindowSurfaceTests.cpp

@@ -50,7 +50,8 @@ class WindowSurfaceInstanceTests : public testing::Test {
         });
         DAWN_SKIP_TEST_IF(!glfwInit());
 
-        dawnProcSetProcs(&dawn_native::GetProcs());
+        DawnProcTable procs = dawn_native::GetProcs();
+        dawnProcSetProcs(&procs);
 
         mInstance = wgpu::CreateInstance();
     }

src/tests/unittests/GetProcAddressTests.cpp

@@ -72,7 +72,7 @@ namespace {
                     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;
                 }
 

src/tests/unittests/PerThreadProcTests.cpp

@@ -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);
-}

src/tests/unittests/validation/ValidationTest.cpp

@@ -40,7 +40,8 @@ ValidationTest::ValidationTest() {
 
     ASSERT(foundNullAdapter);
 
-    dawnProcSetProcs(&dawn_native::GetProcs());
+    DawnProcTable procs = dawn_native::GetProcs();
+    dawnProcSetProcs(&procs);
 
     device = CreateDeviceFromAdapter(adapter, std::vector<const char*>());
 }

src/tests/unittests/wire/WireMultipleDeviceTests.cpp

@@ -29,7 +29,8 @@ using namespace dawn_wire;
 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 {

src/tests/unittests/wire/WireTest.cpp

@@ -66,7 +66,8 @@ void WireTest::SetUp() {
     mS2cBuf->SetHandler(mWireClient.get());
 
     device = mWireClient->GetDevice();
-    dawnProcSetProcs(&dawn_wire::client::GetProcs());
+    DawnProcTable clientProcs = dawn_wire::WireClient::GetProcs();
+    dawnProcSetProcs(&clientProcs);
 
     apiDevice = mockDevice;