From 1070c020db59af1d3baeca9988ffeda13c286bbb Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 11 Mar 2019 12:02:28 -0400 Subject: [PATCH 1/2] renderer_opengl/gl_global_cache: Append missing override specifiers Two of the functions here are overridden functions, so we can append these specifiers to make it explicit. --- src/video_core/renderer_opengl/gl_global_cache.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/video_core/renderer_opengl/gl_global_cache.h b/src/video_core/renderer_opengl/gl_global_cache.h index 37830bb7cf..b8f1f7c2ed 100644 --- a/src/video_core/renderer_opengl/gl_global_cache.h +++ b/src/video_core/renderer_opengl/gl_global_cache.h @@ -30,12 +30,12 @@ public: explicit CachedGlobalRegion(VAddr addr, u32 size); /// Gets the address of the shader in guest memory, required for cache management - VAddr GetAddr() const { + VAddr GetAddr() const override { return addr; } /// Gets the size of the shader in guest memory, required for cache management - std::size_t GetSizeInBytes() const { + std::size_t GetSizeInBytes() const override { return size; } From 3350c0a77944068010794fe5a53cd63929a3fe28 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 11 Mar 2019 12:07:20 -0400 Subject: [PATCH 2/2] renderer_opengl/gl_global_cache: Replace indexing for assignment with insert_or_assign The previous code had some minor issues with it, really not a big deal, but amending it is basically 'free', so I figured, "why not?". With the standard container maps, when: map[key] = thing; is done, this can cause potentially undesirable behavior in certain scenarios. In particular, if there's no value associated with the key, then the map constructs a default initialized instance of the value type. In this case, since it's a std::shared_ptr (as a type alias) that is the value type, this will construct a std::shared_pointer, and then assign over it (with objects that are quite large, or actively heap allocate this can be extremely undesirable). We also make the function take the region by value, as we can avoid a copy (and by extension with std::shared_ptr, a copy causes an atomic reference count increment), in certain scenarios when ownership isn't a concern (i.e. when ReserveGlobalRegion is called with an rvalue reference, then no copy at all occurs). So, it's more-or-less a "free" gain without many downsides. --- src/video_core/renderer_opengl/gl_global_cache.cpp | 4 ++-- src/video_core/renderer_opengl/gl_global_cache.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/video_core/renderer_opengl/gl_global_cache.cpp b/src/video_core/renderer_opengl/gl_global_cache.cpp index c7f32feaa6..7161d1dea5 100644 --- a/src/video_core/renderer_opengl/gl_global_cache.cpp +++ b/src/video_core/renderer_opengl/gl_global_cache.cpp @@ -57,8 +57,8 @@ GlobalRegion GlobalRegionCacheOpenGL::GetUncachedGlobalRegion(VAddr addr, u32 si return region; } -void GlobalRegionCacheOpenGL::ReserveGlobalRegion(const GlobalRegion& region) { - reserve[region->GetAddr()] = region; +void GlobalRegionCacheOpenGL::ReserveGlobalRegion(GlobalRegion region) { + reserve.insert_or_assign(region->GetAddr(), std::move(region)); } GlobalRegionCacheOpenGL::GlobalRegionCacheOpenGL(RasterizerOpenGL& rasterizer) diff --git a/src/video_core/renderer_opengl/gl_global_cache.h b/src/video_core/renderer_opengl/gl_global_cache.h index b8f1f7c2ed..ba2bdc60c1 100644 --- a/src/video_core/renderer_opengl/gl_global_cache.h +++ b/src/video_core/renderer_opengl/gl_global_cache.h @@ -70,7 +70,7 @@ public: private: GlobalRegion TryGetReservedGlobalRegion(VAddr addr, u32 size) const; GlobalRegion GetUncachedGlobalRegion(VAddr addr, u32 size); - void ReserveGlobalRegion(const GlobalRegion& region); + void ReserveGlobalRegion(GlobalRegion region); std::unordered_map reserve; };