From f417be9d3be297f5f4ccdf5d31b80394cfdb6c69 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 19 May 2019 02:27:00 -0400 Subject: [PATCH 1/6] gl_shader_disk_cache: Special-case boolean handling Booleans don't have a guaranteed size, but we still want to have them integrate into the disk cache system without needing to actually use a different type. We can do this by supplying non-template overloads for the bool type. Non-template overloads always have precedence during function resolution, so this is safe to provide. This gets rid of the need to smatter ternary conditionals, as well as the need to use u8 types to store the value in. --- .../renderer_opengl/gl_shader_disk_cache.cpp | 43 +++++++++---------- .../renderer_opengl/gl_shader_disk_cache.h | 18 +++++++- 2 files changed, 37 insertions(+), 24 deletions(-) diff --git a/src/video_core/renderer_opengl/gl_shader_disk_cache.cpp b/src/video_core/renderer_opengl/gl_shader_disk_cache.cpp index 254c0d4996..1ba16d4ca5 100644 --- a/src/video_core/renderer_opengl/gl_shader_disk_cache.cpp +++ b/src/video_core/renderer_opengl/gl_shader_disk_cache.cpp @@ -303,12 +303,12 @@ std::optional ShaderDiskCacheOpenGL::LoadDecompiledEn for (u32 i = 0; i < const_buffers_count; ++i) { u32 max_offset{}; u32 index{}; - u8 is_indirect{}; + bool is_indirect{}; if (!LoadObjectFromPrecompiled(max_offset) || !LoadObjectFromPrecompiled(index) || !LoadObjectFromPrecompiled(is_indirect)) { return {}; } - entry.entries.const_buffers.emplace_back(max_offset, is_indirect != 0, index); + entry.entries.const_buffers.emplace_back(max_offset, is_indirect, index); } u32 samplers_count{}; @@ -320,18 +320,17 @@ std::optional ShaderDiskCacheOpenGL::LoadDecompiledEn u64 offset{}; u64 index{}; u32 type{}; - u8 is_array{}; - u8 is_shadow{}; - u8 is_bindless{}; + bool is_array{}; + bool is_shadow{}; + bool is_bindless{}; if (!LoadObjectFromPrecompiled(offset) || !LoadObjectFromPrecompiled(index) || !LoadObjectFromPrecompiled(type) || !LoadObjectFromPrecompiled(is_array) || !LoadObjectFromPrecompiled(is_shadow) || !LoadObjectFromPrecompiled(is_bindless)) { return {}; } - entry.entries.samplers.emplace_back(static_cast(offset), - static_cast(index), - static_cast(type), - is_array != 0, is_shadow != 0, is_bindless != 0); + entry.entries.samplers.emplace_back( + static_cast(offset), static_cast(index), + static_cast(type), is_array, is_shadow, is_bindless); } u32 global_memory_count{}; @@ -342,21 +341,20 @@ std::optional ShaderDiskCacheOpenGL::LoadDecompiledEn for (u32 i = 0; i < global_memory_count; ++i) { u32 cbuf_index{}; u32 cbuf_offset{}; - u8 is_read{}; - u8 is_written{}; + bool is_read{}; + bool is_written{}; if (!LoadObjectFromPrecompiled(cbuf_index) || !LoadObjectFromPrecompiled(cbuf_offset) || !LoadObjectFromPrecompiled(is_read) || !LoadObjectFromPrecompiled(is_written)) { return {}; } - entry.entries.global_memory_entries.emplace_back(cbuf_index, cbuf_offset, is_read != 0, - is_written != 0); + entry.entries.global_memory_entries.emplace_back(cbuf_index, cbuf_offset, is_read, + is_written); } for (auto& clip_distance : entry.entries.clip_distances) { - u8 clip_distance_raw{}; - if (!LoadObjectFromPrecompiled(clip_distance_raw)) + if (!LoadObjectFromPrecompiled(clip_distance)) { return {}; - clip_distance = clip_distance_raw != 0; + } } u64 shader_length{}; @@ -384,7 +382,7 @@ bool ShaderDiskCacheOpenGL::SaveDecompiledFile(u64 unique_identifier, const std: for (const auto& cbuf : entries.const_buffers) { if (!SaveObjectToPrecompiled(static_cast(cbuf.GetMaxOffset())) || !SaveObjectToPrecompiled(static_cast(cbuf.GetIndex())) || - !SaveObjectToPrecompiled(static_cast(cbuf.IsIndirect() ? 1 : 0))) { + !SaveObjectToPrecompiled(cbuf.IsIndirect())) { return false; } } @@ -396,9 +394,9 @@ bool ShaderDiskCacheOpenGL::SaveDecompiledFile(u64 unique_identifier, const std: if (!SaveObjectToPrecompiled(static_cast(sampler.GetOffset())) || !SaveObjectToPrecompiled(static_cast(sampler.GetIndex())) || !SaveObjectToPrecompiled(static_cast(sampler.GetType())) || - !SaveObjectToPrecompiled(static_cast(sampler.IsArray() ? 1 : 0)) || - !SaveObjectToPrecompiled(static_cast(sampler.IsShadow() ? 1 : 0)) || - !SaveObjectToPrecompiled(static_cast(sampler.IsBindless() ? 1 : 0))) { + !SaveObjectToPrecompiled(sampler.IsArray()) || + !SaveObjectToPrecompiled(sampler.IsShadow()) || + !SaveObjectToPrecompiled(sampler.IsBindless())) { return false; } } @@ -409,14 +407,13 @@ bool ShaderDiskCacheOpenGL::SaveDecompiledFile(u64 unique_identifier, const std: for (const auto& gmem : entries.global_memory_entries) { if (!SaveObjectToPrecompiled(static_cast(gmem.GetCbufIndex())) || !SaveObjectToPrecompiled(static_cast(gmem.GetCbufOffset())) || - !SaveObjectToPrecompiled(static_cast(gmem.IsRead() ? 1 : 0)) || - !SaveObjectToPrecompiled(static_cast(gmem.IsWritten() ? 1 : 0))) { + !SaveObjectToPrecompiled(gmem.IsRead()) || !SaveObjectToPrecompiled(gmem.IsWritten())) { return false; } } for (const bool clip_distance : entries.clip_distances) { - if (!SaveObjectToPrecompiled(static_cast(clip_distance ? 1 : 0))) { + if (!SaveObjectToPrecompiled(clip_distance)) { return false; } } diff --git a/src/video_core/renderer_opengl/gl_shader_disk_cache.h b/src/video_core/renderer_opengl/gl_shader_disk_cache.h index 0142b2e3be..34d4bd6378 100644 --- a/src/video_core/renderer_opengl/gl_shader_disk_cache.h +++ b/src/video_core/renderer_opengl/gl_shader_disk_cache.h @@ -259,12 +259,28 @@ private: return SaveArrayToPrecompiled(&object, 1); } + bool SaveObjectToPrecompiled(bool object) { + const auto value = static_cast(object); + return SaveArrayToPrecompiled(&value, 1); + } + template bool LoadObjectFromPrecompiled(T& object) { return LoadArrayFromPrecompiled(&object, 1); } - // Copre system + bool LoadObjectFromPrecompiled(bool& object) { + u8 value; + const bool read_ok = LoadArrayFromPrecompiled(&value, 1); + if (!read_ok) { + return false; + } + + object = value != 0; + return true; + } + + // Core system Core::System& system; // Stored transferable shaders std::map> transferable; From 5e4c2276081371913d7a46f0a02292e5d522c3e1 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 19 May 2019 02:33:32 -0400 Subject: [PATCH 2/6] gl_shader_disk_cache: Make variable non-const in decompiled entry case std::move does nothing when applied to a const variable. Resources can't be moved if the object is immutable. With this change, we don't end up making several unnecessary heap allocations and copies. --- src/video_core/renderer_opengl/gl_shader_disk_cache.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/video_core/renderer_opengl/gl_shader_disk_cache.cpp b/src/video_core/renderer_opengl/gl_shader_disk_cache.cpp index 1ba16d4ca5..15b9fc6af3 100644 --- a/src/video_core/renderer_opengl/gl_shader_disk_cache.cpp +++ b/src/video_core/renderer_opengl/gl_shader_disk_cache.cpp @@ -243,7 +243,7 @@ ShaderDiskCacheOpenGL::LoadPrecompiledFile(FileUtil::IOFile& file) { return {}; } - const auto entry = LoadDecompiledEntry(); + auto entry = LoadDecompiledEntry(); if (!entry) { return {}; } From 683c4e523f55cef1f22263010f2c26ed0e277af1 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 19 May 2019 02:39:00 -0400 Subject: [PATCH 3/6] gl_shader_disk_cache: Remove redundant code string construction in LoadDecompiledEntry() We don't need to load the code into a vector and then construct a string over the data. We can just create a string with the necessary size ahead of time, and read the data directly into it, getting rid of an unnecessary heap allocation. --- src/video_core/renderer_opengl/gl_shader_disk_cache.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/video_core/renderer_opengl/gl_shader_disk_cache.cpp b/src/video_core/renderer_opengl/gl_shader_disk_cache.cpp index 15b9fc6af3..4cfb88557e 100644 --- a/src/video_core/renderer_opengl/gl_shader_disk_cache.cpp +++ b/src/video_core/renderer_opengl/gl_shader_disk_cache.cpp @@ -287,13 +287,13 @@ std::optional ShaderDiskCacheOpenGL::LoadDecompiledEn return {}; } - std::vector code(code_size); + std::string code(code_size, '\0'); if (!LoadArrayFromPrecompiled(code.data(), code.size())) { return {}; } ShaderDiskCacheDecompiled entry; - entry.code = std::string(reinterpret_cast(code.data()), code_size); + entry.code = std::move(code); u32 const_buffers_count{}; if (!LoadObjectFromPrecompiled(const_buffers_count)) { From 7fdc644c446b3c710d3e3160ced89dd793485eca Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 19 May 2019 02:42:57 -0400 Subject: [PATCH 4/6] gl_shader_disk_cache: Make hash specializations noexcept The standard library expects hash specializations that don't throw exceptions. Make this explicit in the type to allow selection of better code paths if possible in implementations. --- src/video_core/renderer_opengl/gl_shader_disk_cache.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/video_core/renderer_opengl/gl_shader_disk_cache.h b/src/video_core/renderer_opengl/gl_shader_disk_cache.h index 34d4bd6378..7a7183f210 100644 --- a/src/video_core/renderer_opengl/gl_shader_disk_cache.h +++ b/src/video_core/renderer_opengl/gl_shader_disk_cache.h @@ -70,14 +70,14 @@ namespace std { template <> struct hash { - std::size_t operator()(const OpenGL::BaseBindings& bindings) const { + std::size_t operator()(const OpenGL::BaseBindings& bindings) const noexcept { return bindings.cbuf | bindings.gmem << 8 | bindings.sampler << 16; } }; template <> struct hash { - std::size_t operator()(const OpenGL::ShaderDiskCacheUsage& usage) const { + std::size_t operator()(const OpenGL::ShaderDiskCacheUsage& usage) const noexcept { return static_cast(usage.unique_identifier) ^ std::hash()(usage.bindings) ^ usage.primitive << 16; } From 634b78a4c6db225e71799107da913799d84450cd Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 19 May 2019 02:50:47 -0400 Subject: [PATCH 5/6] gl_shader_disk_cache: Default ShaderDiskCacheOpenGL's destructor in the cpp file Given the disk shader cache contains non-trivial types, we should default it in the cpp file in order to prevent inlining of the complex destruction logic. --- src/video_core/renderer_opengl/gl_shader_disk_cache.cpp | 2 ++ src/video_core/renderer_opengl/gl_shader_disk_cache.h | 1 + 2 files changed, 3 insertions(+) diff --git a/src/video_core/renderer_opengl/gl_shader_disk_cache.cpp b/src/video_core/renderer_opengl/gl_shader_disk_cache.cpp index 4cfb88557e..c446945eed 100644 --- a/src/video_core/renderer_opengl/gl_shader_disk_cache.cpp +++ b/src/video_core/renderer_opengl/gl_shader_disk_cache.cpp @@ -107,6 +107,8 @@ bool ShaderDiskCacheRaw::Save(FileUtil::IOFile& file) const { ShaderDiskCacheOpenGL::ShaderDiskCacheOpenGL(Core::System& system) : system{system}, precompiled_cache_virtual_file_offset{0} {} +ShaderDiskCacheOpenGL::~ShaderDiskCacheOpenGL() = default; + std::optional, std::vector>> ShaderDiskCacheOpenGL::LoadTransferable() { // Skip games without title id diff --git a/src/video_core/renderer_opengl/gl_shader_disk_cache.h b/src/video_core/renderer_opengl/gl_shader_disk_cache.h index 7a7183f210..bafd3b3f2a 100644 --- a/src/video_core/renderer_opengl/gl_shader_disk_cache.h +++ b/src/video_core/renderer_opengl/gl_shader_disk_cache.h @@ -162,6 +162,7 @@ struct ShaderDiskCacheDump { class ShaderDiskCacheOpenGL { public: explicit ShaderDiskCacheOpenGL(Core::System& system); + ~ShaderDiskCacheOpenGL(); /// Loads transferable cache. If file has a old version or on failure, it deletes the file. std::optional, std::vector>> From 0a7f09a99b915042269fac8d117326c20f72e7bf Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 19 May 2019 02:53:38 -0400 Subject: [PATCH 6/6] gl_shader_disk_cache: in-class initialize virtual file offset of ShaderDiskCacheOpenGL Given the offset is assigned a fixed value in the constructor, we can just assign it directly and get rid of the need to write the name of the variable again in the constructor initializer list. --- src/video_core/renderer_opengl/gl_shader_disk_cache.cpp | 3 +-- src/video_core/renderer_opengl/gl_shader_disk_cache.h | 5 ++--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/video_core/renderer_opengl/gl_shader_disk_cache.cpp b/src/video_core/renderer_opengl/gl_shader_disk_cache.cpp index c446945eed..fba9c594af 100644 --- a/src/video_core/renderer_opengl/gl_shader_disk_cache.cpp +++ b/src/video_core/renderer_opengl/gl_shader_disk_cache.cpp @@ -104,8 +104,7 @@ bool ShaderDiskCacheRaw::Save(FileUtil::IOFile& file) const { return true; } -ShaderDiskCacheOpenGL::ShaderDiskCacheOpenGL(Core::System& system) - : system{system}, precompiled_cache_virtual_file_offset{0} {} +ShaderDiskCacheOpenGL::ShaderDiskCacheOpenGL(Core::System& system) : system{system} {} ShaderDiskCacheOpenGL::~ShaderDiskCacheOpenGL() = default; diff --git a/src/video_core/renderer_opengl/gl_shader_disk_cache.h b/src/video_core/renderer_opengl/gl_shader_disk_cache.h index bafd3b3f2a..2da0a4a232 100644 --- a/src/video_core/renderer_opengl/gl_shader_disk_cache.h +++ b/src/video_core/renderer_opengl/gl_shader_disk_cache.h @@ -285,11 +285,10 @@ private: Core::System& system; // Stored transferable shaders std::map> transferable; - // Stores whole precompiled cache which will be read from or saved to the precompiled chache - // file + // Stores whole precompiled cache which will be read from/saved to the precompiled cache file FileSys::VectorVfsFile precompiled_cache_virtual_file; // Stores the current offset of the precompiled cache file for IO purposes - std::size_t precompiled_cache_virtual_file_offset; + std::size_t precompiled_cache_virtual_file_offset = 0; // The cache has been loaded at boot bool tried_to_load{};