From 1cc28a176d7220c688d5ab022f0bdf232ecf4844 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 71f4dffb88ed57a8ec156a63389926547731eaa7 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 55feec3b8c35573ff9fa11f544f74d8ccd74bf57 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 3e7c4827ec86fd4eba9b6860bc094938b95d53bf 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 df62a68aba5f8698d13e07ae8783dd29bb05c393 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 41aa8982eb957027d62f0c20bcb3bb30cb78bcf7 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{};