remove double locking. small perf boost

CMakeLists.txt

@@ -433,13 +433,6 @@ add_executable(resource_tracking_tests EXCLUDE_FROM_ALL
 target_link_libraries(resource_tracking_tests fmt::fmt)
 target_include_directories(resource_tracking_tests PRIVATE ${inc_headers})
 
-add_executable(resource_mutex_tests EXCLUDE_FROM_ALL
-	"${KYTY_TESTS_DIR}/ResourceMutexTests.cpp"
-	"${KYTY_SOURCE_DIR}/graphics/host_gpu/renderer/cache/resourceMutex.cpp"
-)
-target_link_libraries(resource_mutex_tests common)
-target_include_directories(resource_mutex_tests PRIVATE ${inc_headers})
-
 add_executable(audio_out2_port_tests EXCLUDE_FROM_ALL
 	"${KYTY_TESTS_DIR}/AudioOut2PortTests.cpp"
 	"${KYTY_SOURCE_DIR}/libs/libAudio2.cpp"
@@ -515,7 +508,6 @@ if(BUILD_TESTING)
 	add_test(NAME shader_vertex_metadata COMMAND $<TARGET_FILE:shader_vertex_metadata_tests>)
 	add_test(NAME shader_stage_runtime COMMAND $<TARGET_FILE:shader_stage_runtime_tests>)
 	add_test(NAME resource_tracking COMMAND $<TARGET_FILE:resource_tracking_tests>)
-	add_test(NAME resource_mutex COMMAND $<TARGET_FILE:resource_mutex_tests>)
 	add_test(NAME event_queue_lifetime COMMAND $<TARGET_FILE:event_queue_lifetime_tests>)
 	add_test(NAME sync_on_address COMMAND $<TARGET_FILE:sync_on_address_tests>)
 	add_test(NAME audio_out2_port COMMAND $<TARGET_FILE:audio_out2_port_tests>)
@@ -565,7 +557,6 @@ if(BUILD_TESTING)
 		shader_vertex_metadata_tests
 		shader_stage_runtime_tests
 		resource_tracking_tests
-		resource_mutex_tests
 		event_queue_lifetime_tests
 		sync_on_address_tests
 		shader_recompiler_compute_tests

src/graphics/host_gpu/renderer/cache/bufferCache.cpp

@@ -5,7 +5,6 @@
 #include "common/profiler.h"
 #include "graphics/guest_gpu/graphicsRun.h"
 #include "graphics/host_gpu/graphicContext.h"
-#include "graphics/host_gpu/renderer/cache/resourceMutex.h"
 #include "graphics/host_gpu/renderer/cache/textureCache.h"
 #include "graphics/host_gpu/renderer/commandScheduler.h"
 #include "graphics/host_gpu/renderer/render.h"
@@ -220,8 +219,7 @@ void BufferCache::QueueGarbageDownload(std::span<const DownloadCopy> copies, Ret
 }
 
 BufferCache::BufferCache(GraphicContext& graphics, CommandScheduler& scheduler,
-                         PageManager& page_manager, TextureCache& texture_cache,
+                         PageManager& page_manager, TextureCache& texture_cache)
-                         ResourceMutex& resource_mutex)
     : m_graphics(graphics), m_scheduler(scheduler),
       m_gds_buffer(graphics, scheduler, MemoryUsage::Stream, 0, AllFlags, GdsBufferSize),
       m_memory_tracker(page_manager),
@@ -229,7 +227,7 @@ BufferCache::BufferCache(GraphicContext& graphics, CommandScheduler& scheduler,
       m_stream_buffer(graphics, scheduler, MemoryUsage::Stream, 64 * MiB),
       m_download_buffer(graphics, scheduler, MemoryUsage::Download, 32 * MiB),
       m_device_buffer(graphics, scheduler, MemoryUsage::DeviceLocal, 128 * MiB),
-      m_texture_cache(texture_cache), m_resource_mutex(resource_mutex) {
+      m_texture_cache(texture_cache) {
 	std::memset(m_gds_buffer.Mapped().data(), 0, static_cast<size_t>(m_gds_buffer.Size()));
 	m_gds_buffer.Flush(0, m_gds_buffer.Size());
 	if (!m_graphics.CanReportMemoryUsage()) {
@@ -304,11 +302,6 @@ void BufferCache::ReadMemory(uint64_t vaddr, uint64_t size, bool is_write) {
 		     "addr=0x%016" PRIx64 " size=0x%016" PRIx64 "\n",
 		     vaddr, size);
 	}
-	if (m_resource_mutex.IsOwnedByCurrentThread()) {
-		EXIT("unsupported buffer readback from a pre-owned resource transaction, "
-		     "addr=0x%016" PRIx64 " size=0x%016" PRIx64 "\n",
-		     vaddr, size);
-	}
 	m_scheduler.Context().GetGpu().SendCommandSync(
 	    [this, vaddr, size, is_write] { ReadMemoryOnGpu(vaddr, size, is_write); });
 }
@@ -529,7 +522,6 @@ BufferBinding BufferCache::ObtainBuffer(CommandBuffer& command, uint64_t vaddr,
 	if (command.IsInvalid() || command.IsExecute()) {
 		EXIT("BufferCache: buffer request requires a recording command buffer\n");
 	}
-	std::lock_guard transaction(m_resource_mutex);
 
 	if (is_read && !is_written && size <= CACHING_PAGE_SIZE &&
 	    !m_memory_tracker.IsRegionGpuModified(vaddr, size) &&
@@ -556,25 +548,33 @@ BufferBinding BufferCache::ObtainBuffer(CommandBuffer& command, uint64_t vaddr,
 		(void)m_texture_cache.InvalidateMemoryFromGPU(vaddr, size, true);
 	}
 
-	FaultSafeCacheLock                         lock(this, m_mutex);
+	std::shared_ptr<Buffer> owner;
-	auto&                                      cached = GetOrCreateBuffer(command, vaddr, size);
+	uint64_t                offset = 0;
-	std::vector<std::pair<uint64_t, uint64_t>> uploads;
+	{
-	m_memory_tracker.ForEachUploadRange(
+		FaultSafeCacheLock                         lock(this, m_mutex);
-	    vaddr, size, is_written,
+		auto&                                      cached = GetOrCreateBuffer(command, vaddr, size);
-	    [&](uint64_t address, uint64_t bytes) noexcept { uploads.emplace_back(address, bytes); },
+		std::vector<std::pair<uint64_t, uint64_t>> uploads;
-	    [&]() noexcept {
+		m_memory_tracker.ForEachUploadRange(
-		    for (const auto& [address, bytes]: uploads) {
+		    vaddr, size, is_written,
-			    Upload(command, *cached.buffer, cached.buffer->Offset(address),
+		    [&](uint64_t address, uint64_t bytes) noexcept {
-			           reinterpret_cast<const void*>(address), bytes);
+			    uploads.emplace_back(address, bytes);
-		    }
+		    },
-	    });
+		    [&]() noexcept {
-	if (is_written) {
+			    for (const auto& [address, bytes]: uploads) {
-		m_gpu_modified_ranges.Add(vaddr, size);
+				    Upload(command, *cached.buffer, cached.buffer->Offset(address),
+				           reinterpret_cast<const void*>(address), bytes);
+			    }
+		    });
+		if (is_written) {
+			m_gpu_modified_ranges.Add(vaddr, size);
+		}
+		owner  = cached.buffer;
+		offset = cached.buffer->Offset(vaddr);
 	}
 	if (is_formatted && is_read && !is_written) {
-		(void)SynchronizeBufferFromImage(*cached.buffer, vaddr, size);
+		(void)SynchronizeBufferFromImage(*owner, vaddr, size);
 	}
-	return {cached.buffer, cached.buffer->Handle(), cached.buffer->Offset(vaddr)};
+	return {owner, owner->Handle(), offset};
 }
 
 std::shared_ptr<Buffer> BufferCache::ObtainNullBuffer() {
@@ -750,8 +750,7 @@ void BufferCache::FillBuffer(uint64_t vaddr, uint64_t size, uint32_t value, bool
 	}
 	(void)m_texture_cache.ClearMeta(vaddr);
 	{
-		std::lock_guard transaction(m_resource_mutex);
+		const auto region = m_texture_cache.QueryRegion(vaddr, size);
-		const auto      region = m_texture_cache.QueryRegion(vaddr, size);
 		if (!HasGpuDirtyBytes(vaddr, size) && !region.gpu_image_bytes) {
 			if (region.image_bytes) {
 				m_texture_cache.InvalidateMemory(vaddr, size);
@@ -790,8 +789,7 @@ void BufferCache::CopyBuffer(uint64_t dst_vaddr, uint64_t src_vaddr, uint64_t si
 		EXIT("BufferCache: invalid or overlapping copy range\n");
 	}
 	if (src_memory || dst_memory) {
-		std::lock_guard transaction(m_resource_mutex);
+		const auto src_region =
-		const auto      src_region =
 		    src_memory ? m_texture_cache.QueryRegion(src_vaddr, size) : TextureCache::RegionInfo {};
 		const auto dst_region =
 		    dst_memory ? m_texture_cache.QueryRegion(dst_vaddr, size) : TextureCache::RegionInfo {};
@@ -870,8 +868,7 @@ bool BufferCache::IsRegionCpuModified(uint64_t vaddr, uint64_t size) {
 }
 
 void BufferCache::RunGarbageCollector() {
-	std::lock_guard transaction(m_resource_mutex);
+	const auto tick = m_gc_tick++;
-	const auto      tick = m_gc_tick++;
 	if (m_graphics.CanReportMemoryUsage()) {
 		m_total_used_memory = m_graphics.GetDeviceMemoryUsage();
 	}

src/graphics/host_gpu/renderer/cache/bufferCache.h

@@ -20,7 +20,6 @@ struct GraphicContext;
 class CommandBuffer;
 class CommandScheduler;
 class TextureCache;
-class ResourceMutex;
 
 struct BufferBinding {
 	std::shared_ptr<void> owner;
@@ -41,7 +40,7 @@ public:
 	}
 
 	BufferCache(GraphicContext& graphics, CommandScheduler& scheduler, PageManager& page_manager,
-	            TextureCache& texture_cache, ResourceMutex& resource_mutex);
+	            TextureCache& texture_cache);
 	~BufferCache();
 	KYTY_CLASS_NO_COPY(BufferCache);
 
@@ -112,7 +111,6 @@ private:
 	StreamBuffer                                      m_download_buffer;
 	StreamBuffer                                      m_device_buffer;
 	TextureCache&                                     m_texture_cache;
-	ResourceMutex&                                    m_resource_mutex;
 	uint64_t                                          m_total_used_memory = 0;
 	uint64_t m_trigger_gc_memory  = 1ull * 1024 * 1024 * 1024;
 	uint64_t m_critical_gc_memory = 2ull * 1024 * 1024 * 1024;

src/graphics/host_gpu/renderer/cache/gpuResourceManager.cpp

@@ -6,9 +6,8 @@
 namespace Libs::Graphics {
 
 GpuResourceManager::GpuResourceManager(GraphicContext& graphics, CommandScheduler& scheduler)
-    : m_scheduler(scheduler),
+    : m_scheduler(scheduler), m_buffer_cache(graphics, scheduler, m_page_manager, m_texture_cache),
-      m_buffer_cache(graphics, scheduler, m_page_manager, m_texture_cache, m_resource_mutex),
+      m_texture_cache(graphics, scheduler, m_page_manager, m_buffer_cache) {}
-      m_texture_cache(graphics, scheduler, m_page_manager, m_buffer_cache, m_resource_mutex) {}
 
 GpuResourceManager::~GpuResourceManager() = default;
 
@@ -58,11 +57,6 @@ void GpuResourceManager::UnmapMemory(uint64_t vaddr, uint64_t size) {
 		     "addr=0x%016" PRIx64 " size=0x%016" PRIx64 "\n",
 		     vaddr, size);
 	}
-	if (m_resource_mutex.IsOwnedByCurrentThread()) {
-		EXIT("unsupported memory unmap from a pre-owned resource transaction, "
-		     "addr=0x%016" PRIx64 " size=0x%016" PRIx64 "\n",
-		     vaddr, size);
-	}
 	const auto unmap = [this, vaddr, size] {
 		if (m_scheduler.Active()) {
 			const auto tick = m_scheduler.CurrentTick();

src/graphics/host_gpu/renderer/cache/gpuResourceManager.h

@@ -5,7 +5,6 @@
 #include "common/common.h"
 #include "graphics/host_gpu/pageManager.h"
 #include "graphics/host_gpu/renderer/cache/bufferCache.h"
-#include "graphics/host_gpu/renderer/cache/resourceMutex.h"
 #include "graphics/host_gpu/renderer/cache/textureCache.h"
 
 #include <cstdint>
@@ -35,7 +34,6 @@ public:
 
 private:
 	PageManager               m_page_manager;
-	ResourceMutex             m_resource_mutex;
 	CommandScheduler&         m_scheduler;
 	BufferCache               m_buffer_cache;
 	TextureCache              m_texture_cache;

src/graphics/host_gpu/renderer/cache/resourceMutex.cpp

@@ -1,47 +0,0 @@
-#include "graphics/host_gpu/renderer/cache/resourceMutex.h"
-
-#include "common/assert.h"
-
-namespace Libs::Graphics {
-
-ResourceMutex::~ResourceMutex() {
-	std::lock_guard state(m_state);
-	if (m_resource_owner != std::thread::id {}) {
-		EXIT("ResourceMutex destroyed with an active owner\n");
-	}
-}
-
-void ResourceMutex::lock() {
-	const auto current = std::this_thread::get_id();
-	{
-		std::lock_guard state(m_state);
-		if (m_resource_owner == current) {
-			EXIT("recursive resource transaction\n");
-		}
-	}
-	m_resource.lock();
-	std::lock_guard state(m_state);
-	if (m_resource_owner != std::thread::id {}) {
-		EXIT("resource mutex acquired with a stale owner\n");
-	}
-	m_resource_owner = current;
-}
-
-void ResourceMutex::unlock() {
-	const auto current = std::this_thread::get_id();
-	{
-		std::lock_guard state(m_state);
-		if (m_resource_owner != current) {
-			EXIT("resource transaction released without ownership\n");
-		}
-		m_resource_owner = {};
-	}
-	m_resource.unlock();
-}
-
-bool ResourceMutex::IsOwnedByCurrentThread() {
-	std::lock_guard state(m_state);
-	return m_resource_owner == std::this_thread::get_id();
-}
-
-} // namespace Libs::Graphics

src/graphics/host_gpu/renderer/cache/resourceMutex.h

@@ -1,30 +0,0 @@
-#ifndef EMULATOR_SRC_GRAPHICS_HOST_GPU_RENDERER_RESOURCEMUTEX_H_
-#define EMULATOR_SRC_GRAPHICS_HOST_GPU_RENDERER_RESOURCEMUTEX_H_
-
-#include "common/common.h"
-
-#include <mutex>
-#include <thread>
-
-namespace Libs::Graphics {
-
-// Owner-tracked shared buffer/image transaction.
-class ResourceMutex final {
-public:
-	ResourceMutex() = default;
-	~ResourceMutex();
-	KYTY_CLASS_NO_COPY(ResourceMutex);
-
-	void               lock();
-	void               unlock();
-	[[nodiscard]] bool IsOwnedByCurrentThread();
-
-private:
-	std::mutex      m_resource;
-	std::mutex      m_state;
-	std::thread::id m_resource_owner;
-};
-
-} // namespace Libs::Graphics
-
-#endif // EMULATOR_SRC_GRAPHICS_HOST_GPU_RENDERER_RESOURCEMUTEX_H_

src/graphics/host_gpu/renderer/cache/textureCache.cpp

@@ -8,7 +8,6 @@
 #include "graphics/guest_gpu/tile.h"
 #include "graphics/host_gpu/graphicContext.h"
 #include "graphics/host_gpu/renderer/cache/bufferCache.h"
-#include "graphics/host_gpu/renderer/cache/resourceMutex.h"
 #include "graphics/host_gpu/renderer/commandScheduler.h"
 #include "graphics/host_gpu/renderer/image/imageView.h"
 #include "graphics/host_gpu/renderer/image/textureCommon.h"
@@ -56,13 +55,12 @@ private:
 } // namespace
 
 TextureCache::TextureCache(GraphicContext& graphics, CommandScheduler& scheduler,
-                           PageManager& page_manager, BufferCache& buffer_cache,
+                           PageManager& page_manager, BufferCache& buffer_cache)
-                           ResourceMutex& resource_mutex)
     : m_graphics(graphics), m_scheduler(scheduler), m_page_manager(page_manager),
       m_blit_helper(graphics, scheduler),
       m_tiler(std::make_unique<TileManager>(graphics, scheduler,
                                             buffer_cache.GetUtilityBuffer(MemoryUsage::Stream))),
-      m_buffer_cache(buffer_cache), m_resource_mutex(resource_mutex),
+      m_buffer_cache(buffer_cache),
       m_readback_linear_images(Config::ReadbackLinearImagesEnabled()) {
 	if (m_graphics.CanReportMemoryUsage()) {
 		constexpr int64_t GiB = 1024ll * 1024 * 1024;
@@ -434,9 +432,8 @@ void TextureCache::UntrackImageTail(ImageId id) {
 }
 
 void TextureCache::TrackImageDownload(ImageId id) {
-	std::lock_guard transaction(m_resource_mutex);
+	CacheLock lock(*this, m_lock);
-	CacheLock       lock(*this, m_lock);
+	auto&     image = ResolveImage(id);
-	auto&           image = ResolveImage(id);
 	TrackImageDownloadLocked(id, image);
 }
 
@@ -1146,8 +1143,7 @@ void TextureCache::RefreshImage(ImageId id, const ImageDesc& desc) {
 }
 
 void TextureCache::AssociateStencil(ImageId depth_id, GuestRange stencil) {
-	std::lock_guard transaction(m_resource_mutex);
+	CacheLock lock(*this, m_lock);
-	CacheLock       lock(*this, m_lock);
 	AssociateStencilLocked(depth_id, stencil);
 }
 
@@ -1191,9 +1187,8 @@ ImageId TextureCache::FindImage(ImageDesc& desc, bool exact_format) {
 
 	ImageId result {};
 	{
-		std::lock_guard transaction(m_resource_mutex);
+		CacheLock  lock(*this, m_lock);
-		CacheLock       lock(*this, m_lock);
+		const auto candidates =
-		const auto      candidates =
 		    FindImagesInRegion(desc.info.data.address, desc.info.data.size, false);
 
 		for (const auto id: candidates) {
@@ -1267,9 +1262,8 @@ ImageId TextureCache::FindImage(ImageDesc& desc, bool exact_format) {
 }
 
 void TextureCache::UpdateImage(ImageId id) {
-	std::lock_guard transaction(m_resource_mutex);
+	CacheLock lock(*this, m_lock);
-	CacheLock       lock(*this, m_lock);
+	auto&     image = ResolveImage(id);
-	auto&           image = ResolveImage(id);
 	TouchImage(image);
 	RefreshImage(id, ImageDesc {.info = image.info, .type = UploadBinding(image)});
 }
@@ -1318,9 +1312,8 @@ ImageId TextureCache::FindImageFromRange(uint64_t address, uint64_t size, bool e
 }
 
 vk::ImageView TextureCache::FindTexture(ImageId id, const ImageDesc& desc) {
-	std::lock_guard transaction(m_resource_mutex);
+	CacheLock lock(*this, m_lock);
-	CacheLock       lock(*this, m_lock);
+	auto&     image = ResolveImage(id);
-	auto&           image = ResolveImage(id);
 	TouchImage(image);
 	if (!image.info.data.Empty()) {
 		if (!image.registered || image.depth_id || image.binding.needs_rebind) {
@@ -1355,9 +1348,8 @@ vk::ImageView TextureCache::FindRenderTarget(ImageId id, const ImageDesc& desc)
 	if (desc.type != BindingType::RenderTarget) {
 		EXIT("TextureCache: invalid color-target binding\n");
 	}
-	std::lock_guard transaction(m_resource_mutex);
+	CacheLock lock(*this, m_lock);
-	CacheLock       lock(*this, m_lock);
+	auto&     image = ResolveImage(id);
-	auto&           image = ResolveImage(id);
 	if (!image.registered || image.depth_id || image.binding.needs_rebind) {
 		EXIT("TextureCache: color target requires rediscovery before final acquisition\n");
 	}
@@ -1376,9 +1368,8 @@ vk::ImageView TextureCache::FindDepthTarget(ImageId id, const ImageDesc& desc) {
 	if (desc.type != BindingType::DepthTarget) {
 		EXIT("TextureCache: invalid depth-target binding\n");
 	}
-	std::lock_guard transaction(m_resource_mutex);
+	CacheLock lock(*this, m_lock);
-	CacheLock       lock(*this, m_lock);
+	auto&     image = ResolveImage(id);
-	auto&           image = ResolveImage(id);
 	if (!image.registered || image.depth_id || image.binding.needs_rebind) {
 		EXIT("TextureCache: depth target requires rediscovery before final acquisition\n");
 	}
@@ -1401,9 +1392,8 @@ vk::ImageView TextureCache::FindDepthTarget(ImageId id, const ImageDesc& desc) {
 }
 
 void TextureCache::MarkGpuWritten(ImageId id) {
-	std::lock_guard transaction(m_resource_mutex);
+	CacheLock lock(*this, m_lock);
-	CacheLock       lock(*this, m_lock);
+	auto&     image = ResolveImage(id);
-	auto&           image = ResolveImage(id);
 	if (!image.registered || image.depth_id) {
 		EXIT("TextureCache: cannot mark an unavailable image GPU-written\n");
 	}
@@ -1427,7 +1417,6 @@ bool TextureCache::ClearImageFromBuffer(CommandBuffer& command, uint64_t address
 	if (command.IsInvalid() || !GuestRange {address, size}.Valid()) {
 		EXIT("TextureCache: invalid image clear\n");
 	}
-	std::lock_guard      transaction(m_resource_mutex);
 	CacheLock            lock(*this, m_lock);
 	ImageId              selected {};
 	vk::ImageAspectFlags aspect {};
@@ -1885,9 +1874,8 @@ bool TextureCache::IsMetaCleared(uint64_t address, uint32_t slice) {
 }
 
 bool TextureCache::ClearMeta(uint64_t address) {
-	std::lock_guard transaction(m_resource_mutex);
+	CacheLock  lock(*this, m_lock);
-	CacheLock       lock(*this, m_lock);
+	const auto found = m_surface_metas.find(address);
-	const auto      found = m_surface_metas.find(address);
 	if (found == m_surface_metas.end()) {
 		return false;
 	}
@@ -1913,9 +1901,8 @@ void TextureCache::UnmapMemory(uint64_t address, uint64_t size) {
 	if (!GuestRange {address, size}.Valid()) {
 		EXIT("TextureCache: invalid unmap range\n");
 	}
-	std::lock_guard transaction(m_resource_mutex);
+	CacheLock lock(*this, m_lock);
-	CacheLock       lock(*this, m_lock);
+	auto      images = FindImagesInRegion(address, size, false);
-	auto            images = FindImagesInRegion(address, size, false);
 	if (!images.empty()) {
 		m_scheduler.Finish();
 	}
@@ -1932,9 +1919,8 @@ void TextureCache::UnmapMemory(uint64_t address, uint64_t size) {
 }
 
 void TextureCache::RunGarbageCollector() {
-	std::lock_guard transaction(m_resource_mutex);
+	CacheLock      lock(*this, m_lock);
-	CacheLock       lock(*this, m_lock);
+	const uint64_t tick = m_gc_tick++;
-	const uint64_t  tick = m_gc_tick++;
 	if (m_graphics.CanReportMemoryUsage()) {
 		m_total_used_memory = m_graphics.GetDeviceMemoryUsage();
 	}
@@ -1994,8 +1980,7 @@ void TextureCache::RunGarbageCollector() {
 }
 
 void TextureCache::ProcessDownloadImages() {
-	std::lock_guard transaction(m_resource_mutex);
+	CacheLock lock(*this, m_lock);
-	CacheLock       lock(*this, m_lock);
 	for (const auto id: m_download_images) {
 		const auto owner = ResolveOwner(id);
 		if (owner != nullptr && owner->registered && owner->IsGpuModified()) {

src/graphics/host_gpu/renderer/cache/textureCache.h

@@ -26,7 +26,6 @@ class Buffer;
 class BufferCache;
 class CommandBuffer;
 class CommandScheduler;
-class ResourceMutex;
 class RenderExecutor;
 class StreamBuffer;
 class TileManager;
@@ -49,7 +48,7 @@ public:
 	};
 
 	TextureCache(GraphicContext& graphics, CommandScheduler& scheduler, PageManager& page_manager,
-	             BufferCache& buffer_cache, ResourceMutex& resource_mutex);
+	             BufferCache& buffer_cache);
 	~TextureCache();
 	KYTY_CLASS_NO_COPY(TextureCache);
 
@@ -169,7 +168,6 @@ private:
 	BlitHelper                                        m_blit_helper;
 	std::unique_ptr<TileManager>                      m_tiler;
 	BufferCache&                                      m_buffer_cache;
-	ResourceMutex&                                    m_resource_mutex;
 	std::vector<Slot>                                 m_slots;
 	std::vector<uint32_t>                             m_free_slots;
 	ImagePageTable                                    m_image_page_table;

tests/ResourceMutexTests.cpp

@@ -1,120 +0,0 @@
-#include "graphics/host_gpu/renderer/cache/resourceMutex.h"
-
-#include <atomic>
-#include <cstdio>
-#include <cstdlib>
-#include <cstring>
-#include <mutex>
-#include <string>
-#include <thread>
-#include <vector>
-
-#if KYTY_PLATFORM == KYTY_PLATFORM_WINDOWS
-#ifndef NOMINMAX
-#define NOMINMAX
-#endif
-#include <windows.h>
-#undef min
-#undef max
-#endif
-
-namespace {
-
-using Libs::Graphics::ResourceMutex;
-
-void Check(bool value, const char* text) {
-	if (!value) {
-		std::fprintf(stderr, "ResourceMutexTests: failed: %s\n", text);
-		std::abort();
-	}
-}
-
-void YieldMany() {
-	for (uint32_t i = 0; i < 4096; i++) {
-		std::this_thread::yield();
-	}
-}
-
-void TestOwnership() {
-	ResourceMutex mutex;
-	Check(!mutex.IsOwnedByCurrentThread(), "fresh mutex reported an owner");
-	{
-		std::lock_guard lock(mutex);
-		Check(mutex.IsOwnedByCurrentThread(), "owner tracking was not installed");
-	}
-	Check(!mutex.IsOwnedByCurrentThread(), "owner tracking survived unlock");
-}
-
-void TestSerializesTransactions() {
-	ResourceMutex    mutex;
-	std::unique_lock owner(mutex);
-	std::atomic_bool contender_started {false};
-	std::atomic_bool contender_entered {false};
-	std::thread      contender([&] {
-		contender_started.store(true, std::memory_order_release);
-		std::lock_guard lock(mutex);
-		contender_entered.store(true, std::memory_order_release);
-	});
-	while (!contender_started.load(std::memory_order_acquire)) {
-		std::this_thread::yield();
-	}
-	YieldMany();
-	Check(!contender_entered.load(std::memory_order_acquire),
-	      "contender entered an active resource transaction");
-	owner.unlock();
-	contender.join();
-	Check(contender_entered.load(std::memory_order_acquire),
-	      "contender did not resume after resource transaction");
-}
-
-#if KYTY_PLATFORM == KYTY_PLATFORM_WINDOWS
-[[noreturn]] void RunDeathCase() {
-	ResourceMutex mutex;
-	std::lock_guard first(mutex);
-	std::lock_guard second(mutex);
-	std::_Exit(0x7f);
-}
-
-void CheckDeathCase() {
-	char path[MAX_PATH] {};
-	Check(GetModuleFileNameA(nullptr, path, MAX_PATH) != 0, "GetModuleFileName failed");
-	std::string       command = std::string("\"") + path + "\" --death";
-	std::vector<char> mutable_command(command.begin(), command.end());
-	mutable_command.push_back('\0');
-	STARTUPINFOA        startup {sizeof(startup)};
-	PROCESS_INFORMATION process {};
-	Check(CreateProcessA(nullptr, mutable_command.data(), nullptr, nullptr, FALSE, CREATE_NO_WINDOW,
-	                     nullptr, nullptr, &startup, &process) != 0,
-	      "CreateProcess failed");
-	const auto wait = WaitForSingleObject(process.hProcess, 10000);
-	if (wait != WAIT_OBJECT_0) {
-		TerminateProcess(process.hProcess, 0x7e);
-	}
-	Check(wait == WAIT_OBJECT_0, "ResourceMutex death test timed out");
-	DWORD exit_code = 0;
-	Check(GetExitCodeProcess(process.hProcess, &exit_code) != 0 && exit_code == 321,
-	      "ResourceMutex death path used the wrong exit");
-	CloseHandle(process.hThread);
-	CloseHandle(process.hProcess);
-}
-#endif
-
-} // namespace
-
-int main(int argc, char** argv) {
-#if KYTY_PLATFORM == KYTY_PLATFORM_WINDOWS
-	if (argc == 2 && std::strcmp(argv[1], "--death") == 0) {
-		RunDeathCase();
-	}
-#else
-	(void)argc;
-	(void)argv;
-#endif
-	TestOwnership();
-	TestSerializesTransactions();
-#if KYTY_PLATFORM == KYTY_PLATFORM_WINDOWS
-	CheckDeathCase();
-#endif
-	std::puts("ResourceMutexTests: all cases passed");
-	return 0;
-}

tests/ShaderRecompilerComputeTests.cpp

@@ -16,7 +16,6 @@
 #include "graphics/host_gpu/pageManager.h"
 #include "graphics/host_gpu/renderer/cache/bufferCache.h"
 #include "graphics/host_gpu/renderer/cache/gpuResourceManager.h"
-#include "graphics/host_gpu/renderer/cache/resourceMutex.h"
 #include "graphics/host_gpu/renderer/cache/textureCache.h"
 #include "graphics/host_gpu/renderer/colorRenderTarget.h"
 #include "graphics/host_gpu/renderer/depthRenderTarget.h"
@@ -71,6 +70,7 @@
 #include <initializer_list>
 #include <limits>
 #include <memory>
+#include <mutex>
 #include <numeric>
 #include <optional>
 #include <semaphore>
@@ -169,6 +169,10 @@ struct TextureCacheTestAccess {
 	static_assert(TextureCache::ImagePageTable::kAddressSpaceBits == 40);
 	static_assert(TextureCache::ImagePageTable::kFirstLevelBits == 10);
 
+	static std::unique_lock<TrackingSpinLock> Lock(TextureCache& cache) {
+		return std::unique_lock(cache.m_lock);
+	}
+
 	static void ConfigureGarbageCollection(TextureCache& cache, std::span<const ImageId> oldest,
 	                                       uint64_t tick, uint64_t pressure) {
 		cache.m_trigger_gc_memory  = 0;
@@ -261,13 +265,11 @@ struct TextureCacheTestAccess {
 	}
 
 	static ImageId InsertImage(TextureCache& cache, const ImageInfo& info) {
-		std::lock_guard transaction(cache.m_resource_mutex);
 		std::lock_guard lock(cache.m_lock);
 		return cache.InsertImage(info);
 	}
 
 	static void DeleteImage(TextureCache& cache, ImageId id) {
-		std::lock_guard transaction(cache.m_resource_mutex);
 		std::lock_guard lock(cache.m_lock);
 		cache.DeleteImage(id);
 	}
@@ -3021,8 +3023,41 @@ public:
 			        "mip, rejected a fitting mip "
 			        "prefix, or transferred ownership");
 
-			auto mip_formatted = resources.GetBufferCache().ObtainBuffer(
+			auto& buffer_cache = resources.GetBufferCache();
-			    command, mip_desc.info.data.address, mip_desc.info.data.size, false, true, true);
+			Require(name, "formatted lock-handoff fixture",
+			        !buffer_cache.HasPageOverlap(mip_desc.info.data.address,
+			                                     mip_desc.info.data.size),
+			        "formatted image already had a cached Buffer owner");
+			BufferBinding         mip_formatted;
+			std::binary_semaphore obtain_entered {0};
+			std::binary_semaphore obtain_complete {0};
+			std::binary_semaphore buffer_visible {0};
+			auto                  texture_lock = TextureCacheTestAccess::Lock(texture_cache);
+			gpu.SendCommand([&] {
+				obtain_entered.release();
+				mip_formatted = buffer_cache.ObtainBuffer(command, mip_desc.info.data.address,
+				                                            mip_desc.info.data.size, false, true, true);
+				obtain_complete.release();
+			});
+			obtain_entered.acquire();
+			std::jthread buffer_probe([&] {
+				while (!buffer_cache.HasPageOverlap(mip_desc.info.data.address,
+				                                    mip_desc.info.data.size)) {
+					std::this_thread::yield();
+				}
+				buffer_visible.release();
+			});
+			const bool buffer_released_before_texture =
+			    buffer_visible.try_acquire_for(std::chrono::seconds(2));
+			texture_lock.unlock();
+			if (!buffer_released_before_texture) {
+				buffer_visible.acquire();
+			}
+			obtain_complete.acquire();
+			buffer_probe.join();
+			Require(name, "formatted cache lock handoff", buffer_released_before_texture,
+			        "formatted Buffer acquisition retained the BufferCache lock while waiting for "
+			        "TextureCache");
 			Require(name, "formatted Buffer path",
 			        mip_formatted.owner != nullptr && mip_formatted.buffer != nullptr &&
 			            texture_cache.GetImage(mip_image).IsGpuModified(),