Vulkan: Centralized deferred deletion, use it for BufferVk's handle

This introduce a new FencedDeleter service as part of the Device
objects that tracks when resources are no longer used and deletes them.
BufferVk takes advantage of this to defer the deletion of its handle
that was previously incorrectly delete directly in ~BufferVk.

src/backend/CMakeLists.txt

@@ -293,6 +293,8 @@ if (NXT_ENABLE_VULKAN)
         ${VULKAN_DIR}/BufferUploader.h
         ${VULKAN_DIR}/BufferVk.cpp
         ${VULKAN_DIR}/BufferVk.h
+        ${VULKAN_DIR}/FencedDeleter.cpp
+        ${VULKAN_DIR}/FencedDeleter.h
         ${VULKAN_DIR}/MemoryAllocator.cpp
         ${VULKAN_DIR}/MemoryAllocator.h
         ${VULKAN_DIR}/VulkanBackend.cpp

src/backend/vulkan/BufferUploader.cpp

@@ -14,6 +14,7 @@
 
 #include "backend/vulkan/BufferUploader.h"
 
+#include "backend/vulkan/FencedDeleter.h"
 #include "backend/vulkan/MemoryAllocator.h"
 #include "backend/vulkan/VulkanBackend.h"
 
@@ -25,7 +26,6 @@ namespace backend { namespace vulkan {
     }
 
     BufferUploader::~BufferUploader() {
-        ASSERT(mBuffersToDelete.Empty());
     }
 
     void BufferUploader::BufferSubData(VkBuffer buffer,
@@ -93,14 +93,10 @@ namespace backend { namespace vulkan {
         // TODO(cwallez@chromium.org): Buffers must be deleted before the memory.
         // This happens to work for now, but is fragile.
         mDevice->GetMemoryAllocator()->Free(&allocation);
-        mBuffersToDelete.Enqueue(stagingBuffer, mDevice->GetSerial());
+        mDevice->GetFencedDeleter()->DeleteWhenUnused(stagingBuffer);
     }
 
-    void BufferUploader::Tick(Serial completedSerial) {
+    void BufferUploader::Tick(Serial) {
-        for (VkBuffer buffer : mBuffersToDelete.IterateUpTo(completedSerial)) {
-            mDevice->fn.DestroyBuffer(mDevice->GetVkDevice(), buffer, nullptr);
-        }
-        mBuffersToDelete.ClearUpTo(completedSerial);
     }
 
 }}  // namespace backend::vulkan

src/backend/vulkan/BufferUploader.h

@@ -36,7 +36,6 @@ namespace backend { namespace vulkan {
 
       private:
         Device* mDevice = nullptr;
-        SerialQueue<VkBuffer> mBuffersToDelete;
     };
 
 }}  // namespace backend::vulkan

src/backend/vulkan/BufferVk.cpp

@@ -15,6 +15,7 @@
 #include "backend/vulkan/BufferVk.h"
 
 #include "backend/vulkan/BufferUploader.h"
+#include "backend/vulkan/FencedDeleter.h"
 #include "backend/vulkan/VulkanBackend.h"
 
 #include <cstring>
@@ -92,7 +93,7 @@ namespace backend { namespace vulkan {
         device->GetMemoryAllocator()->Free(&mMemoryAllocation);
 
         if (mHandle != VK_NULL_HANDLE) {
-            device->fn.DestroyBuffer(device->GetVkDevice(), mHandle, nullptr);
+            device->GetFencedDeleter()->DeleteWhenUnused(mHandle);
             mHandle = VK_NULL_HANDLE;
         }
     }

src/backend/vulkan/FencedDeleter.cpp

@@ -0,0 +1,51 @@
+// Copyright 2017 The NXT 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 "backend/vulkan/FencedDeleter.h"
+
+#include "backend/vulkan/VulkanBackend.h"
+
+namespace backend { namespace vulkan {
+
+    FencedDeleter::FencedDeleter(Device* device) : mDevice(device) {
+    }
+
+    FencedDeleter::~FencedDeleter() {
+        ASSERT(mBuffersToDelete.Empty());
+        ASSERT(mMemoriesToDelete.Empty());
+    }
+
+    void FencedDeleter::DeleteWhenUnused(VkBuffer buffer) {
+        mBuffersToDelete.Enqueue(buffer, mDevice->GetSerial());
+    }
+
+    void FencedDeleter::DeleteWhenUnused(VkDeviceMemory memory) {
+        mMemoriesToDelete.Enqueue(memory, mDevice->GetSerial());
+    }
+
+    void FencedDeleter::Tick(Serial completedSerial) {
+        // Buffers and textures must be deleted before memories because it is invalid to free memory
+        // that still have resources bound to it.
+        for (VkBuffer buffer : mBuffersToDelete.IterateUpTo(completedSerial)) {
+            mDevice->fn.DestroyBuffer(mDevice->GetVkDevice(), buffer, nullptr);
+        }
+        mBuffersToDelete.ClearUpTo(completedSerial);
+
+        for (VkDeviceMemory memory : mMemoriesToDelete.IterateUpTo(completedSerial)) {
+            mDevice->fn.FreeMemory(mDevice->GetVkDevice(), memory, nullptr);
+        }
+        mMemoriesToDelete.ClearUpTo(completedSerial);
+    }
+
+}}  // namespace backend::vulkan

src/backend/vulkan/FencedDeleter.h

@@ -0,0 +1,43 @@
+// Copyright 2017 The NXT 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 BACKEND_VULKAN_FENCEDDELETER_H_
+#define BACKEND_VULKAN_FENCEDDELETER_H_
+
+#include "backend/vulkan/vulkan_platform.h"
+#include "common/SerialQueue.h"
+
+namespace backend { namespace vulkan {
+
+    class Device;
+
+    class FencedDeleter {
+      public:
+        FencedDeleter(Device* device);
+        ~FencedDeleter();
+
+        void DeleteWhenUnused(VkBuffer buffer);
+        void DeleteWhenUnused(VkDeviceMemory memory);
+
+        void Tick(Serial completedSerial);
+
+      private:
+        Device* mDevice = nullptr;
+        SerialQueue<VkBuffer> mBuffersToDelete;
+        SerialQueue<VkDeviceMemory> mMemoriesToDelete;
+    };
+
+}}  // namespace backend::vulkan
+
+#endif  // BACKEND_VULKAN_FENCEDDELETER_H_

src/backend/vulkan/MemoryAllocator.cpp

@@ -13,6 +13,8 @@
 // limitations under the License.
 
 #include "backend/vulkan/MemoryAllocator.h"
+
+#include "backend/vulkan/FencedDeleter.h"
 #include "backend/vulkan/VulkanBackend.h"
 
 namespace backend { namespace vulkan {
@@ -37,7 +39,6 @@ namespace backend { namespace vulkan {
     }
 
     MemoryAllocator::~MemoryAllocator() {
-        ASSERT(mReleasedMemory.Empty());
     }
 
     bool MemoryAllocator::Allocate(VkMemoryRequirements requirements,
@@ -121,16 +122,12 @@ namespace backend { namespace vulkan {
     }
 
     void MemoryAllocator::Free(DeviceMemoryAllocation* allocation) {
-        mReleasedMemory.Enqueue(allocation->mMemory, mDevice->GetSerial());
+        mDevice->GetFencedDeleter()->DeleteWhenUnused(allocation->mMemory);
         allocation->mMemory = VK_NULL_HANDLE;
         allocation->mOffset = 0;
         allocation->mMappedPointer = nullptr;
     }
 
-    void MemoryAllocator::Tick(Serial finishedSerial) {
+    void MemoryAllocator::Tick(Serial) {
-        for (auto memory : mReleasedMemory.IterateUpTo(finishedSerial)) {
-            mDevice->fn.FreeMemory(mDevice->GetVkDevice(), memory, nullptr);
-        }
-        mReleasedMemory.ClearUpTo(finishedSerial);
     }
 }}  // namespace backend::vulkan

src/backend/vulkan/MemoryAllocator.h

@@ -51,7 +51,6 @@ namespace backend { namespace vulkan {
 
       private:
         Device* mDevice = nullptr;
-        SerialQueue<VkDeviceMemory> mReleasedMemory;
     };
 
 }}  // namespace backend::vulkan

src/backend/vulkan/VulkanBackend.cpp

@@ -17,6 +17,7 @@
 #include "backend/Commands.h"
 #include "backend/vulkan/BufferUploader.h"
 #include "backend/vulkan/BufferVk.h"
+#include "backend/vulkan/FencedDeleter.h"
 #include "common/Platform.h"
 
 #include <spirv-cross/spirv_cross.hpp>
@@ -107,9 +108,10 @@ namespace backend { namespace vulkan {
 
         GatherQueueFromDevice();
 
+        mBufferUploader = new BufferUploader(this);
+        mDeleter = new FencedDeleter(this);
         mMapReadRequestTracker = new MapReadRequestTracker(this);
         mMemoryAllocator = new MemoryAllocator(this);
-        mBufferUploader = new BufferUploader(this);
     }
 
     Device::~Device() {
@@ -139,20 +141,17 @@ namespace backend { namespace vulkan {
         }
         mUnusedFences.clear();
 
-        if (mBufferUploader) {
+        delete mBufferUploader;
-            delete mBufferUploader;
+        mBufferUploader = nullptr;
-            mBufferUploader = nullptr;
-        }
 
-        if (mMemoryAllocator) {
+        delete mDeleter;
-            delete mMemoryAllocator;
+        mDeleter = nullptr;
-            mMemoryAllocator = nullptr;
-        }
 
-        if (mMapReadRequestTracker) {
+        delete mMapReadRequestTracker;
-            delete mMapReadRequestTracker;
+        mMapReadRequestTracker = nullptr;
-            mMapReadRequestTracker = nullptr;
+
-        }
+        delete mMemoryAllocator;
+        mMemoryAllocator = nullptr;
 
         // VkQueues are destroyed when the VkDevice is destroyed
         if (mVkDevice != VK_NULL_HANDLE) {
@@ -243,6 +242,8 @@ namespace backend { namespace vulkan {
         mBufferUploader->Tick(mCompletedSerial);
         mMemoryAllocator->Tick(mCompletedSerial);
 
+        mDeleter->Tick(mCompletedSerial);
+
         if (mPendingCommands.pool != VK_NULL_HANDLE) {
             SubmitPendingCommands();
         }
@@ -264,6 +265,10 @@ namespace backend { namespace vulkan {
         return mBufferUploader;
     }
 
+    FencedDeleter* Device::GetFencedDeleter() const {
+        return mDeleter;
+    }
+
     Serial Device::GetSerial() const {
         return mNextSerial;
     }

src/backend/vulkan/VulkanBackend.h

@@ -66,9 +66,10 @@ namespace backend { namespace vulkan {
     class Texture;
     using TextureView = TextureViewBase;
 
+    class BufferUploader;
+    class FencedDeleter;
     class MapReadRequestTracker;
     class MemoryAllocator;
-    class BufferUploader;
 
     struct VulkanBackendTraits {
         using BindGroupType = BindGroup;
@@ -103,6 +104,24 @@ namespace backend { namespace vulkan {
         Device();
         ~Device();
 
+        // Contains all the Vulkan entry points, vkDoFoo is called via device->fn.DoFoo.
+        const VulkanFunctions fn;
+
+        const VulkanDeviceInfo& GetDeviceInfo() const;
+        VkInstance GetInstance() const;
+        VkDevice GetVkDevice() const;
+
+        BufferUploader* GetBufferUploader() const;
+        FencedDeleter* GetFencedDeleter() const;
+        MapReadRequestTracker* GetMapReadRequestTracker() const;
+        MemoryAllocator* GetMemoryAllocator() const;
+
+        Serial GetSerial() const;
+
+        VkCommandBuffer GetPendingCommandBuffer();
+        void SubmitPendingCommands();
+
+        // NXT API
         BindGroupBase* CreateBindGroup(BindGroupBuilder* builder) override;
         BindGroupLayoutBase* CreateBindGroupLayout(BindGroupLayoutBuilder* builder) override;
         BlendStateBase* CreateBlendState(BlendStateBuilder* builder) override;
@@ -125,22 +144,6 @@ namespace backend { namespace vulkan {
 
         void TickImpl() override;
 
-        const VulkanDeviceInfo& GetDeviceInfo() const;
-        MapReadRequestTracker* GetMapReadRequestTracker() const;
-        MemoryAllocator* GetMemoryAllocator() const;
-        BufferUploader* GetBufferUploader() const;
-
-        Serial GetSerial() const;
-
-        VkCommandBuffer GetPendingCommandBuffer();
-        void SubmitPendingCommands();
-
-        // Contains all the Vulkan entry points, vkDoFoo is called via device->fn.DoFoo.
-        const VulkanFunctions fn;
-
-        VkInstance GetInstance() const;
-        VkDevice GetVkDevice() const;
-
       private:
         bool CreateInstance(VulkanGlobalKnobs* usedKnobs);
         bool CreateDevice(VulkanDeviceKnobs* usedKnobs);
@@ -173,9 +176,10 @@ namespace backend { namespace vulkan {
         VkQueue mQueue = VK_NULL_HANDLE;
         VkDebugReportCallbackEXT mDebugReportCallback = VK_NULL_HANDLE;
 
+        BufferUploader* mBufferUploader = nullptr;
+        FencedDeleter* mDeleter = nullptr;
         MapReadRequestTracker* mMapReadRequestTracker = nullptr;
         MemoryAllocator* mMemoryAllocator = nullptr;
-        BufferUploader* mBufferUploader = nullptr;
 
         VkFence GetUnusedFence();
         void CheckPassedFences();