[Vulkan] Remove background spirv optimization

(and move inline optimization to background creation threads)

src/xenia/gpu/shader.h

@@ -830,6 +830,8 @@ class Shader {
     // If there was some failure during preparation on the implementation side.
     void MakeInvalid() { is_valid_ = false; }
 
+    std::vector<uint8_t> translated_binary_;
+
    private:
     friend class Shader;
     friend class ShaderTranslator;
@@ -840,7 +842,6 @@ class Shader {
     bool is_valid_ = false;
     bool is_translated_ = false;
     std::vector<Error> errors_;
-    std::vector<uint8_t> translated_binary_;
     std::string host_disassembly_;
   };
 

src/xenia/gpu/vulkan/vulkan_pipeline_cache.cc

@@ -60,18 +60,10 @@ DEFINE_int32(
     "Vulkan");
 
 DEFINE_bool(
-    vulkan_spirv_background_optimization, false,
-    "Enable background SPIR-V shader optimization. When enabled, shaders "
-    "are initially compiled without optimization for faster startup, then "
-    "optimized in a background thread.",
-    "Vulkan");
-
-DEFINE_bool(
-    vulkan_spirv_inline_optimization, false,
-    "Enable inline SPIR-V shader optimization. When enabled, shaders are "
-    "optimized immediately during translation, blocking the main thread. "
-    "This increases shader compilation time but ensures all shaders are "
-    "optimized before use. Can be combined with background optimization.",
+    vulkan_spirv_optimization, false,
+    "Enable SPIR-V shader optimization. When enabled, shaders are optimized "
+    "on pipeline creation threads before the shader module is created. This "
+    "only affects async pipeline creation and does not block the main thread.",
     "Vulkan");
 
 DECLARE_bool(vulkan_dynamic_rendering);
@@ -250,18 +242,6 @@ bool VulkanPipelineCache::Initialize() {
     }
   }
 
-  // Start background optimization thread
-  if (cvars::vulkan_spirv_background_optimization && spirv_tools_context_) {
-    optimization_thread_shutdown_.store(false, std::memory_order_release);
-    optimization_thread_ =
-        xe::threading::Thread::Create({}, [this]() { OptimizationThread(); });
-    assert_not_null(optimization_thread_);
-    optimization_thread_->set_name("SPIRV Optimizer");
-    XELOGI("SPIR-V background optimization thread started");
-  } else if (!cvars::vulkan_spirv_background_optimization) {
-    XELOGI("SPIR-V background optimization disabled");
-  }
-
   return true;
 }
 
@@ -281,17 +261,6 @@ void VulkanPipelineCache::Shutdown() {
   }
   creation_completion_event_.reset();
 
-  // Shut down the optimization thread
-  if (optimization_thread_) {
-    {
-      std::lock_guard<std::mutex> lock(optimization_queue_lock_);
-      optimization_thread_shutdown_.store(true, std::memory_order_release);
-    }
-    optimization_queue_cond_.notify_all();
-    xe::threading::Wait(optimization_thread_.get(), false);
-    optimization_thread_.reset();
-  }
-
   const ui::vulkan::VulkanDevice* const vulkan_device =
       command_processor_.GetVulkanDevice();
   const ui::vulkan::VulkanDevice::Functions& dfn = vulkan_device->functions();
@@ -301,12 +270,6 @@ void VulkanPipelineCache::Shutdown() {
   // device should be idle at shutdown).
   {
     std::lock_guard<std::mutex> lock(deferred_destroy_mutex_);
-    for (VkShaderModule module : deferred_destroy_shader_modules_) {
-      if (module != VK_NULL_HANDLE) {
-        dfn.vkDestroyShaderModule(device, module, nullptr);
-      }
-    }
-    deferred_destroy_shader_modules_.clear();
     for (const auto& pipeline_pair : deferred_destroy_pipelines_) {
       if (pipeline_pair.first != VK_NULL_HANDLE) {
         dfn.vkDestroyPipeline(device, pipeline_pair.first, nullptr);
@@ -814,8 +777,17 @@ void VulkanPipelineCache::CreationThread() {
     if (!EnsureShadersTranslated(creation_arguments.vertex_shader,
                                  creation_arguments.pixel_shader)) {
       XELOGE("Failed to translate shaders for pipeline creation");
-    } else if (!EnsurePipelineCreated(creation_arguments)) {
-      XELOGE("Failed to create Vulkan pipeline");
+    } else {
+      // Optimize shaders on the creation thread before creating the pipeline.
+      // This keeps the main thread fast while still benefiting from
+      // optimization.
+      OptimizeTranslationIfNeeded(*creation_arguments.vertex_shader);
+      if (creation_arguments.pixel_shader) {
+        OptimizeTranslationIfNeeded(*creation_arguments.pixel_shader);
+      }
+      if (!EnsurePipelineCreated(creation_arguments)) {
+        XELOGE("Failed to create Vulkan pipeline");
+      }
     }
     // On failure: if a placeholder exists it will remain in use permanently.
     // Clear the flag so we're not in a misleading "waiting for real" state.
@@ -854,52 +826,6 @@ bool VulkanPipelineCache::TranslateAnalyzedShader(
     return false;
   }
 
-  // Perform inline optimization if enabled - optimize before the shader module
-  // is created
-  if (cvars::vulkan_spirv_inline_optimization && spirv_tools_context_ &&
-      translation.is_valid()) {
-    const std::vector<uint8_t>& unoptimized_binary =
-        translation.translated_binary();
-    if (!unoptimized_binary.empty()) {
-      // Reinterpret the byte vector as uint32_t for SPIRV-Tools
-      const uint32_t* spirv_words =
-          reinterpret_cast<const uint32_t*>(unoptimized_binary.data());
-      size_t word_count = unoptimized_binary.size() / sizeof(uint32_t);
-
-      std::vector<uint32_t> optimized_spirv;
-      spv_result_t result = spirv_tools_context_->Optimize(
-          spirv_words, word_count, optimized_spirv, true);
-
-      if (result == SPV_SUCCESS && !optimized_spirv.empty()) {
-        // Convert back to byte vector
-        std::vector<uint8_t> optimized_binary;
-        optimized_binary.resize(optimized_spirv.size() * sizeof(uint32_t));
-        std::memcpy(optimized_binary.data(), optimized_spirv.data(),
-                    optimized_binary.size());
-
-        // Store as optimized binary (will be used by GetOrCreateShaderModule)
-        translation.SetOptimizedBinary(optimized_binary);
-
-        size_t original_size = word_count;
-        size_t optimized_size = optimized_spirv.size();
-        XELOGI("Inline SPIRV optimization: {} -> {} words ({:.1f}% reduction)",
-               original_size, optimized_size,
-               100.0f * (1.0f - float(optimized_size) / float(original_size)));
-      } else {
-        XELOGW("Inline SPIRV optimization failed with error code: {}",
-               static_cast<int>(result));
-      }
-    }
-  }
-
-  // Store unoptimized binary and queue for background optimization
-  // (only if inline optimization is disabled)
-  if (spirv_tools_context_ && translation.NeedsOptimization() &&
-      !cvars::vulkan_spirv_inline_optimization) {
-    translation.StoreUnoptimizedBinary();
-    QueueShaderForOptimization(&translation);
-  }
-
 #ifndef NDEBUG
   // Validate SPIR-V before creating shader module to get detailed error
   // messages. This is a warning only - we still try to create the shader
@@ -3027,40 +2953,16 @@ bool VulkanPipelineCache::EnsurePipelineCreated(
   return true;
 }
 
-void VulkanPipelineCache::QueueShaderForOptimization(
-    VulkanShader::VulkanTranslation* translation) {
-  if (!cvars::vulkan_spirv_background_optimization || !spirv_tools_context_ ||
-      !optimization_thread_ || !translation) {
-    return;
-  }
-
-  // Verify the shader has unoptimized binary before queuing
-  if (translation->GetUnoptimizedBinary().empty()) {
-    return;
-  }
-
-  // Queue for optimization
-  {
-    std::lock_guard<std::mutex> lock(optimization_queue_lock_);
-    optimization_queue_.push_back({translation});
-  }
-  optimization_queue_cond_.notify_one();
-}
-
 void VulkanPipelineCache::ProcessDeferredDestructions() {
-  std::vector<VkShaderModule> modules_to_destroy;
   std::vector<VkPipeline> pipelines_to_destroy;
 
   uint64_t completed_submission = command_processor_.GetCompletedSubmission();
 
   {
     std::lock_guard<std::mutex> lock(deferred_destroy_mutex_);
-    if (deferred_destroy_shader_modules_.empty() &&
-        deferred_destroy_pipelines_.empty()) {
+    if (deferred_destroy_pipelines_.empty()) {
       return;
     }
-    modules_to_destroy = std::move(deferred_destroy_shader_modules_);
-    deferred_destroy_shader_modules_.clear();
 
     // Only destroy pipelines whose submission has completed on the GPU.
     // Keep pipelines that are still potentially in-flight.
@@ -3076,18 +2978,16 @@ void VulkanPipelineCache::ProcessDeferredDestructions() {
     }
   }
 
-  // Destroy the modules and pipelines now that we know GPU is done with them.
+  if (pipelines_to_destroy.empty()) {
+    return;
+  }
+
+  // Destroy pipelines now that we know GPU is done with them.
   const ui::vulkan::VulkanDevice* vulkan_device =
       command_processor_.GetVulkanDevice();
   const ui::vulkan::VulkanDevice::Functions& dfn = vulkan_device->functions();
   VkDevice device = vulkan_device->device();
 
-  for (VkShaderModule module : modules_to_destroy) {
-    if (module != VK_NULL_HANDLE) {
-      dfn.vkDestroyShaderModule(device, module, nullptr);
-    }
-  }
-
   for (VkPipeline pipeline : pipelines_to_destroy) {
     if (pipeline != VK_NULL_HANDLE) {
       dfn.vkDestroyPipeline(device, pipeline, nullptr);
@@ -3095,75 +2995,55 @@ void VulkanPipelineCache::ProcessDeferredDestructions() {
   }
 }
 
-void VulkanPipelineCache::OptimizationThread() {
-  for (;;) {
-    ShaderOptimizationRequest request;
-    {
-      std::unique_lock<std::mutex> lock(optimization_queue_lock_);
-      optimization_queue_cond_.wait(lock, [this]() {
-        return !optimization_queue_.empty() ||
-               optimization_thread_shutdown_.load(std::memory_order_acquire);
-      });
+void VulkanPipelineCache::OptimizeTranslationIfNeeded(
+    VulkanShader::VulkanTranslation& translation) {
+  // Only optimize if enabled and spirv-tools is available.
+  if (!cvars::vulkan_spirv_optimization || !spirv_tools_context_) {
+    return;
+  }
 
-      if (optimization_thread_shutdown_.load(std::memory_order_acquire)) {
-        break;
-      }
+  // Only optimize if the shader module hasn't been created yet.
+  // Once created, we can't replace it without the complexity of the old
+  // background optimization system.
+  if (translation.shader_module() != VK_NULL_HANDLE) {
+    return;
+  }
 
-      if (optimization_queue_.empty()) {
-        continue;
-      }
+  if (!translation.is_valid()) {
+    return;
+  }
 
-      request = std::move(optimization_queue_.front());
-      optimization_queue_.pop_front();
-    }
+  const std::vector<uint8_t>& unoptimized_binary =
+      translation.translated_binary();
+  if (unoptimized_binary.empty()) {
+    return;
+  }
 
-    // Perform optimization outside of the lock
-    if (request.translation) {
-      const std::vector<uint8_t>& unoptimized_binary =
-          request.translation->GetUnoptimizedBinary();
-      if (!unoptimized_binary.empty()) {
-        // Reinterpret the byte vector as uint32_t for SPIRV-Tools
-        const uint32_t* spirv_words =
-            reinterpret_cast<const uint32_t*>(unoptimized_binary.data());
-        size_t word_count = unoptimized_binary.size() / sizeof(uint32_t);
+  // Reinterpret the byte vector as uint32_t for SPIRV-Tools
+  const uint32_t* spirv_words =
+      reinterpret_cast<const uint32_t*>(unoptimized_binary.data());
+  size_t word_count = unoptimized_binary.size() / sizeof(uint32_t);
 
-        std::vector<uint32_t> optimized_spirv;
-        spv_result_t result = spirv_tools_context_->Optimize(
-            spirv_words, word_count, optimized_spirv, true);
+  std::vector<uint32_t> optimized_spirv;
+  spv_result_t result = spirv_tools_context_->Optimize(spirv_words, word_count,
+                                                       optimized_spirv, true);
 
-        if (result == SPV_SUCCESS && !optimized_spirv.empty()) {
-          // Convert back to byte vector
-          std::vector<uint8_t> optimized_binary;
-          optimized_binary.resize(optimized_spirv.size() * sizeof(uint32_t));
-          std::memcpy(optimized_binary.data(), optimized_spirv.data(),
-                      optimized_binary.size());
+  if (result == SPV_SUCCESS && !optimized_spirv.empty()) {
+    // Convert back to byte vector and replace the translated binary
+    std::vector<uint8_t> optimized_binary;
+    optimized_binary.resize(optimized_spirv.size() * sizeof(uint32_t));
+    std::memcpy(optimized_binary.data(), optimized_spirv.data(),
+                optimized_binary.size());
+    translation.SetOptimizedBinary(std::move(optimized_binary));
 
-          // Update the translation with optimized binary
-          request.translation->SetOptimizedBinary(optimized_binary);
-
-          // Collect any old shader modules that need deferred destruction
-          std::vector<VkShaderModule> modules_to_destroy =
-              request.translation->CollectPendingDestroyModules();
-          if (!modules_to_destroy.empty()) {
-            std::lock_guard<std::mutex> lock(deferred_destroy_mutex_);
-            deferred_destroy_shader_modules_.insert(
-                deferred_destroy_shader_modules_.end(),
-                modules_to_destroy.begin(), modules_to_destroy.end());
-          }
-
-          size_t original_size = word_count;
-          size_t optimized_size = optimized_spirv.size();
-          XELOGI(
-              "Background SPIRV optimization: {} -> {} words ({:.1f}% "
-              "reduction)",
-              original_size, optimized_size,
-              100.0f * (1.0f - float(optimized_size) / float(original_size)));
-        } else {
-          XELOGW("Background SPIRV optimization failed with error code: {}",
-                 static_cast<int>(result));
-        }
-      }
-    }
+    size_t original_size = word_count;
+    size_t optimized_size = optimized_spirv.size();
+    XELOGI("SPIRV optimization: {} -> {} words ({:.1f}% reduction)",
+           original_size, optimized_size,
+           100.0f * (1.0f - float(optimized_size) / float(original_size)));
+  } else {
+    XELOGW("SPIRV optimization failed with error code: {}",
+           static_cast<int>(result));
   }
 }
 

src/xenia/gpu/vulkan/vulkan_pipeline_cache.h

@@ -14,7 +14,6 @@
 #include <condition_variable>
 #include <cstddef>
 #include <cstring>
-#include <deque>
 #include <functional>
 #include <memory>
 #include <mutex>
@@ -368,6 +367,11 @@ class VulkanPipelineCache {
     return EnsurePipelineCreated(creation_arguments, placeholder_pixel_shader_);
   }
 
+  // Optimizes a shader's SPIR-V binary if optimization is enabled and the
+  // shader module hasn't been created yet. Called from creation threads.
+  void OptimizeTranslationIfNeeded(
+      VulkanShader::VulkanTranslation& translation);
+
   VulkanCommandProcessor& command_processor_;
   const RegisterFile& register_file_;
   VulkanRenderTargetCache& render_target_cache_;
@@ -465,25 +469,10 @@ class VulkanPipelineCache {
   std::unique_ptr<xe::threading::Event> creation_completion_event_ = nullptr;
   std::atomic<bool> creation_completion_set_event_{false};
 
-  // Background SPIRV optimization
-  struct ShaderOptimizationRequest {
-    VulkanShader::VulkanTranslation* translation;
-  };
-  std::unique_ptr<xe::threading::Thread> optimization_thread_;
-  std::deque<ShaderOptimizationRequest> optimization_queue_;
-  std::mutex optimization_queue_lock_;
-  std::condition_variable optimization_queue_cond_;
-  std::atomic<bool> optimization_thread_shutdown_{false};
-  void OptimizationThread();
-  void QueueShaderForOptimization(VulkanShader::VulkanTranslation* translation);
-
-  // Deferred destruction of replaced shader modules and pipelines.
+  // Deferred destruction of pipelines.
   // Pipelines are only destroyed after the GPU submission that might reference
   // them has completed (tracked via submission numbers from command processor).
   void ProcessDeferredDestructions();
-  std::vector<VkShaderModule> deferred_destroy_shader_modules_;
-  // Pipelines pending destruction, paired with the submission number they were
-  // last potentially used in. Only destroyed when that submission completes.
   std::vector<std::pair<VkPipeline, uint64_t>> deferred_destroy_pipelines_;
   std::mutex deferred_destroy_mutex_;
 };

src/xenia/gpu/vulkan/vulkan_shader.cc

@@ -20,12 +20,11 @@ namespace gpu {
 namespace vulkan {
 
 VulkanShader::VulkanTranslation::~VulkanTranslation() {
-  VkShaderModule module = shader_module_.load(std::memory_order_acquire);
-  if (module != VK_NULL_HANDLE) {
+  if (shader_module_ != VK_NULL_HANDLE) {
     const ui::vulkan::VulkanDevice* const vulkan_device =
         static_cast<const VulkanShader&>(shader()).vulkan_device_;
     vulkan_device->functions().vkDestroyShaderModule(vulkan_device->device(),
-                                                     module, nullptr);
+                                                     shader_module_, nullptr);
   }
 }
 
@@ -34,40 +33,27 @@ VkShaderModule VulkanShader::VulkanTranslation::GetOrCreateShaderModule() {
     return VK_NULL_HANDLE;
   }
 
-  VkShaderModule existing_module =
-      shader_module_.load(std::memory_order_acquire);
-  if (existing_module != VK_NULL_HANDLE) {
-    return existing_module;
-  }
+  // Lock for creation - multiple threads may try to create the same shader
+  std::lock_guard<std::mutex> lock(shader_module_mutex_);
 
-  // Lock for creation
-  std::lock_guard<std::mutex> lock(optimization_mutex_);
-
-  // Check again after acquiring lock
-  existing_module = shader_module_.load(std::memory_order_acquire);
-  if (existing_module != VK_NULL_HANDLE) {
-    return existing_module;
+  if (shader_module_ != VK_NULL_HANDLE) {
+    return shader_module_;
   }
 
   const ui::vulkan::VulkanDevice* const vulkan_device =
       static_cast<const VulkanShader&>(shader()).vulkan_device_;
 
-  // Use optimized binary if available, otherwise use the original
-  const std::vector<uint8_t>& binary_to_use =
-      !optimized_binary_.empty() ? optimized_binary_ : translated_binary();
-
   VkShaderModuleCreateInfo shader_module_create_info;
   shader_module_create_info.sType = VK_STRUCTURE_TYPE_SHADER_MODULE_CREATE_INFO;
   shader_module_create_info.pNext = nullptr;
   shader_module_create_info.flags = 0;
-  shader_module_create_info.codeSize = binary_to_use.size();
+  shader_module_create_info.codeSize = translated_binary().size();
   shader_module_create_info.pCode =
-      reinterpret_cast<const uint32_t*>(binary_to_use.data());
+      reinterpret_cast<const uint32_t*>(translated_binary().data());
 
-  VkShaderModule new_module = VK_NULL_HANDLE;
   if (vulkan_device->functions().vkCreateShaderModule(
           vulkan_device->device(), &shader_module_create_info, nullptr,
-          &new_module) != VK_SUCCESS) {
+          &shader_module_) != VK_SUCCESS) {
     XELOGE(
         "VulkanShader::VulkanTranslation: Failed to create a Vulkan shader "
         "module for shader {:016X} modification {:016X}",
@@ -76,49 +62,7 @@ VkShaderModule VulkanShader::VulkanTranslation::GetOrCreateShaderModule() {
     return VK_NULL_HANDLE;
   }
 
-  shader_module_.store(new_module, std::memory_order_release);
-  return new_module;
-}
-
-void VulkanShader::VulkanTranslation::SetOptimizedBinary(
-    const std::vector<uint8_t>& optimized_binary) {
-  std::lock_guard<std::mutex> lock(optimization_mutex_);
-
-  // Store the optimized binary
-  optimized_binary_ = optimized_binary;
-
-  // If we already have a shader module, we need to recreate it with optimized
-  // code
-  VkShaderModule old_module = shader_module_.load(std::memory_order_acquire);
-  if (old_module != VK_NULL_HANDLE) {
-    const ui::vulkan::VulkanDevice* const vulkan_device =
-        static_cast<const VulkanShader&>(shader()).vulkan_device_;
-
-    VkShaderModuleCreateInfo shader_module_create_info;
-    shader_module_create_info.sType =
-        VK_STRUCTURE_TYPE_SHADER_MODULE_CREATE_INFO;
-    shader_module_create_info.pNext = nullptr;
-    shader_module_create_info.flags = 0;
-    shader_module_create_info.codeSize = optimized_binary_.size();
-    shader_module_create_info.pCode =
-        reinterpret_cast<const uint32_t*>(optimized_binary_.data());
-
-    VkShaderModule new_module = VK_NULL_HANDLE;
-    if (vulkan_device->functions().vkCreateShaderModule(
-            vulkan_device->device(), &shader_module_create_info, nullptr,
-            &new_module) == VK_SUCCESS) {
-      // Atomically swap the modules
-      shader_module_.store(new_module, std::memory_order_release);
-
-      // Queue the old module for deferred destruction
-      // The pipeline cache will destroy these when it's safe (after GPU idle or
-      // fence wait)
-      pending_destroy_modules_.push_back(old_module);
-    }
-  }
-
-  // Mark as optimized
-  is_optimized_.store(true, std::memory_order_release);
+  return shader_module_;
 }
 
 VulkanShader::VulkanShader(const ui::vulkan::VulkanDevice* const vulkan_device,

src/xenia/gpu/vulkan/vulkan_shader.h

@@ -13,7 +13,6 @@
 #include <atomic>
 #include <cstdint>
 #include <mutex>
-#include <vector>
 
 #include "xenia/gpu/spirv_shader.h"
 #include "xenia/gpu/xenos.h"
@@ -32,41 +31,17 @@ class VulkanShader : public SpirvShader {
     ~VulkanTranslation() override;
 
     VkShaderModule GetOrCreateShaderModule();
-    VkShaderModule shader_module() const {
-      return shader_module_.load(std::memory_order_acquire);
-    }
+    VkShaderModule shader_module() const { return shader_module_; }
 
-    // Background optimization support
-    bool IsOptimized() const {
-      return is_optimized_.load(std::memory_order_acquire);
-    }
-    void SetOptimizedBinary(const std::vector<uint8_t>& optimized_binary);
-    bool NeedsOptimization() const {
-      return !is_optimized_.load(std::memory_order_acquire) && is_valid();
-    }
-    const std::vector<uint8_t>& GetUnoptimizedBinary() const {
-      return unoptimized_binary_;
-    }
-    void StoreUnoptimizedBinary() { unoptimized_binary_ = translated_binary(); }
-
-    // Collect shader modules that need deferred destruction
-    std::vector<VkShaderModule> CollectPendingDestroyModules() {
-      std::lock_guard<std::mutex> lock(optimization_mutex_);
-      std::vector<VkShaderModule> modules = std::move(pending_destroy_modules_);
-      pending_destroy_modules_.clear();
-      return modules;
+    // Replace the translated binary with an optimized version.
+    // Must be called before GetOrCreateShaderModule() creates the module.
+    void SetOptimizedBinary(std::vector<uint8_t>&& binary) {
+      translated_binary_ = std::move(binary);
     }
 
    private:
-    std::atomic<VkShaderModule> shader_module_{VK_NULL_HANDLE};
-    std::atomic<bool> is_optimized_{false};
-    std::vector<uint8_t> unoptimized_binary_;
-    std::vector<uint8_t> optimized_binary_;
-    std::mutex optimization_mutex_;
-
-    // Shader modules pending destruction (replaced by optimized versions)
-    // These will be destroyed when it's safe (handled by pipeline cache)
-    std::vector<VkShaderModule> pending_destroy_modules_;
+    VkShaderModule shader_module_ = VK_NULL_HANDLE;
+    std::mutex shader_module_mutex_;
   };
 
   explicit VulkanShader(const ui::vulkan::VulkanDevice* vulkan_device,