Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix a race condition during Vulkan shader cache load. #16804

Merged
merged 1 commit into from
Jan 13, 2023
Merged

Fix a race condition during Vulkan shader cache load. #16804

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Common/GPU/Vulkan/VulkanRenderManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down
6 changes: 4 additions & 2 deletions Common/GPU/Vulkan/VulkanRenderManager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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,
Expand Down Expand Up @@ -225,7 +227,7 @@ class VulkanRenderManager {
// 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() {
Expand Down
2 changes: 1 addition & 1 deletion Common/GPU/Vulkan/thin3d_vulkan.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion GPU/GLES/ShaderManagerGLES.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions GPU/Vulkan/DrawEngineVulkan.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion GPU/Vulkan/GPU_Vulkan.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,5 +100,5 @@ class GPU_Vulkan : public GPUCommon {
FrameData frameData_[VulkanContext::MAX_INFLIGHT_FRAMES]{};

Path shaderCachePath_;
bool shaderCacheLoaded_ = false;
std::atomic<bool> shaderCacheLoaded_{};
};
11 changes: 6 additions & 5 deletions GPU/Vulkan/PipelineManagerVulkan.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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) {
Expand All @@ -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_);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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++) {
Expand All @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion GPU/Vulkan/PipelineManagerVulkan.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ class PipelineManagerVulkan {
~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();
Expand Down
2 changes: 1 addition & 1 deletion GPU/Vulkan/ShaderManagerVulkan.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down