From f301035ba082a411c95d4c6053b117cfeccf1b40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 12 Oct 2023 23:28:44 +0200 Subject: [PATCH 1/2] Step 1 --- Common/GPU/Vulkan/VulkanDescSet.cpp | 10 +++++-- Common/GPU/Vulkan/VulkanDescSet.h | 4 +++ Common/GPU/Vulkan/VulkanRenderManager.cpp | 36 +++++++++++++---------- 3 files changed, 33 insertions(+), 17 deletions(-) diff --git a/Common/GPU/Vulkan/VulkanDescSet.cpp b/Common/GPU/Vulkan/VulkanDescSet.cpp index 906fc22099..165c8885c7 100644 --- a/Common/GPU/Vulkan/VulkanDescSet.cpp +++ b/Common/GPU/Vulkan/VulkanDescSet.cpp @@ -85,8 +85,6 @@ void VulkanDescSetPool::Reset() { } void VulkanDescSetPool::Destroy() { - _assert_msg_(vulkan_ != nullptr, "VulkanDescSetPool::Destroy without VulkanContext"); - if (descPool_ != VK_NULL_HANDLE) { vulkan_->Delete().QueueDeleteDescriptorPool(descPool_); usage_ = 0; @@ -94,6 +92,14 @@ void VulkanDescSetPool::Destroy() { sizes_.clear(); } +void VulkanDescSetPool::DestroyImmediately() { + if (descPool_ != VK_NULL_HANDLE) { + vkDestroyDescriptorPool(vulkan_->GetDevice(), descPool_, nullptr); + usage_ = 0; + } + sizes_.clear(); +} + VkResult VulkanDescSetPool::Recreate(bool grow) { _assert_msg_(vulkan_ != nullptr, "VulkanDescSetPool::Recreate without VulkanContext"); diff --git a/Common/GPU/Vulkan/VulkanDescSet.h b/Common/GPU/Vulkan/VulkanDescSet.h index 014d3aeaaa..9133f99f94 100644 --- a/Common/GPU/Vulkan/VulkanDescSet.h +++ b/Common/GPU/Vulkan/VulkanDescSet.h @@ -25,7 +25,11 @@ public: // Use only for the current frame. bool Allocate(VkDescriptorSet *descriptorSets, int count, const VkDescriptorSetLayout *layouts); void Reset(); + + // This queues up destruction. void Destroy(); + // This actually destroys immediately. + void DestroyImmediately(); bool IsDestroyed() const { return !descPool_; diff --git a/Common/GPU/Vulkan/VulkanRenderManager.cpp b/Common/GPU/Vulkan/VulkanRenderManager.cpp index ae8f523a81..ac1a33b561 100644 --- a/Common/GPU/Vulkan/VulkanRenderManager.cpp +++ b/Common/GPU/Vulkan/VulkanRenderManager.cpp @@ -515,20 +515,18 @@ void VulkanRenderManager::CompileThreadFunc() { Task *task = new CreateMultiPipelinesTask(vulkan_, entries); g_threadManager.EnqueueTask(task); } - + queueRunner_.NotifyCompileDone(); } } void VulkanRenderManager::DrainAndBlockCompileQueue() { - EndCurRenderStep(); std::unique_lock lock(compileMutex_); compileBlocked_ = true; compileCond_.notify_all(); while (!compileQueue_.empty()) { queueRunner_.WaitForCompileNotification(); } - FlushSync(); } void VulkanRenderManager::ReleaseCompileQueue() { @@ -1662,19 +1660,27 @@ VKRPipelineLayout *VulkanRenderManager::CreatePipelineLayout(BindingType *bindin } void VulkanRenderManager::DestroyPipelineLayout(VKRPipelineLayout *layout) { - for (int i = 0; i < VulkanContext::MAX_INFLIGHT_FRAMES; i++) { - layout->frameData[i].pool.Destroy(); - } - - vulkan_->Delete().QueueDeletePipelineLayout(layout->pipelineLayout); - vulkan_->Delete().QueueDeleteDescriptorSetLayout(layout->descriptorSetLayout); - for (auto iter = pipelineLayouts_.begin(); iter != pipelineLayouts_.end(); iter++) { - if (*iter == layout) { - pipelineLayouts_.erase(iter); - break; + struct DelInfo { + VulkanRenderManager *rm; + VKRPipelineLayout *layout; + }; + vulkan_->Delete().QueueCallback([](VulkanContext *vulkan, void *userdata) { + DelInfo *info = (DelInfo *)userdata; + for (auto iter = info->rm->pipelineLayouts_.begin(); iter != info->rm->pipelineLayouts_.end(); iter++) { + if (*iter == info->layout) { + info->rm->pipelineLayouts_.erase(iter); + break; + } } - } - delete layout; + for (int i = 0; i < VulkanContext::MAX_INFLIGHT_FRAMES; i++) { + info->layout->frameData[i].pool.DestroyImmediately(); + } + vkDestroyPipelineLayout(vulkan->GetDevice(), info->layout->pipelineLayout, nullptr); + vkDestroyDescriptorSetLayout(vulkan->GetDevice(), info->layout->descriptorSetLayout, nullptr); + + delete info->layout; + delete info; + }, new DelInfo{this, layout}); } void VulkanRenderManager::FlushDescriptors(int frame) { From 6357b95ff5a21f20464ca5fe125ff7c2545872fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Thu, 12 Oct 2023 23:33:54 +0200 Subject: [PATCH 2/2] Better version. --- Common/File/FileUtil.cpp | 1 + Common/GPU/Vulkan/VulkanDescSet.cpp | 1 + Common/GPU/Vulkan/VulkanRenderManager.cpp | 30 +++++++++-------------- 3 files changed, 14 insertions(+), 18 deletions(-) diff --git a/Common/File/FileUtil.cpp b/Common/File/FileUtil.cpp index 30c323b768..9754450ce8 100644 --- a/Common/File/FileUtil.cpp +++ b/Common/File/FileUtil.cpp @@ -1181,6 +1181,7 @@ uint8_t *ReadLocalFile(const Path &filename, size_t *size) { return nullptr; } fseek(file, 0, SEEK_SET); + // NOTE: If you find ~10 memory leaks from here, with very varying sizes, it might be the VFPU LUTs. uint8_t *contents = new uint8_t[f_size + 1]; if (fread(contents, 1, f_size, file) != f_size) { delete[] contents; diff --git a/Common/GPU/Vulkan/VulkanDescSet.cpp b/Common/GPU/Vulkan/VulkanDescSet.cpp index 165c8885c7..640f034d39 100644 --- a/Common/GPU/Vulkan/VulkanDescSet.cpp +++ b/Common/GPU/Vulkan/VulkanDescSet.cpp @@ -95,6 +95,7 @@ void VulkanDescSetPool::Destroy() { void VulkanDescSetPool::DestroyImmediately() { if (descPool_ != VK_NULL_HANDLE) { vkDestroyDescriptorPool(vulkan_->GetDevice(), descPool_, nullptr); + descPool_ = VK_NULL_HANDLE; usage_ = 0; } sizes_.clear(); diff --git a/Common/GPU/Vulkan/VulkanRenderManager.cpp b/Common/GPU/Vulkan/VulkanRenderManager.cpp index ac1a33b561..e39b3d1bb5 100644 --- a/Common/GPU/Vulkan/VulkanRenderManager.cpp +++ b/Common/GPU/Vulkan/VulkanRenderManager.cpp @@ -1660,27 +1660,22 @@ VKRPipelineLayout *VulkanRenderManager::CreatePipelineLayout(BindingType *bindin } void VulkanRenderManager::DestroyPipelineLayout(VKRPipelineLayout *layout) { - struct DelInfo { - VulkanRenderManager *rm; - VKRPipelineLayout *layout; - }; + for (auto iter = pipelineLayouts_.begin(); iter != pipelineLayouts_.end(); iter++) { + if (*iter == layout) { + pipelineLayouts_.erase(iter); + break; + } + } vulkan_->Delete().QueueCallback([](VulkanContext *vulkan, void *userdata) { - DelInfo *info = (DelInfo *)userdata; - for (auto iter = info->rm->pipelineLayouts_.begin(); iter != info->rm->pipelineLayouts_.end(); iter++) { - if (*iter == info->layout) { - info->rm->pipelineLayouts_.erase(iter); - break; - } - } + VKRPipelineLayout *layout = (VKRPipelineLayout *)userdata; for (int i = 0; i < VulkanContext::MAX_INFLIGHT_FRAMES; i++) { - info->layout->frameData[i].pool.DestroyImmediately(); + layout->frameData[i].pool.DestroyImmediately(); } - vkDestroyPipelineLayout(vulkan->GetDevice(), info->layout->pipelineLayout, nullptr); - vkDestroyDescriptorSetLayout(vulkan->GetDevice(), info->layout->descriptorSetLayout, nullptr); + vkDestroyPipelineLayout(vulkan->GetDevice(), layout->pipelineLayout, nullptr); + vkDestroyDescriptorSetLayout(vulkan->GetDevice(), layout->descriptorSetLayout, nullptr); - delete info->layout; - delete info; - }, new DelInfo{this, layout}); + delete layout; + }, layout); } void VulkanRenderManager::FlushDescriptors(int frame) { @@ -1700,7 +1695,6 @@ void VulkanRenderManager::ResetDescriptorLists(int frame) { } VKRPipelineLayout::~VKRPipelineLayout() { - _assert_(!pipelineLayout && !descriptorSetLayout); _assert_(frameData[0].pool.IsDestroyed()); }