Fix a race condition during Vulkan shader cache load.

Could lead to unnecessary pipelines being created.
This commit is contained in:
Henrik Rydgård 2023-01-13 10:14:29 +01:00
parent 5b3ac098ae
commit 784e8ab782
9 changed files with 20 additions and 17 deletions

View File

@ -558,7 +558,7 @@ VkCommandBuffer VulkanRenderManager::GetInitCmd() {
return frameData_[curFrame].GetInitCmd(vulkan_);
}
VKRGraphicsPipeline *VulkanRenderManager::CreateGraphicsPipeline(VKRGraphicsPipelineDesc *desc, PipelineFlags pipelineFlags, uint32_t variantBitmask, VkSampleCountFlagBits sampleCount, const char *tag) {
VKRGraphicsPipeline *VulkanRenderManager::CreateGraphicsPipeline(VKRGraphicsPipelineDesc *desc, PipelineFlags pipelineFlags, uint32_t variantBitmask, VkSampleCountFlagBits sampleCount, bool cacheLoad, const char *tag) {
VKRGraphicsPipeline *pipeline = new VKRGraphicsPipeline(pipelineFlags, tag);
if (!desc->vertexShader || !desc->fragmentShader) {
@ -568,8 +568,8 @@ VKRGraphicsPipeline *VulkanRenderManager::CreateGraphicsPipeline(VKRGraphicsPipe
pipeline->desc = desc;
pipeline->desc->AddRef();
if (curRenderStep_) {
// The common case
if (curRenderStep_ && !cacheLoad) {
// The common case during gameplay.
pipelinesToCheck_.push_back(pipeline);
} else {
if (!variantBitmask) {

View File

@ -135,6 +135,8 @@ struct VKRGraphicsPipeline {
Promise<VkPipeline> *pipeline[(size_t)RenderPassType::TYPE_COUNT]{};
VkSampleCountFlagBits SampleCount() const { return sampleCount_; }
const char *Tag() const { return tag_.c_str(); }
private:
void DestroyVariantsInstant(VkDevice device);
@ -160,7 +162,7 @@ struct VKRComputePipeline {
struct CompileQueueEntry {
CompileQueueEntry(VKRGraphicsPipeline *p, VkRenderPass _compatibleRenderPass, RenderPassType _renderPassType, VkSampleCountFlagBits _sampleCount)
: type(Type::GRAPHICS), graphics(p), compatibleRenderPass(_compatibleRenderPass), renderPassType(_renderPassType), sampleCount(_sampleCount) {}
CompileQueueEntry(VKRComputePipeline *p) : type(Type::COMPUTE), compute(p), renderPassType(RenderPassType::HAS_DEPTH), sampleCount(VK_SAMPLE_COUNT_1_BIT) {} // renderpasstype here shouldn't matter
CompileQueueEntry(VKRComputePipeline *p) : type(Type::COMPUTE), compute(p), renderPassType(RenderPassType::DEFAULT), sampleCount(VK_SAMPLE_COUNT_1_BIT), compatibleRenderPass(VK_NULL_HANDLE) {} // renderpasstype here shouldn't matter
enum class Type {
GRAPHICS,
COMPUTE,
@ -225,7 +227,7 @@ public:
// We delay creating pipelines until the end of the current render pass, so we can create the right type immediately.
// Unless a variantBitmask is passed in, in which case we can just go ahead.
// WARNING: desc must stick around during the lifetime of the pipeline! It's not enough to build it on the stack and drop it.
VKRGraphicsPipeline *CreateGraphicsPipeline(VKRGraphicsPipelineDesc *desc, PipelineFlags pipelineFlags, uint32_t variantBitmask, VkSampleCountFlagBits sampleCount, const char *tag);
VKRGraphicsPipeline *CreateGraphicsPipeline(VKRGraphicsPipelineDesc *desc, PipelineFlags pipelineFlags, uint32_t variantBitmask, VkSampleCountFlagBits sampleCount, bool cacheLoad, const char *tag);
VKRComputePipeline *CreateComputePipeline(VKRComputePipelineDesc *desc);
void NudgeCompilerThread() {

View File

@ -1224,7 +1224,7 @@ Pipeline *VKContext::CreateGraphicsPipeline(const PipelineDesc &desc, const char
VkPipelineRasterizationStateCreateInfo rs{ VK_STRUCTURE_TYPE_PIPELINE_RASTERIZATION_STATE_CREATE_INFO };
raster->ToVulkan(&gDesc.rs);
pipeline->pipeline = renderManager_.CreateGraphicsPipeline(&gDesc, pipelineFlags, 1 << (size_t)RenderPassType::BACKBUFFER, VK_SAMPLE_COUNT_1_BIT, tag ? tag : "thin3d");
pipeline->pipeline = renderManager_.CreateGraphicsPipeline(&gDesc, pipelineFlags, 1 << (size_t)RenderPassType::BACKBUFFER, VK_SAMPLE_COUNT_1_BIT, false, tag ? tag : "thin3d");
if (desc.uniformDesc) {
pipeline->dynamicUniformSize = (int)desc.uniformDesc->uniformBufferSize;

View File

@ -956,7 +956,7 @@ enum class CacheDetectFlags {
};
#define CACHE_HEADER_MAGIC 0x83277592
#define CACHE_VERSION 26
#define CACHE_VERSION 27
struct CacheHeader {
uint32_t magic;

View File

@ -799,7 +799,7 @@ void DrawEngineVulkan::DoFlush() {
}
_dbg_assert_msg_(vshader->UseHWTransform(), "Bad vshader");
VulkanPipeline *pipeline = pipelineManager_->GetOrCreatePipeline(renderManager, pipelineLayout_, pipelineKey_, &dec_->decFmt, vshader, fshader, gshader, true, 0, framebufferManager_->GetMSAALevel());
VulkanPipeline *pipeline = pipelineManager_->GetOrCreatePipeline(renderManager, pipelineLayout_, pipelineKey_, &dec_->decFmt, vshader, fshader, gshader, true, 0, framebufferManager_->GetMSAALevel(), false);
if (!pipeline || !pipeline->pipeline) {
// Already logged, let's bail out.
return;
@ -928,7 +928,7 @@ void DrawEngineVulkan::DoFlush() {
shaderManager_->GetShaders(prim, dec_, &vshader, &fshader, &gshader, pipelineState_, false, false, decOptions_.expandAllWeightsToFloat, true);
_dbg_assert_msg_(!vshader->UseHWTransform(), "Bad vshader");
VulkanPipeline *pipeline = pipelineManager_->GetOrCreatePipeline(renderManager, pipelineLayout_, pipelineKey_, &dec_->decFmt, vshader, fshader, gshader, false, 0, framebufferManager_->GetMSAALevel());
VulkanPipeline *pipeline = pipelineManager_->GetOrCreatePipeline(renderManager, pipelineLayout_, pipelineKey_, &dec_->decFmt, vshader, fshader, gshader, false, 0, framebufferManager_->GetMSAALevel(), false);
if (!pipeline || !pipeline->pipeline) {
// Already logged, let's bail out.
decodedVerts_ = 0;

View File

@ -100,5 +100,5 @@ private:
FrameData frameData_[VulkanContext::MAX_INFLIGHT_FRAMES]{};
Path shaderCachePath_;
bool shaderCacheLoaded_ = false;
std::atomic<bool> shaderCacheLoaded_{};
};

View File

@ -179,7 +179,7 @@ static std::string CutFromMain(std::string str) {
static VulkanPipeline *CreateVulkanPipeline(VulkanRenderManager *renderManager, VkPipelineCache pipelineCache,
VkPipelineLayout layout, PipelineFlags pipelineFlags, VkSampleCountFlagBits sampleCount, const VulkanPipelineRasterStateKey &key,
const DecVtxFormat *decFmt, VulkanVertexShader *vs, VulkanFragmentShader *fs, VulkanGeometryShader *gs, bool useHwTransform, u32 variantBitmask) {
const DecVtxFormat *decFmt, VulkanVertexShader *vs, VulkanFragmentShader *fs, VulkanGeometryShader *gs, bool useHwTransform, u32 variantBitmask, bool cacheLoad) {
if (!fs->GetModule()) {
ERROR_LOG(G3D, "Fragment shader missing in CreateVulkanPipeline");
@ -319,7 +319,7 @@ static VulkanPipeline *CreateVulkanPipeline(VulkanRenderManager *renderManager,
tag = FragmentShaderDesc(fs->GetID()) + " VS " + VertexShaderDesc(vs->GetID());
#endif
VKRGraphicsPipeline *pipeline = renderManager->CreateGraphicsPipeline(desc, pipelineFlags, variantBitmask, sampleCount, tag.c_str());
VKRGraphicsPipeline *pipeline = renderManager->CreateGraphicsPipeline(desc, pipelineFlags, variantBitmask, sampleCount, cacheLoad, tag.c_str());
vulkanPipeline->pipeline = pipeline;
if (useBlendConstant) {
@ -335,7 +335,7 @@ static VulkanPipeline *CreateVulkanPipeline(VulkanRenderManager *renderManager,
return vulkanPipeline;
}
VulkanPipeline *PipelineManagerVulkan::GetOrCreatePipeline(VulkanRenderManager *renderManager, VkPipelineLayout layout, const VulkanPipelineRasterStateKey &rasterKey, const DecVtxFormat *decFmt, VulkanVertexShader *vs, VulkanFragmentShader *fs, VulkanGeometryShader *gs, bool useHwTransform, u32 variantBitmask, int multiSampleLevel) {
VulkanPipeline *PipelineManagerVulkan::GetOrCreatePipeline(VulkanRenderManager *renderManager, VkPipelineLayout layout, const VulkanPipelineRasterStateKey &rasterKey, const DecVtxFormat *decFmt, VulkanVertexShader *vs, VulkanFragmentShader *fs, VulkanGeometryShader *gs, bool useHwTransform, u32 variantBitmask, int multiSampleLevel, bool cacheLoad) {
if (!pipelineCache_) {
VkPipelineCacheCreateInfo pc{ VK_STRUCTURE_TYPE_PIPELINE_CACHE_CREATE_INFO };
VkResult res = vkCreatePipelineCache(vulkan_->GetDevice(), &pc, nullptr, &pipelineCache_);
@ -370,7 +370,7 @@ VulkanPipeline *PipelineManagerVulkan::GetOrCreatePipeline(VulkanRenderManager *
VulkanPipeline *pipeline = CreateVulkanPipeline(
renderManager, pipelineCache_, layout, pipelineFlags, sampleCount,
rasterKey, decFmt, vs, fs, gs, useHwTransform, variantBitmask);
rasterKey, decFmt, vs, fs, gs, useHwTransform, variantBitmask, cacheLoad);
// If the above failed, we got a null pipeline. We still insert it to keep track.
pipelines_.Insert(key, pipeline);
@ -786,6 +786,7 @@ bool PipelineManagerVulkan::LoadPipelineCache(FILE *file, bool loadRawPipelineCa
}
// Avoid creating multisampled shaders if it's not enabled, as that results in an invalid combination.
// Note that variantsToBuild is NOT directly a RenderPassType! instead, it's a collection of (1 << RenderPassType).
u32 variantsToBuild = key.variants;
if (multiSampleLevel == 0) {
for (u32 i = 0; i < (int)RenderPassType::TYPE_COUNT; i++) {
@ -798,7 +799,7 @@ bool PipelineManagerVulkan::LoadPipelineCache(FILE *file, bool loadRawPipelineCa
DecVtxFormat fmt;
fmt.InitializeFromID(key.vtxFmtId);
VulkanPipeline *pipeline = GetOrCreatePipeline(
rm, layout, key.raster, key.useHWTransform ? &fmt : 0, vs, fs, gs, key.useHWTransform, variantsToBuild, multiSampleLevel);
rm, layout, key.raster, key.useHWTransform ? &fmt : 0, vs, fs, gs, key.useHWTransform, variantsToBuild, multiSampleLevel, true);
if (!pipeline) {
pipelineCreateFailCount += 1;
}

View File

@ -86,7 +86,7 @@ public:
~PipelineManagerVulkan();
// variantMask is only used when loading pipelines from cache.
VulkanPipeline *GetOrCreatePipeline(VulkanRenderManager *renderManager, VkPipelineLayout layout, const VulkanPipelineRasterStateKey &rasterKey, const DecVtxFormat *decFmt, VulkanVertexShader *vs, VulkanFragmentShader *fs, VulkanGeometryShader *gs, bool useHwTransform, u32 variantMask, int multiSampleLevel);
VulkanPipeline *GetOrCreatePipeline(VulkanRenderManager *renderManager, VkPipelineLayout layout, const VulkanPipelineRasterStateKey &rasterKey, const DecVtxFormat *decFmt, VulkanVertexShader *vs, VulkanFragmentShader *fs, VulkanGeometryShader *gs, bool useHwTransform, u32 variantMask, int multiSampleLevel, bool cacheLoad);
int GetNumPipelines() const { return (int)pipelines_.size(); }
void Clear();

View File

@ -511,7 +511,7 @@ enum class VulkanCacheDetectFlags {
};
#define CACHE_HEADER_MAGIC 0xff51f420
#define CACHE_VERSION 40
#define CACHE_VERSION 41
struct VulkanCacheHeader {
uint32_t magic;