From 70b0bfec3462a0adc671d2450fa0b97964daec2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Tue, 10 Nov 2026 22:43:32 +0100 Subject: [PATCH] Vulkan: Don't hold a pipeline's lock while waiting for it to compile When the render thread reaches a pipeline that isn't compiled yet, it waits for it, and it did so holding the pipeline's mutex. EndCurRenderStep takes that mutex for every pipeline a render step used, so the emulation thread stopped in the middle of a draw until the compile was done: 302 ms in one frame of a Tekken 6 fight on a Galaxy S8, for a pipeline that waited 251 ms for its shaders and then compiled in 68 ms. The render thread now only holds it to find (or make) the Promise, and waits without it. DestroyVariants, which can drop Promises meanwhile, deletes them through the deletion queue like the VkPipelines, so they outlive the wait. The emulation thread now runs on until the frame queue is full. Co-Authored-By: Claude Opus 5.5 --- Common/GPU/Vulkan/VulkanQueueRunner.cpp | 20 ++++++++++++++------ Common/GPU/Vulkan/VulkanRenderManager.cpp | 8 +++++--- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/Common/GPU/Vulkan/VulkanQueueRunner.cpp b/Common/GPU/Vulkan/VulkanQueueRunner.cpp index 0e9620d474..0b4ca51983 100644 --- a/Common/GPU/Vulkan/VulkanQueueRunner.cpp +++ b/Common/GPU/Vulkan/VulkanQueueRunner.cpp @@ -1281,21 +1281,29 @@ void VulkanQueueRunner::PerformRenderPass(const VKRStep &step, VkCommandBuffer c "expected %d sample count, got %d", fbSampleCount, graphicsPipeline->SampleCount()); } - VkPipeline pipeline; - + Promise *promise; + bool compileHere = false; { std::lock_guard lock(graphicsPipeline->mutex_); - if (!graphicsPipeline->pipeline[(size_t)rpType]) { + promise = graphicsPipeline->pipeline[(size_t)rpType]; + if (!promise) { // NOTE: If render steps got merged, it can happen that, as they ended during recording, // they didn't know their final render pass type so they created the wrong pipelines in EndCurRenderStep(). // Unfortunately I don't know if we can fix it in any more sensible place than here. // Maybe a middle pass. But let's try to just block and compile here for now, this doesn't // happen all that much. - graphicsPipeline->pipeline[(size_t)rpType] = Promise::CreateEmpty(); - graphicsPipeline->Create(vulkan_, renderPass->Get(vulkan_, rpType, fbSampleCount), rpType, fbSampleCount, time_now_d(), -1); + promise = Promise::CreateEmpty(); + graphicsPipeline->pipeline[(size_t)rpType] = promise; + compileHere = true; } - pipeline = graphicsPipeline->pipeline[(size_t)rpType]->BlockUntilReady(); } + // Not under the lock: the main thread takes it for every pipeline a render step used + // (EndCurRenderStep), and waiting for the shaders and the compile can take hundreds of ms. + // DestroyVariants deletes Promises later, so this one stays valid. + if (compileHere) { + graphicsPipeline->Create(vulkan_, renderPass->Get(vulkan_, rpType, fbSampleCount), rpType, fbSampleCount, time_now_d(), -1); + } + VkPipeline pipeline = promise->BlockUntilReady(); if (pipeline != VK_NULL_HANDLE) { vkCmdBindPipeline(cmd, VK_PIPELINE_BIND_POINT_GRAPHICS, pipeline); diff --git a/Common/GPU/Vulkan/VulkanRenderManager.cpp b/Common/GPU/Vulkan/VulkanRenderManager.cpp index 81b5860801..033db37843 100644 --- a/Common/GPU/Vulkan/VulkanRenderManager.cpp +++ b/Common/GPU/Vulkan/VulkanRenderManager.cpp @@ -171,8 +171,8 @@ bool VKRGraphicsPipeline::Create(VulkanContext *vulkan, VkRenderPass compatibleR void VKRGraphicsPipeline::DestroyVariants(VulkanContext *vulkan, bool msaaOnly) { // Called from InvalidateMSAAPipelines on the main thread, mid-frame, while the render thread may be // reading and replacing these same slots in PerformRenderPass - so take the lock that's documented - // as protecting the array. It also has to be held across the delete below, or the render thread can - // be left holding a freed Promise. + // as protecting the array. The render thread waits for a Promise without the lock, so the Promise + // is deleted later, like the VkPipeline. std::lock_guard lock(mutex_); for (size_t i = 0; i < (size_t)RenderPassType::TYPE_COUNT; i++) { @@ -188,7 +188,9 @@ void VKRGraphicsPipeline::DestroyVariants(VulkanContext *vulkan, bool msaaOnly) } // The array owns the Promise - DestroyVariantsInstant deletes it too. Forgetting it here leaked // one per destroyed variant on every MSAA or resolution change. - delete this->pipeline[i]; + vulkan->Delete().QueueCallback([promise = this->pipeline[i]](VulkanContext *vulkan) { + delete promise; + }); this->pipeline[i] = nullptr; } sampleCount_ = VK_SAMPLE_COUNT_FLAG_BITS_MAX_ENUM;