Vulkan: Implement pipeline UID cache

[stenzek]

This stores enough information to recreate the pipeline, including the shader UIDs, blend/depth/rasterization state, primitive and vertex format.

Should get rid of shader compiling stutter on second boots of games to the same degree that it is mitigated on the D3D/GL backends, if not even better, because we won't be hitting shader recompiles due to state changes (all these are tracked).

Especially makes a difference for AMD users, where the Vulkan pipeline cache does not appear to be populated with any data, so every boot resulted in full recompiles (apart from the initial GLSL generation being skipped).

Also adds validation of pipeline objects, I assume drivers should be doing this already, but just in case.

NOTE: Currently if a user flips between MSAA/SSAA on/off, this will increase the boot time of games because all variations of shaders will be generated. This is an issue that affects all backends, when/if we decouple MSAA state from the UIDs it will sort itself out.

---

This change is 

[lioncash]

Review status: 0 of 5 files reviewed at latest revision, 6 unresolved discussions.

---

Source/Core/VideoBackends/Vulkan/ObjectCache.cpp, line 162 at r1 (raw file):

}

VkPipeline ObjectCache::GetPipeline(const PipelineInfo& info, bool* was_cache_miss)

This should probably return a tuple or pair containing vkPipeline and a boolean instead of using an out parameter.

---

Source/Core/VideoBackends/Vulkan/ObjectCache.cpp, line 385 at r1 (raw file):

  u32 device_id;
  u8 uuid[VK_UUID_SIZE];
};

@degasus Did you update the buildbot to GCC 5 yet? This is a prime candidate for having

static_assert(std::is_trivially_copyable<VK_PIPELINE_CACHE_HEADER>(), "Header must be trivially copyable");

after it.

---

Source/Core/VideoBackends/Vulkan/StateTracker.cpp, line 123 at r1 (raw file):

void StateTracker::LoadPipelineUIDCache()
{
  class PipelineInserter : public LinearDiskCacheReader<SerializedPipelineUID, u32>

This should be final if it's not going to be extended.

---

Source/Core/VideoBackends/Vulkan/StateTracker.cpp, line 126 at r1 (raw file):

  {
  public:
    PipelineInserter(StateTracker* this_ptr_) : this_ptr(this_ptr_) {}

explicit PipelineInserter

---

Source/Core/VideoBackends/Vulkan/StateTracker.cpp, line 167 at r1 (raw file):

  // Do we have a vertex format created that matches the declaration?
  auto vertex_format_map = VertexLoaderManager::GetNativeVertexFormatMap();
  auto iter = vertex_format_map->find(uid.vertex_decl);

These and the following if statement should probably be their own function.

---

Source/Core/VideoBackends/Vulkan/StateTracker.cpp, line 174 at r1 (raw file):

    // Requires the new operator because we need to upcast it.
    auto ipair = vertex_format_map->emplace(
        uid.vertex_decl, std::unique_ptr<NativeVertexFormat>(new VertexFormat(uid.vertex_decl)));

You can use std::make_unique here.

---

Comments from Reviewable

[stenzek]

Review status: 0 of 7 files reviewed at latest revision, 6 unresolved discussions, some commit checks failed.

---

Source/Core/VideoBackends/Vulkan/ObjectCache.cpp, line 162 at r1 (raw file):

Previously, lioncash (Mat M.) wrote…

This should probably return a tuple or pair containing vkPipeline and a boolean instead of using an out parameter.

Done, although I added a second method for this case, as all but one of the callers don't care about whether it was a cache hit or miss.

---

Source/Core/VideoBackends/Vulkan/ObjectCache.cpp, line 385 at r1 (raw file):

Previously, lioncash (Mat M.) wrote…

@degasus Did you update the buildbot to GCC 5 yet? This is a prime candidate for having

static_assert(std::is_trivially_copyable<VK_PIPELINE_CACHE_HEADER>(), "Header must be trivially copyable");

after it.

Done. Looks like the builder isn't updated though, so I'll have to add a guard around it for now.

---

Source/Core/VideoBackends/Vulkan/StateTracker.cpp, line 123 at r1 (raw file):

Previously, lioncash (Mat M.) wrote…

This should be final if it's not going to be extended.

Done.

---

Source/Core/VideoBackends/Vulkan/StateTracker.cpp, line 126 at r1 (raw file):

Previously, lioncash (Mat M.) wrote…

explicit PipelineInserter

Done.

---

Source/Core/VideoBackends/Vulkan/StateTracker.cpp, line 167 at r1 (raw file):

Previously, lioncash (Mat M.) wrote…

These and the following if statement should probably be their own function.

Done.

---

Source/Core/VideoBackends/Vulkan/StateTracker.cpp, line 174 at r1 (raw file):

Previously, lioncash (Mat M.) wrote…

You can use std::make_unique here.

Done.

---

Comments from Reviewable

[degasus]

Sounds fine.

---

Reviewed 2 of 5 files at r1, 5 of 5 files at r2.
Review status: all files reviewed at latest revision, 10 unresolved discussions.

---

Source/Core/VideoBackends/Vulkan/ObjectCache.cpp, line 291 at r2 (raw file):

VkPipeline ObjectCache::GetPipeline(const PipelineInfo& info)
{
  auto iter = m_pipeline_objects.find(info);

return GetPipelineWithCacheResult(info).first;

Do you need both functions?

---

Source/Core/VideoBackends/Vulkan/StateTracker.cpp, line 140 at r2 (raw file):

  // OpenAndRead calls Close() first, which will flush all data to disk when reloading.
  // This assertion must hold true, otherwise data corruption will result.
  m_uid_cache.OpenAndRead(filename, inserter);

Feel free to change the API of our LinearDiskCache to use a std::function ;)

---

Source/Core/VideoBackends/Vulkan/StateTracker.cpp, line 154 at r2 (raw file):

  sinfo.ps_uid = m_ps_uid;
  sinfo.primitive_topology = info.primitive_topology;
  sinfo.bbox_enabled = m_bbox_enabled;

Out of curiosity, is this the global config option, for the hardware support, or the global state? I wonder why this isn't within the ps_uid.

---

Source/Core/VideoBackends/Vulkan/StateTracker.cpp, line 923 at r2 (raw file):

    // the existing state, since we don't want to break the next draw.
    PipelineInfo temp_info = GetAlphaPassPipeline(m_pipeline_state);
    auto result = g_object_cache->GetPipelineWithCacheResult(temp_info);

Should there be a function for this code as it's used twice?

auto result = g_object_cache->GetPipelineWithCacheResult(STATE);
m_pipeline_object = result.first;
if (m_pipeline_object == VK_NULL_HANDLE)
  return false;

 
// Add to the UID cache if it is a new pipeline.
if (!result.second)
AppendToPipelineUIDCache(STATE);

---

Comments from Reviewable

[stenzek]

Review status: 3 of 7 files reviewed at latest revision, 10 unresolved discussions.

---

Source/Core/VideoBackends/Vulkan/ObjectCache.cpp, line 291 at r2 (raw file):

Previously, degasus (Markus Wick) wrote…

return GetPipelineWithCacheResult(info).first;

Do you need both functions?

I've simplified the GetPipeline() method. I originally just had it as an out parameter of GetPipeline, but following Lioncash's recommendation I used a pair return instead. This was the cleanest way I could think of accomplishing this without modifying all the callers (which all except one don't care about whether it was a cache miss).

---

Source/Core/VideoBackends/Vulkan/StateTracker.cpp, line 140 at r2 (raw file):

Previously, degasus (Markus Wick) wrote…

Feel free to change the API of our LinearDiskCache to use a std::function ;)

Sounds like a good idea, outside of the scope of this PR I think.

---

Source/Core/VideoBackends/Vulkan/StateTracker.cpp, line 154 at r2 (raw file):

Previously, degasus (Markus Wick) wrote…

Out of curiosity, is this the global config option, for the hardware support, or the global state? I wonder why this isn't within the ps_uid.

It's an variable here because I use a different pipeline layout depending on whether bbox is enabled or not (avoiding the need to bind the shader storage buffer). But you're right about it being part of the ps_uid, I've simplified things and used that field instead. Could probably remove the boolean from StateTracker too, later on.

---

Source/Core/VideoBackends/Vulkan/StateTracker.cpp, line 923 at r2 (raw file):

Previously, degasus (Markus Wick) wrote…

Should there be a function for this code as it's used twice?

auto result = g_object_cache->GetPipelineWithCacheResult(STATE);
m_pipeline_object = result.first;
if (m_pipeline_object == VK_NULL_HANDLE)
  return false;

 
// Add to the UID cache if it is a new pipeline.
if (!result.second)
AppendToPipelineUIDCache(STATE);

Done.

---

Comments from Reviewable

[degasus]

---

Reviewed 4 of 4 files at r3.
Review status: all files reviewed at latest revision, 6 unresolved discussions.

---

Comments from Reviewable