From 0cbae39e41bd616120a1c102271a6b837b385e13 Mon Sep 17 00:00:00 2001 From: nmzik Date: Thu, 6 Aug 2026 23:23:28 +0200 Subject: [PATCH] remove double locking. small perf boost --- CMakeLists.txt | 9 -- .../host_gpu/renderer/cache/bufferCache.cpp | 61 +++++---- .../host_gpu/renderer/cache/bufferCache.h | 4 +- .../renderer/cache/gpuResourceManager.cpp | 10 +- .../renderer/cache/gpuResourceManager.h | 2 - .../host_gpu/renderer/cache/resourceMutex.cpp | 47 ------- .../host_gpu/renderer/cache/resourceMutex.h | 30 ----- .../host_gpu/renderer/cache/textureCache.cpp | 63 ++++----- .../host_gpu/renderer/cache/textureCache.h | 4 +- tests/ResourceMutexTests.cpp | 120 ------------------ tests/ShaderRecompilerComputeTests.cpp | 45 ++++++- 11 files changed, 97 insertions(+), 298 deletions(-) delete mode 100644 src/graphics/host_gpu/renderer/cache/resourceMutex.cpp delete mode 100644 src/graphics/host_gpu/renderer/cache/resourceMutex.h delete mode 100644 tests/ResourceMutexTests.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index a03148627..74fa3268c 100644 --- a/CMakeLists.txt +++ b/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 $) add_test(NAME shader_stage_runtime COMMAND $) add_test(NAME resource_tracking COMMAND $) - add_test(NAME resource_mutex COMMAND $) add_test(NAME event_queue_lifetime COMMAND $) add_test(NAME sync_on_address COMMAND $) add_test(NAME audio_out2_port COMMAND $) @@ -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 diff --git a/src/graphics/host_gpu/renderer/cache/bufferCache.cpp b/src/graphics/host_gpu/renderer/cache/bufferCache.cpp index b84bc5ffd..da2597cc5 100644 --- a/src/graphics/host_gpu/renderer/cache/bufferCache.cpp +++ b/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 copies, Ret } BufferCache::BufferCache(GraphicContext& graphics, CommandScheduler& scheduler, - PageManager& page_manager, TextureCache& texture_cache, - ResourceMutex& resource_mutex) + PageManager& page_manager, TextureCache& texture_cache) : 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(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); - auto& cached = GetOrCreateBuffer(command, vaddr, size); - std::vector> uploads; - m_memory_tracker.ForEachUploadRange( - vaddr, size, is_written, - [&](uint64_t address, uint64_t bytes) noexcept { uploads.emplace_back(address, bytes); }, - [&]() noexcept { - for (const auto& [address, bytes]: uploads) { - Upload(command, *cached.buffer, cached.buffer->Offset(address), - reinterpret_cast(address), bytes); - } - }); - if (is_written) { - m_gpu_modified_ranges.Add(vaddr, size); + std::shared_ptr owner; + uint64_t offset = 0; + { + FaultSafeCacheLock lock(this, m_mutex); + auto& cached = GetOrCreateBuffer(command, vaddr, size); + std::vector> uploads; + m_memory_tracker.ForEachUploadRange( + vaddr, size, is_written, + [&](uint64_t address, uint64_t bytes) noexcept { + uploads.emplace_back(address, bytes); + }, + [&]() noexcept { + for (const auto& [address, bytes]: uploads) { + Upload(command, *cached.buffer, cached.buffer->Offset(address), + reinterpret_cast(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 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(); } diff --git a/src/graphics/host_gpu/renderer/cache/bufferCache.h b/src/graphics/host_gpu/renderer/cache/bufferCache.h index 31644e0bd..c9d005d9e 100644 --- a/src/graphics/host_gpu/renderer/cache/bufferCache.h +++ b/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 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; diff --git a/src/graphics/host_gpu/renderer/cache/gpuResourceManager.cpp b/src/graphics/host_gpu/renderer/cache/gpuResourceManager.cpp index 3432a3942..baa1bde32 100644 --- a/src/graphics/host_gpu/renderer/cache/gpuResourceManager.cpp +++ b/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_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_resource_mutex) {} + : m_scheduler(scheduler), m_buffer_cache(graphics, scheduler, m_page_manager, m_texture_cache), + m_texture_cache(graphics, scheduler, m_page_manager, m_buffer_cache) {} 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(); diff --git a/src/graphics/host_gpu/renderer/cache/gpuResourceManager.h b/src/graphics/host_gpu/renderer/cache/gpuResourceManager.h index d9aa17750..501b62253 100644 --- a/src/graphics/host_gpu/renderer/cache/gpuResourceManager.h +++ b/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 @@ -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; diff --git a/src/graphics/host_gpu/renderer/cache/resourceMutex.cpp b/src/graphics/host_gpu/renderer/cache/resourceMutex.cpp deleted file mode 100644 index 6d5545c3e..000000000 --- a/src/graphics/host_gpu/renderer/cache/resourceMutex.cpp +++ /dev/null @@ -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 diff --git a/src/graphics/host_gpu/renderer/cache/resourceMutex.h b/src/graphics/host_gpu/renderer/cache/resourceMutex.h deleted file mode 100644 index b9aaf18d2..000000000 --- a/src/graphics/host_gpu/renderer/cache/resourceMutex.h +++ /dev/null @@ -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 -#include - -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_ diff --git a/src/graphics/host_gpu/renderer/cache/textureCache.cpp b/src/graphics/host_gpu/renderer/cache/textureCache.cpp index ab44e210d..2e5b2a487 100644 --- a/src/graphics/host_gpu/renderer/cache/textureCache.cpp +++ b/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, - ResourceMutex& resource_mutex) + PageManager& page_manager, BufferCache& buffer_cache) : m_graphics(graphics), m_scheduler(scheduler), m_page_manager(page_manager), m_blit_helper(graphics, scheduler), m_tiler(std::make_unique(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); - auto& image = ResolveImage(id); + CacheLock lock(*this, m_lock); + 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); - const auto candidates = + CacheLock lock(*this, m_lock); + 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); - auto& image = ResolveImage(id); + CacheLock lock(*this, m_lock); + 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); - auto& image = ResolveImage(id); + CacheLock lock(*this, m_lock); + 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); - auto& image = ResolveImage(id); + CacheLock lock(*this, m_lock); + 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); - auto& image = ResolveImage(id); + CacheLock lock(*this, m_lock); + 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); - auto& image = ResolveImage(id); + CacheLock lock(*this, m_lock); + 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); - const auto found = m_surface_metas.find(address); + CacheLock lock(*this, m_lock); + 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); - auto images = FindImagesInRegion(address, size, false); + CacheLock lock(*this, m_lock); + 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); - const uint64_t tick = m_gc_tick++; + CacheLock lock(*this, m_lock); + 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()) { diff --git a/src/graphics/host_gpu/renderer/cache/textureCache.h b/src/graphics/host_gpu/renderer/cache/textureCache.h index 892f153d2..7ee55ede6 100644 --- a/src/graphics/host_gpu/renderer/cache/textureCache.h +++ b/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 m_tiler; BufferCache& m_buffer_cache; - ResourceMutex& m_resource_mutex; std::vector m_slots; std::vector m_free_slots; ImagePageTable m_image_page_table; diff --git a/tests/ResourceMutexTests.cpp b/tests/ResourceMutexTests.cpp deleted file mode 100644 index fbde8b8a9..000000000 --- a/tests/ResourceMutexTests.cpp +++ /dev/null @@ -1,120 +0,0 @@ -#include "graphics/host_gpu/renderer/cache/resourceMutex.h" - -#include -#include -#include -#include -#include -#include -#include -#include - -#if KYTY_PLATFORM == KYTY_PLATFORM_WINDOWS -#ifndef NOMINMAX -#define NOMINMAX -#endif -#include -#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 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; -} diff --git a/tests/ShaderRecompilerComputeTests.cpp b/tests/ShaderRecompilerComputeTests.cpp index ac9339ad8..59c3d738d 100644 --- a/tests/ShaderRecompilerComputeTests.cpp +++ b/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 #include #include +#include #include #include #include @@ -169,6 +169,10 @@ struct TextureCacheTestAccess { static_assert(TextureCache::ImagePageTable::kAddressSpaceBits == 40); static_assert(TextureCache::ImagePageTable::kFirstLevelBits == 10); + static std::unique_lock Lock(TextureCache& cache) { + return std::unique_lock(cache.m_lock); + } + static void ConfigureGarbageCollection(TextureCache& cache, std::span 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( - command, mip_desc.info.data.address, mip_desc.info.data.size, false, true, true); + auto& buffer_cache = resources.GetBufferCache(); + 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(),