diff --git a/Source/Core/Common/Config/Config.h b/Source/Core/Common/Config/Config.h index ea6058a8d3..e3e3d3df3b 100644 --- a/Source/Core/Common/Config/Config.h +++ b/Source/Core/Common/Config/Config.h @@ -72,10 +72,10 @@ T Get(const Info& info) cached.value = GetUncached(info); cached.config_version = config_version; - info.SetCachedValue(cached); + info.TryToSetCachedValue(cached); } - return cached.value; + return std::move(cached.value); } template diff --git a/Source/Core/Common/Config/ConfigInfo.h b/Source/Core/Common/Config/ConfigInfo.h index e8d904fae2..5c9edb741d 100644 --- a/Source/Core/Common/Config/ConfigInfo.h +++ b/Source/Core/Common/Config/ConfigInfo.h @@ -35,73 +35,58 @@ template class Info { public: - constexpr Info(const Location& location, const T& default_value) - : m_location{location}, m_default_value{default_value}, m_cached_value{default_value, 0} + constexpr Info(Location location, T default_value) + : m_location{std::move(location)}, m_default_value{default_value}, + m_cached_value{std::move(default_value), 0} { } - Info(const Info& other) { *this = other; } - - // Not thread-safe - Info(Info&& other) { *this = std::move(other); } + Info(const Info& other) + : m_location{other.m_location}, m_default_value{other.m_default_value}, + m_cached_value(other.GetCachedValue()) + { + } // Make it easy to convert Info into Info> // so that enum settings can still easily work with code that doesn't care about the enum values. template Enum> Info(const Info& other) + : m_location{other.GetLocation()}, m_default_value{static_cast(other.GetDefaultValue())}, + m_cached_value(other.template GetCachedValueCasted()) { - *this = other; } - Info& operator=(const Info& other) - { - m_location = other.GetLocation(); - m_default_value = other.GetDefaultValue(); - m_cached_value = other.GetCachedValue(); - return *this; - } + ~Info() = default; - // Not thread-safe - Info& operator=(Info&& other) - { - m_location = std::move(other.m_location); - m_default_value = std::move(other.m_default_value); - m_cached_value = std::move(other.m_cached_value); - return *this; - } - - // Make it easy to convert Info into Info> - // so that enum settings can still easily work with code that doesn't care about the enum values. - template Enum> - Info& operator=(const Info& other) - { - m_location = other.GetLocation(); - m_default_value = static_cast(other.GetDefaultValue()); - m_cached_value = other.template GetCachedValueCasted(); - return *this; - } + // Assignments after construction would require more locking to be thread safe. + // It seems unnecessary to have this functionality anyways. + Info& operator=(const Info&) = delete; + Info& operator=(Info&&) = delete; + // Moves are also unnecessary and would be thread unsafe without additional locking. + Info(Info&&) = delete; constexpr const Location& GetLocation() const { return m_location; } constexpr const T& GetDefaultValue() const { return m_default_value; } CachedValue GetCachedValue() const { - std::shared_lock lock(m_cached_value_mutex); + std::shared_lock lk{m_cached_value_mutex}; return m_cached_value; } template CachedValue GetCachedValueCasted() const { - std::shared_lock lock(m_cached_value_mutex); - return CachedValue{static_cast(m_cached_value.value), m_cached_value.config_version}; + std::shared_lock lk{m_cached_value_mutex}; + return {static_cast(m_cached_value.value), m_cached_value.config_version}; } - void SetCachedValue(const CachedValue& cached_value) const + // Only updates if the provided config_version is newer. + void TryToSetCachedValue(CachedValue new_value) const { - std::unique_lock lock(m_cached_value_mutex); - if (m_cached_value.config_version < cached_value.config_version) - m_cached_value = cached_value; + std::lock_guard lk{m_cached_value_mutex}; + if (new_value.config_version > m_cached_value.config_version) + m_cached_value = std::move(new_value); } private: