commit bf27bd7a5e612bbdc65973ab6309f48c7f069c04
author Tomek Ponitka <tommek@google.com> Mon Jun 29 11:13:43 2020 +0000
committer Commit Bot service account <commit-bot@chromium.org> Mon Jun 29 11:13:43 2020 +0000
tree 1203e5637af504af8321b31bd7be251bedf4cc66
parent b7ece23ffceb7b889e61db2bea04b3ccc4c6d222

Caching empty bind group layouts

Added Device::GetOrCreateEmptyBindGroupLayout which caches the result
after being run and modified PipelineBase::getBindGroupLayout to use
that instead of creating a new empty bind group layout each time.

Bug: dawn:466
Change-Id: I70a5719265784eb8f60c2eedf6db7596462b21bd
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/23980
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Tomek Ponitka <tommek@google.com>

src/dawn_native/Device.cpp

diff --git a/src/dawn_native/Device.cpp b/src/dawn_native/Device.cpp
index 1c30cd5..b0ef00d 100644
--- a/src/dawn_native/Device.cpp
+++ b/src/dawn_native/Device.cpp
@@ -169,6 +169,8 @@
         mDynamicUploader = nullptr;
         mMapRequestTracker = nullptr;
 
+        mEmptyBindGroupLayout = nullptr;
+
         AssumeCommandsComplete();
         // Tell the backend that it can free all the objects now that the GPU timeline is empty.
         ShutDownImpl();
@@ -411,6 +413,24 @@
         ASSERT(removedCount == 1);
     }
 
+    ResultOrError<BindGroupLayoutBase*> DeviceBase::GetOrCreateEmptyBindGroupLayout() {
+        if (!mEmptyBindGroupLayout) {
+            BindGroupLayoutDescriptor desc = {};
+            desc.entryCount = 0;
+            desc.entries = nullptr;
+
+            BindGroupLayoutBase* bgl = nullptr;
+            if (ConsumedError(GetOrCreateBindGroupLayout(&desc), &bgl)) {
+                return BindGroupLayoutBase::MakeError(this);
+            }
+            mEmptyBindGroupLayout = bgl;
+            return bgl;
+        } else {
+            mEmptyBindGroupLayout->Reference();
+            return mEmptyBindGroupLayout.Get();
+        }
+    }
+
     ResultOrError<ComputePipelineBase*> DeviceBase::GetOrCreateComputePipeline(
         const ComputePipelineDescriptor* descriptor) {
         ComputePipelineBase blueprint(this, descriptor);

src/dawn_native/Device.h

diff --git a/src/dawn_native/Device.h b/src/dawn_native/Device.h
index 94d916f..f9a3b43 100644
--- a/src/dawn_native/Device.h
+++ b/src/dawn_native/Device.h
@@ -112,6 +112,8 @@
             const BindGroupLayoutDescriptor* descriptor);
         void UncacheBindGroupLayout(BindGroupLayoutBase* obj);
 
+        ResultOrError<BindGroupLayoutBase*> GetOrCreateEmptyBindGroupLayout();
+
         ResultOrError<ComputePipelineBase*> GetOrCreateComputePipeline(
             const ComputePipelineDescriptor* descriptor);
         void UncacheComputePipeline(ComputePipelineBase* obj);
@@ -340,6 +342,8 @@
         struct Caches;
         std::unique_ptr<Caches> mCaches;
 
+        Ref<BindGroupLayoutBase> mEmptyBindGroupLayout;
+
         std::unique_ptr<DynamicUploader> mDynamicUploader;
         std::unique_ptr<ErrorScopeTracker> mErrorScopeTracker;
         std::unique_ptr<FenceSignalTracker> mFenceSignalTracker;

src/dawn_native/Pipeline.cpp

diff --git a/src/dawn_native/Pipeline.cpp b/src/dawn_native/Pipeline.cpp
index df34441..15dd068 100644
--- a/src/dawn_native/Pipeline.cpp
+++ b/src/dawn_native/Pipeline.cpp
@@ -91,19 +91,10 @@
         }
 
         BindGroupIndex groupIndex(groupIndexIn);
-        if (!mLayout->GetBindGroupLayoutsMask()[groupIndex]) {
-            // Get or create an empty bind group layout.
-            // TODO(enga): Consider caching this object on the Device and reusing it.
-            // Today, this can't be done correctly because of the order of Device destruction.
-            // For example, vulkan::~Device will be called before ~DeviceBase. If DeviceBase owns
-            // a Ref<BindGroupLayoutBase>, then the VkDevice will be destroyed before the
-            // VkDescriptorSetLayout.
-            BindGroupLayoutDescriptor desc = {};
-            desc.entryCount = 0;
-            desc.entries = nullptr;
 
+        if (!mLayout->GetBindGroupLayoutsMask()[groupIndex]) {
             BindGroupLayoutBase* bgl = nullptr;
-            if (GetDevice()->ConsumedError(GetDevice()->GetOrCreateBindGroupLayout(&desc), &bgl)) {
+            if (GetDevice()->ConsumedError(GetDevice()->GetOrCreateEmptyBindGroupLayout(), &bgl)) {
                 return BindGroupLayoutBase::MakeError(GetDevice());
             }
             return bgl;