From f413d4d084269b2a32c6f27754617e26ad616d4f Mon Sep 17 00:00:00 2001
From: baldurk <baldurk@baldurk.org>
Date: Thu, 13 Apr 2023 11:31:53 +0100
Subject: [PATCH] Defer memory frees onto a thread

---
 renderdoc/driver/vulkan/vk_core.h             |  1 +
 renderdoc/driver/vulkan/vk_memory.cpp         | 25 ++++++++++++++-----
 .../vulkan/wrappers/vk_device_funcs.cpp       | 14 +++++++++++
 3 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/renderdoc/driver/vulkan/vk_core.h b/renderdoc/driver/vulkan/vk_core.h
index 2404088bf..efb915027 100644
--- a/renderdoc/driver/vulkan/vk_core.h
+++ b/renderdoc/driver/vulkan/vk_core.h
@@ -583,6 +583,7 @@ private:
   // all 'base' allocations. The offset is used to indicate the current offset, and the size is the
   // total size, thus the free space can be determined with size - offset.
   rdcarray<MemoryAllocation> m_MemoryBlocks[arraydim<MemoryScope>()];
+  Threading::ThreadHandle m_MemoryFreeThread = 0;
 
   // Per memory scope, the size of the next allocation. This allows us to balance number of memory
   // allocation objects with size by incrementally allocating larger blocks.
diff --git a/renderdoc/driver/vulkan/vk_memory.cpp b/renderdoc/driver/vulkan/vk_memory.cpp
index 9f6db8095..b48aadd5c 100644
--- a/renderdoc/driver/vulkan/vk_memory.cpp
+++ b/renderdoc/driver/vulkan/vk_memory.cpp
@@ -408,15 +408,28 @@ void WrappedVulkan::FreeAllMemory(MemoryScope scope)
   if(allocList.empty())
     return;
 
-  VkDevice d = GetDev();
-
-  for(MemoryAllocation alloc : allocList)
+  // freeing a lot of memory can take a while on some implementations. Since this only needs to
+  // externally synchronise the memory we do it on a thread and synchronise if we need to free again
+  // or on device shutdown
+  if(m_MemoryFreeThread)
   {
-    ObjDisp(d)->FreeMemory(Unwrap(d), Unwrap(alloc.mem), NULL);
-    GetResourceManager()->ReleaseWrappedResource(alloc.mem);
+    Threading::JoinThread(m_MemoryFreeThread);
+    Threading::CloseThread(m_MemoryFreeThread);
+    m_MemoryFreeThread = 0;
   }
 
-  allocList.clear();
+  VkDevice d = GetDev();
+
+  rdcarray<MemoryAllocation> allocs;
+  allocs.swap(allocList);
+
+  m_MemoryFreeThread = Threading::CreateThread([this, d, allocs]() {
+    for(const MemoryAllocation &alloc : allocs)
+    {
+      ObjDisp(d)->FreeMemory(Unwrap(d), Unwrap(alloc.mem), NULL);
+      GetResourceManager()->ReleaseWrappedResource(alloc.mem);
+    }
+  });
 }
 
 void WrappedVulkan::FreeMemoryAllocation(MemoryAllocation alloc)
diff --git a/renderdoc/driver/vulkan/wrappers/vk_device_funcs.cpp b/renderdoc/driver/vulkan/wrappers/vk_device_funcs.cpp
index 364218c5b..29e72b10a 100644
--- a/renderdoc/driver/vulkan/wrappers/vk_device_funcs.cpp
+++ b/renderdoc/driver/vulkan/wrappers/vk_device_funcs.cpp
@@ -955,6 +955,13 @@ void WrappedVulkan::Shutdown()
 
   FreeAllMemory(MemoryScope::InitialContents);
 
+  if(m_MemoryFreeThread)
+  {
+    Threading::JoinThread(m_MemoryFreeThread);
+    Threading::CloseThread(m_MemoryFreeThread);
+    m_MemoryFreeThread = 0;
+  }
+
   // we do more in Shutdown than the equivalent vkDestroyInstance since on replay there's
   // no explicit vkDestroyDevice, we destroy the device here then the instance
 
@@ -4237,6 +4244,13 @@ void WrappedVulkan::vkDestroyDevice(VkDevice device, const VkAllocationCallbacks
   if(device == VK_NULL_HANDLE)
     return;
 
+  if(m_MemoryFreeThread)
+  {
+    Threading::JoinThread(m_MemoryFreeThread);
+    Threading::CloseThread(m_MemoryFreeThread);
+    m_MemoryFreeThread = 0;
+  }
+
   // flush out any pending commands/semaphores
   SubmitCmds();
   SubmitSemaphores();