From bf27bd7a5e612bbdc65973ab6309f48c7f069c04 Mon Sep 17 00:00:00 2001
From: Tomek Ponitka <tommek@google.com>
Date: Mon, 29 Jun 2020 11:13:43 +0000
Subject: [PATCH] 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   | 20 ++++++++++++++++++++
 src/dawn_native/Device.h     |  4 ++++
 src/dawn_native/Pipeline.cpp | 13 ++-----------
 3 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/src/dawn_native/Device.cpp b/src/dawn_native/Device.cpp
index 1c30cd509c..b0ef00db13 100644
--- a/src/dawn_native/Device.cpp
+++ b/src/dawn_native/Device.cpp
@@ -169,6 +169,8 @@ namespace dawn_native {
         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 @@ namespace dawn_native {
         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);
diff --git a/src/dawn_native/Device.h b/src/dawn_native/Device.h
index 94d916f7c2..f9a3b43aba 100644
--- a/src/dawn_native/Device.h
+++ b/src/dawn_native/Device.h
@@ -112,6 +112,8 @@ namespace dawn_native {
             const BindGroupLayoutDescriptor* descriptor);
         void UncacheBindGroupLayout(BindGroupLayoutBase* obj);
 
+        ResultOrError<BindGroupLayoutBase*> GetOrCreateEmptyBindGroupLayout();
+
         ResultOrError<ComputePipelineBase*> GetOrCreateComputePipeline(
             const ComputePipelineDescriptor* descriptor);
         void UncacheComputePipeline(ComputePipelineBase* obj);
@@ -340,6 +342,8 @@ namespace dawn_native {
         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;
diff --git a/src/dawn_native/Pipeline.cpp b/src/dawn_native/Pipeline.cpp
index df344416a5..15dd06838d 100644
--- a/src/dawn_native/Pipeline.cpp
+++ b/src/dawn_native/Pipeline.cpp
@@ -91,19 +91,10 @@ namespace dawn_native {
         }
 
         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;