From 810d8c0890a18db0cf143e62562b0401c39e6685 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 24 Sep 2023 18:03:12 -0700 Subject: [PATCH 1/4] Debugger: Use dedicated func to notify mem copy. --- Core/Debugger/MemBlockInfo.cpp | 8 ++++++ Core/Debugger/MemBlockInfo.h | 1 + Core/HLE/ReplaceTables.cpp | 49 ++++++++++++++++----------------- Core/HLE/sceDmac.cpp | 9 +++--- Core/HLE/sceKernelInterrupt.cpp | 15 ++-------- Core/MemMapHelpers.h | 9 +++--- GPU/GPUCommon.cpp | 18 ++++-------- 7 files changed, 49 insertions(+), 60 deletions(-) diff --git a/Core/Debugger/MemBlockInfo.cpp b/Core/Debugger/MemBlockInfo.cpp index 93f44f41d0ad..92242a81264e 100644 --- a/Core/Debugger/MemBlockInfo.cpp +++ b/Core/Debugger/MemBlockInfo.cpp @@ -484,6 +484,14 @@ void NotifyMemInfo(MemBlockFlags flags, uint32_t start, uint32_t size, const cha NotifyMemInfoPC(flags, start, size, currentMIPS->pc, str, strLength); } +void NotifyMemInfoCopy(uint32_t destPtr, uint32_t srcPtr, uint32_t size, const char *prefix) { + // TODO + char tagData[128]; + size_t tagSize = FormatMemWriteTagAt(tagData, sizeof(tagData), prefix, srcPtr, size); + NotifyMemInfo(MemBlockFlags::READ, srcPtr, size, tagData, tagSize); + NotifyMemInfo(MemBlockFlags::WRITE, destPtr, size, tagData, tagSize); +} + std::vector FindMemInfo(uint32_t start, uint32_t size) { start = NormalizeAddress(start); diff --git a/Core/Debugger/MemBlockInfo.h b/Core/Debugger/MemBlockInfo.h index 108423d53f4b..b07c326f82b0 100644 --- a/Core/Debugger/MemBlockInfo.h +++ b/Core/Debugger/MemBlockInfo.h @@ -53,6 +53,7 @@ struct MemBlockInfo { void NotifyMemInfo(MemBlockFlags flags, uint32_t start, uint32_t size, const char *tag, size_t tagLength); void NotifyMemInfoPC(MemBlockFlags flags, uint32_t start, uint32_t size, uint32_t pc, const char *tag, size_t tagLength); +void NotifyMemInfoCopy(uint32_t destPtr, uint32_t srcPtr, uint32_t size, const char *prefix); // This lets us avoid calling strlen on string constants, instead the string length (including null, // so we have to subtract 1) is computed at compile time. diff --git a/Core/HLE/ReplaceTables.cpp b/Core/HLE/ReplaceTables.cpp index f71b8b945cb2..4695d13926aa 100644 --- a/Core/HLE/ReplaceTables.cpp +++ b/Core/HLE/ReplaceTables.cpp @@ -159,16 +159,19 @@ static int Replace_memcpy() { RETURN(destPtr); if (MemBlockInfoDetailed(bytes)) { - char tagData[128]; - size_t tagSize = FormatMemWriteTagAt(tagData, sizeof(tagData), "ReplaceMemcpy/", srcPtr, bytes); - NotifyMemInfo(MemBlockFlags::READ, srcPtr, bytes, tagData, tagSize); - NotifyMemInfo(MemBlockFlags::WRITE, destPtr, bytes, tagData, tagSize); - // It's pretty common that games will copy video data. - if (!strcmp(tagData, "ReplaceMemcpy/VideoDecode") || !strcmp(tagData, "ReplaceMemcpy/VideoDecodeRange")) { - if (bytes == 512 * 272 * 4) { + // Detect that by manually reading the tag when the size looks right. + if (bytes == 512 * 272 * 4) { + char tagData[128]; + size_t tagSize = FormatMemWriteTagAt(tagData, sizeof(tagData), "ReplaceMemcpy/", srcPtr, bytes); + NotifyMemInfo(MemBlockFlags::READ, srcPtr, bytes, tagData, tagSize); + NotifyMemInfo(MemBlockFlags::WRITE, destPtr, bytes, tagData, tagSize); + + if (!strcmp(tagData, "ReplaceMemcpy/VideoDecode") || !strcmp(tagData, "ReplaceMemcpy/VideoDecodeRange")) { gpu->PerformWriteFormattedFromMemory(destPtr, bytes, 512, GE_FORMAT_8888); } + } else { + NotifyMemInfoCopy(destPtr, srcPtr, bytes, "ReplaceMemcpy/"); } } @@ -212,16 +215,19 @@ static int Replace_memcpy_jak() { RETURN(destPtr); if (MemBlockInfoDetailed(bytes)) { - char tagData[128]; - size_t tagSize = FormatMemWriteTagAt(tagData, sizeof(tagData), "ReplaceMemcpy/", srcPtr, bytes); - NotifyMemInfo(MemBlockFlags::READ, srcPtr, bytes, tagData, tagSize); - NotifyMemInfo(MemBlockFlags::WRITE, destPtr, bytes, tagData, tagSize); - // It's pretty common that games will copy video data. - if (!strcmp(tagData, "ReplaceMemcpy/VideoDecode") || !strcmp(tagData, "ReplaceMemcpy/VideoDecodeRange")) { - if (bytes == 512 * 272 * 4) { + // Detect that by manually reading the tag when the size looks right. + if (bytes == 512 * 272 * 4) { + char tagData[128]; + size_t tagSize = FormatMemWriteTagAt(tagData, sizeof(tagData), "ReplaceMemcpy/", srcPtr, bytes); + NotifyMemInfo(MemBlockFlags::READ, srcPtr, bytes, tagData, tagSize); + NotifyMemInfo(MemBlockFlags::WRITE, destPtr, bytes, tagData, tagSize); + + if (!strcmp(tagData, "ReplaceMemcpy/VideoDecode") || !strcmp(tagData, "ReplaceMemcpy/VideoDecodeRange")) { gpu->PerformWriteFormattedFromMemory(destPtr, bytes, 512, GE_FORMAT_8888); } + } else { + NotifyMemInfoCopy(destPtr, srcPtr, bytes, "ReplaceMemcpy/"); } } @@ -252,10 +258,7 @@ static int Replace_memcpy16() { RETURN(destPtr); if (MemBlockInfoDetailed(bytes)) { - char tagData[128]; - size_t tagSize = FormatMemWriteTagAt(tagData, sizeof(tagData), "ReplaceMemcpy16/", srcPtr, bytes); - NotifyMemInfo(MemBlockFlags::READ, srcPtr, bytes, tagData, tagSize); - NotifyMemInfo(MemBlockFlags::WRITE, destPtr, bytes, tagData, tagSize); + NotifyMemInfoCopy(destPtr, srcPtr, bytes, "ReplaceMemcpy16/"); } return 10 + bytes / 4; // approximation @@ -294,10 +297,7 @@ static int Replace_memcpy_swizzled() { RETURN(0); if (MemBlockInfoDetailed(pitch * h)) { - char tagData[128]; - size_t tagSize = FormatMemWriteTagAt(tagData, sizeof(tagData), "ReplaceMemcpySwizzle/", srcPtr, pitch * h); - NotifyMemInfo(MemBlockFlags::READ, srcPtr, pitch * h, tagData, tagSize); - NotifyMemInfo(MemBlockFlags::WRITE, destPtr, pitch * h, tagData, tagSize); + NotifyMemInfoCopy(destPtr, srcPtr, pitch * h, "ReplaceMemcpySwizzle/"); } return 10 + (pitch * h) / 4; // approximation @@ -326,10 +326,7 @@ static int Replace_memmove() { RETURN(destPtr); if (MemBlockInfoDetailed(bytes)) { - char tagData[128]; - size_t tagSize = FormatMemWriteTagAt(tagData, sizeof(tagData), "ReplaceMemmove/", srcPtr, bytes); - NotifyMemInfo(MemBlockFlags::READ, srcPtr, bytes, tagData, tagSize); - NotifyMemInfo(MemBlockFlags::WRITE, destPtr, bytes, tagData, tagSize); + NotifyMemInfoCopy(destPtr, srcPtr, bytes, "ReplaceMemmove/"); } return 10 + bytes / 4; // approximation diff --git a/Core/HLE/sceDmac.cpp b/Core/HLE/sceDmac.cpp index f7bcf0d0f6f5..8feb1fc89e74 100644 --- a/Core/HLE/sceDmac.cpp +++ b/Core/HLE/sceDmac.cpp @@ -51,12 +51,11 @@ static int __DmacMemcpy(u32 dst, u32 src, u32 size) { } if (!skip && size != 0) { currentMIPS->InvalidateICache(src, size); + if (Memory::IsValidRange(dst, size) && Memory::IsValidRange(src, size)) { + memcpy(Memory::GetPointerWriteUnchecked(dst), Memory::GetPointerUnchecked(src), size); + } if (MemBlockInfoDetailed(size)) { - char tagData[128]; - size_t tagSize = FormatMemWriteTagAt(tagData, sizeof(tagData), "DmacMemcpy/", src, size); - Memory::Memcpy(dst, src, size, tagData, tagSize); - } else { - Memory::Memcpy(dst, src, size, "DmacMemcpy"); + NotifyMemInfoCopy(dst, src, size, "DmacMemcpy/"); } currentMIPS->InvalidateICache(dst, size); } diff --git a/Core/HLE/sceKernelInterrupt.cpp b/Core/HLE/sceKernelInterrupt.cpp index ec4b452a6402..76e1788e397e 100644 --- a/Core/HLE/sceKernelInterrupt.cpp +++ b/Core/HLE/sceKernelInterrupt.cpp @@ -657,10 +657,7 @@ static u32 sceKernelMemcpy(u32 dst, u32 src, u32 size) } if (MemBlockInfoDetailed(size)) { - char tagData[128]; - size_t tagSize = FormatMemWriteTagAt(tagData, sizeof(tagData), "KernelMemcpy/", src, size); - NotifyMemInfo(MemBlockFlags::READ, src, size, tagData, tagSize); - NotifyMemInfo(MemBlockFlags::WRITE, dst, size, tagData, tagSize); + NotifyMemInfoCopy(dst, src, size, "KernelMemcpy/"); } return dst; @@ -693,10 +690,7 @@ static u32 sysclib_memcpy(u32 dst, u32 src, u32 size) { memcpy(Memory::GetPointerWriteUnchecked(dst), Memory::GetPointerUnchecked(src), size); } if (MemBlockInfoDetailed(size)) { - char tagData[128]; - size_t tagSize = FormatMemWriteTagAt(tagData, sizeof(tagData), "KernelMemcpy/", src, size); - NotifyMemInfo(MemBlockFlags::READ, src, size, tagData, tagSize); - NotifyMemInfo(MemBlockFlags::WRITE, dst, size, tagData, tagSize); + NotifyMemInfoCopy(dst, src, size, "KernelMemcpy/"); } return dst; } @@ -797,10 +791,7 @@ static u32 sysclib_memmove(u32 dst, u32 src, u32 size) { memmove(Memory::GetPointerWriteUnchecked(dst), Memory::GetPointerUnchecked(src), size); } if (MemBlockInfoDetailed(size)) { - char tagData[128]; - size_t tagSize = FormatMemWriteTagAt(tagData, sizeof(tagData), "KernelMemmove/", src, size); - NotifyMemInfo(MemBlockFlags::READ, src, size, tagData, tagSize); - NotifyMemInfo(MemBlockFlags::WRITE, dst, size, tagData, tagSize); + NotifyMemInfoCopy(dst, src, size, "KernelMemmove/"); } return 0; } diff --git a/Core/MemMapHelpers.h b/Core/MemMapHelpers.h index 6f2ceaca637f..5f89f60312ff 100644 --- a/Core/MemMapHelpers.h +++ b/Core/MemMapHelpers.h @@ -69,13 +69,12 @@ inline void Memcpy(const u32 to_address, const u32 from_address, const u32 len, memcpy(to, from, len); if (MemBlockInfoDetailed(len)) { - char tagData[128]; if (!tag) { - tagLen = FormatMemWriteTagAt(tagData, sizeof(tagData), "Memcpy/", from_address, len); - tag = tagData; + NotifyMemInfoCopy(to_address, from_address, len, "Memcpy/"); + } else { + NotifyMemInfo(MemBlockFlags::READ, from_address, len, tag, tagLen); + NotifyMemInfo(MemBlockFlags::WRITE, to_address, len, tag, tagLen); } - NotifyMemInfo(MemBlockFlags::READ, from_address, len, tag, tagLen); - NotifyMemInfo(MemBlockFlags::WRITE, to_address, len, tag, tagLen); } } diff --git a/GPU/GPUCommon.cpp b/GPU/GPUCommon.cpp index cd8824f2e333..9dfd2cdbbf8f 100644 --- a/GPU/GPUCommon.cpp +++ b/GPU/GPUCommon.cpp @@ -1704,9 +1704,7 @@ void GPUCommon::DoBlockTransfer(u32 skipDrawReason) { memcpy(dstp, srcp, bytesToCopy); if (MemBlockInfoDetailed(bytesToCopy)) { - tagSize = FormatMemWriteTagAt(tag, sizeof(tag), "GPUBlockTransfer/", src, bytesToCopy); - NotifyMemInfo(MemBlockFlags::READ, src, bytesToCopy, tag, tagSize); - NotifyMemInfo(MemBlockFlags::WRITE, dst, bytesToCopy, tag, tagSize); + NotifyMemInfoCopy(dst, src, bytesToCopy, "GPUBlockTransfer/"); } } else if ((srcDstOverlap || srcWraps || dstWraps) && (srcValid || srcWraps) && (dstValid || dstWraps)) { // This path means we have either src/dst overlap, OR one or both of src and dst wrap. @@ -1862,12 +1860,11 @@ bool GPUCommon::PerformMemoryCopy(u32 dest, u32 src, int size, GPUCopyFlag flags // We use matching values in PerformReadbackToMemory/PerformWriteColorFromMemory. // Since they're identical we don't need to copy. if (dest != src) { + if (Memory::IsValidRange(dest, size) && Memory::IsValidRange(src, size)) { + memcpy(Memory::GetPointerWriteUnchecked(dest), Memory::GetPointerUnchecked(src), size); + } if (MemBlockInfoDetailed(size)) { - char tag[128]; - size_t tagSize = FormatMemWriteTagAt(tag, sizeof(tag), "GPUMemcpy/", src, size); - Memory::Memcpy(dest, src, size, tag, tagSize); - } else { - Memory::Memcpy(dest, src, size, "GPUMemcpy"); + NotifyMemInfoCopy(dest, src, size, "GPUMemcpy/"); } } } @@ -1876,10 +1873,7 @@ bool GPUCommon::PerformMemoryCopy(u32 dest, u32 src, int size, GPUCopyFlag flags } if (MemBlockInfoDetailed(size)) { - char tag[128]; - size_t tagSize = FormatMemWriteTagAt(tag, sizeof(tag), "GPUMemcpy/", src, size); - NotifyMemInfo(MemBlockFlags::READ, src, size, tag, tagSize); - NotifyMemInfo(MemBlockFlags::WRITE, dest, size, tag, tagSize); + NotifyMemInfoCopy(dest, src, size, "GPUMemcpy/"); } InvalidateCache(dest, size, GPU_INVALIDATE_HINT); if (!(flags & GPUCopyFlag::DEBUG_NOTIFIED)) From a416478780642eb2162d84e0f36cc0d4dc81b696 Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 24 Sep 2023 18:19:50 -0700 Subject: [PATCH 2/4] Debugger: Skip tag copy on READ notify. --- Core/Debugger/MemBlockInfo.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Core/Debugger/MemBlockInfo.cpp b/Core/Debugger/MemBlockInfo.cpp index 92242a81264e..38fa3c6c81b9 100644 --- a/Core/Debugger/MemBlockInfo.cpp +++ b/Core/Debugger/MemBlockInfo.cpp @@ -440,7 +440,7 @@ void NotifyMemInfoPC(MemBlockFlags flags, uint32_t start, uint32_t size, uint32_ bool needFlush = false; // When the setting is off, we skip smaller info to keep things fast. - if (MemBlockInfoDetailed(size)) { + if (MemBlockInfoDetailed(size) && flags != MemBlockFlags::READ) { PendingNotifyMem info{ flags, start, size }; info.ticks = CoreTiming::GetTicks(); info.pc = pc; From b0da32f41fdc3a50d19d32d55ebf588d0a42967a Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 24 Sep 2023 18:29:43 -0700 Subject: [PATCH 3/4] Debugger: Defer copy src tag lookups. --- Core/Debugger/MemBlockInfo.cpp | 78 +++++++++++++++++++++++++++++----- 1 file changed, 68 insertions(+), 10 deletions(-) diff --git a/Core/Debugger/MemBlockInfo.cpp b/Core/Debugger/MemBlockInfo.cpp index 38fa3c6c81b9..f66db106741f 100644 --- a/Core/Debugger/MemBlockInfo.cpp +++ b/Core/Debugger/MemBlockInfo.cpp @@ -78,6 +78,7 @@ struct PendingNotifyMem { MemBlockFlags flags; uint32_t start; uint32_t size; + uint32_t copySrc; uint64_t ticks; uint32_t pc; char tag[128]; @@ -369,9 +370,18 @@ void MemSlabMap::FillHeads(Slab *slab) { } } +size_t FormatMemWriteTagAtNoFlush(char *buf, size_t sz, const char *prefix, uint32_t start, uint32_t size); + void FlushPendingMemInfo() { std::lock_guard guard(pendingMutex); for (const auto &info : pendingNotifies) { + if (info.copySrc != 0) { + char tagData[128]; + size_t tagSize = FormatMemWriteTagAtNoFlush(tagData, sizeof(tagData), info.tag, info.copySrc, info.size); + writeMap.Mark(info.start, info.size, info.ticks, info.pc, true, tagData); + continue; + } + if (info.flags & MemBlockFlags::ALLOC) { allocMap.Mark(info.start, info.size, info.ticks, info.pc, true, info.tag); } else if (info.flags & MemBlockFlags::FREE) { @@ -411,6 +421,9 @@ static inline bool MergeRecentMemInfo(const PendingNotifyMem &info, size_t copyL for (size_t i = 1; i <= 4; ++i) { auto &prev = pendingNotifies[pendingNotifies.size() - i]; + if (prev.copySrc != 0) + return false; + if (prev.flags != info.flags) continue; @@ -485,11 +498,41 @@ void NotifyMemInfo(MemBlockFlags flags, uint32_t start, uint32_t size, const cha } void NotifyMemInfoCopy(uint32_t destPtr, uint32_t srcPtr, uint32_t size, const char *prefix) { - // TODO - char tagData[128]; - size_t tagSize = FormatMemWriteTagAt(tagData, sizeof(tagData), prefix, srcPtr, size); - NotifyMemInfo(MemBlockFlags::READ, srcPtr, size, tagData, tagSize); - NotifyMemInfo(MemBlockFlags::WRITE, destPtr, size, tagData, tagSize); + if (size == 0) + return; + + if (CBreakPoints::HasMemChecks()) { + // This will cause a flush, but it's needed to trigger memchecks with proper data. + char tagData[128]; + size_t tagSize = FormatMemWriteTagAt(tagData, sizeof(tagData), prefix, srcPtr, size); + NotifyMemInfo(MemBlockFlags::READ, srcPtr, size, tagData, tagSize); + NotifyMemInfo(MemBlockFlags::WRITE, destPtr, size, tagData, tagSize); + } else if (MemBlockInfoDetailed(size)) { + srcPtr = NormalizeAddress(srcPtr); + destPtr = NormalizeAddress(destPtr); + + PendingNotifyMem info{ MemBlockFlags::WRITE, destPtr, size }; + info.copySrc = srcPtr; + info.ticks = CoreTiming::GetTicks(); + info.pc = currentMIPS->pc; + + // Store the prefix for now. The correct tag will be calculated on flush. + truncate_cpy(info.tag, prefix); + + std::lock_guard guard(pendingMutex); + if (destPtr < 0x08000000) { + pendingNotifyMinAddr1 = std::min(pendingNotifyMinAddr1.load(), destPtr); + pendingNotifyMaxAddr1 = std::max(pendingNotifyMaxAddr1.load(), destPtr + size); + } else { + pendingNotifyMinAddr2 = std::min(pendingNotifyMinAddr2.load(), destPtr); + pendingNotifyMaxAddr2 = std::max(pendingNotifyMaxAddr2.load(), destPtr + size); + } + pendingNotifies.push_back(info); + } + + if (pendingNotifies.size() > MAX_PENDING_NOTIFIES) { + FlushPendingMemInfo(); + } } std::vector FindMemInfo(uint32_t start, uint32_t size) { @@ -528,13 +571,15 @@ std::vector FindMemInfoByFlag(MemBlockFlags flags, uint32_t start, return results; } -static const char *FindWriteTagByFlag(MemBlockFlags flags, uint32_t start, uint32_t size) { +static const char *FindWriteTagByFlag(MemBlockFlags flags, uint32_t start, uint32_t size, bool flush = true) { start = NormalizeAddress(start); - if (pendingNotifyMinAddr1 < start + size && pendingNotifyMaxAddr1 >= start) - FlushPendingMemInfo(); - if (pendingNotifyMinAddr2 < start + size && pendingNotifyMaxAddr2 >= start) - FlushPendingMemInfo(); + if (flush) { + if (pendingNotifyMinAddr1 < start + size && pendingNotifyMaxAddr1 >= start) + FlushPendingMemInfo(); + if (pendingNotifyMinAddr2 < start + size && pendingNotifyMaxAddr2 >= start) + FlushPendingMemInfo(); + } if (flags & MemBlockFlags::ALLOC) { const char *tag = allocMap.FastFindWriteTag(MemBlockFlags::ALLOC, start, size); @@ -572,6 +617,19 @@ size_t FormatMemWriteTagAt(char *buf, size_t sz, const char *prefix, uint32_t st return snprintf(buf, sz, "%s%08x_size_%08x", prefix, start, size); } +size_t FormatMemWriteTagAtNoFlush(char *buf, size_t sz, const char *prefix, uint32_t start, uint32_t size) { + const char *tag = FindWriteTagByFlag(MemBlockFlags::WRITE, start, size, false); + if (tag && strcmp(tag, "MemInit") != 0) { + return snprintf(buf, sz, "%s%s", prefix, tag); + } + // Fall back to alloc and texture, especially for VRAM. We prefer write above. + tag = FindWriteTagByFlag(MemBlockFlags::ALLOC | MemBlockFlags::TEXTURE, start, size, false); + if (tag) { + return snprintf(buf, sz, "%s%s", prefix, tag); + } + return snprintf(buf, sz, "%s%08x_size_%08x", prefix, start, size); +} + void MemBlockInfoInit() { std::lock_guard guard(pendingMutex); pendingNotifies.reserve(MAX_PENDING_NOTIFIES); From fc133f49944dbc1f74cddd734fb933e351f6bb5c Mon Sep 17 00:00:00 2001 From: "Unknown W. Brackets" Date: Sun, 24 Sep 2023 19:03:57 -0700 Subject: [PATCH 4/4] Debugger: Use a thread to flush meminfo. --- Core/Debugger/MemBlockInfo.cpp | 103 ++++++++++++++++++++++++++------- 1 file changed, 81 insertions(+), 22 deletions(-) diff --git a/Core/Debugger/MemBlockInfo.cpp b/Core/Debugger/MemBlockInfo.cpp index f66db106741f..9daa39d4faee 100644 --- a/Core/Debugger/MemBlockInfo.cpp +++ b/Core/Debugger/MemBlockInfo.cpp @@ -17,8 +17,10 @@ #include #include +#include #include #include +#include #include "Common/Log.h" #include "Common/Serialize/Serializer.h" @@ -84,7 +86,9 @@ struct PendingNotifyMem { char tag[128]; }; -static constexpr size_t MAX_PENDING_NOTIFIES = 512; +// 160 KB. +static constexpr size_t MAX_PENDING_NOTIFIES = 1024; +static constexpr size_t MAX_PENDING_NOTIFIES_THREAD = 1000; static MemSlabMap allocMap; static MemSlabMap suballocMap; static MemSlabMap writeMap; @@ -94,9 +98,17 @@ static std::atomic pendingNotifyMinAddr1; static std::atomic pendingNotifyMaxAddr1; static std::atomic pendingNotifyMinAddr2; static std::atomic pendingNotifyMaxAddr2; -static std::mutex pendingMutex; +// To prevent deadlocks, acquire Read before Write if you're going to acquire both. +static std::mutex pendingWriteMutex; +static std::mutex pendingReadMutex; static int detailedOverride; +static std::thread flushThread; +static std::atomic flushThreadRunning; +static std::atomic flushThreadPending; +static std::mutex flushLock; +static std::condition_variable flushCond; + MemSlabMap::MemSlabMap() { Reset(); } @@ -373,8 +385,22 @@ void MemSlabMap::FillHeads(Slab *slab) { size_t FormatMemWriteTagAtNoFlush(char *buf, size_t sz, const char *prefix, uint32_t start, uint32_t size); void FlushPendingMemInfo() { - std::lock_guard guard(pendingMutex); - for (const auto &info : pendingNotifies) { + // This lock prevents us from another thread reading while we're busy flushing. + std::lock_guard guard(pendingReadMutex); + std::vector thisBatch; + { + std::lock_guard guard(pendingWriteMutex); + thisBatch = std::move(pendingNotifies); + pendingNotifies.clear(); + pendingNotifies.reserve(MAX_PENDING_NOTIFIES); + + pendingNotifyMinAddr1 = 0xFFFFFFFF; + pendingNotifyMaxAddr1 = 0; + pendingNotifyMinAddr2 = 0xFFFFFFFF; + pendingNotifyMaxAddr2 = 0; + } + + for (const auto &info : thisBatch) { if (info.copySrc != 0) { char tagData[128]; size_t tagSize = FormatMemWriteTagAtNoFlush(tagData, sizeof(tagData), info.tag, info.copySrc, info.size); @@ -402,11 +428,6 @@ void FlushPendingMemInfo() { writeMap.Mark(info.start, info.size, info.ticks, info.pc, true, info.tag); } } - pendingNotifies.clear(); - pendingNotifyMinAddr1 = 0xFFFFFFFF; - pendingNotifyMaxAddr1 = 0; - pendingNotifyMinAddr2 = 0xFFFFFFFF; - pendingNotifyMaxAddr2 = 0; } static inline uint32_t NormalizeAddress(uint32_t addr) { @@ -465,7 +486,7 @@ void NotifyMemInfoPC(MemBlockFlags flags, uint32_t start, uint32_t size, uint32_ memcpy(info.tag, tagStr, copyLength); info.tag[copyLength] = 0; - std::lock_guard guard(pendingMutex); + std::lock_guard guard(pendingWriteMutex); // Sometimes we get duplicates, quickly check. if (!MergeRecentMemInfo(info, copyLength)) { if (start < 0x08000000) { @@ -477,11 +498,15 @@ void NotifyMemInfoPC(MemBlockFlags flags, uint32_t start, uint32_t size, uint32_ } pendingNotifies.push_back(info); } - needFlush = pendingNotifies.size() > MAX_PENDING_NOTIFIES; + needFlush = pendingNotifies.size() > MAX_PENDING_NOTIFIES_THREAD; } if (needFlush) { - FlushPendingMemInfo(); + { + std::lock_guard guard(flushLock); + flushThreadPending = true; + } + flushCond.notify_one(); } if (!(flags & MemBlockFlags::SKIP_MEMCHECK)) { @@ -501,6 +526,7 @@ void NotifyMemInfoCopy(uint32_t destPtr, uint32_t srcPtr, uint32_t size, const c if (size == 0) return; + bool needsFlush = false; if (CBreakPoints::HasMemChecks()) { // This will cause a flush, but it's needed to trigger memchecks with proper data. char tagData[128]; @@ -519,7 +545,7 @@ void NotifyMemInfoCopy(uint32_t destPtr, uint32_t srcPtr, uint32_t size, const c // Store the prefix for now. The correct tag will be calculated on flush. truncate_cpy(info.tag, prefix); - std::lock_guard guard(pendingMutex); + std::lock_guard guard(pendingWriteMutex); if (destPtr < 0x08000000) { pendingNotifyMinAddr1 = std::min(pendingNotifyMinAddr1.load(), destPtr); pendingNotifyMaxAddr1 = std::max(pendingNotifyMaxAddr1.load(), destPtr + size); @@ -528,10 +554,15 @@ void NotifyMemInfoCopy(uint32_t destPtr, uint32_t srcPtr, uint32_t size, const c pendingNotifyMaxAddr2 = std::max(pendingNotifyMaxAddr2.load(), destPtr + size); } pendingNotifies.push_back(info); + needsFlush = pendingNotifies.size() > MAX_PENDING_NOTIFIES_THREAD; } - if (pendingNotifies.size() > MAX_PENDING_NOTIFIES) { - FlushPendingMemInfo(); + if (needsFlush) { + { + std::lock_guard guard(flushLock); + flushThreadPending = true; + } + flushCond.notify_one(); } } @@ -630,22 +661,50 @@ size_t FormatMemWriteTagAtNoFlush(char *buf, size_t sz, const char *prefix, uint return snprintf(buf, sz, "%s%08x_size_%08x", prefix, start, size); } +static void FlushMemInfoThread() { + while (flushThreadRunning.load()) { + flushThreadPending = false; + FlushPendingMemInfo(); + + std::unique_lock guard(flushLock); + flushCond.wait(guard, [] { + return flushThreadPending.load(); + }); + } +} + void MemBlockInfoInit() { - std::lock_guard guard(pendingMutex); + std::lock_guard guard(pendingReadMutex); + std::lock_guard guardW(pendingWriteMutex); pendingNotifies.reserve(MAX_PENDING_NOTIFIES); pendingNotifyMinAddr1 = 0xFFFFFFFF; pendingNotifyMaxAddr1 = 0; pendingNotifyMinAddr2 = 0xFFFFFFFF; pendingNotifyMaxAddr2 = 0; + + flushThreadRunning = true; + flushThreadPending = false; + flushThread = std::thread(&FlushMemInfoThread); } void MemBlockInfoShutdown() { - std::lock_guard guard(pendingMutex); - allocMap.Reset(); - suballocMap.Reset(); - writeMap.Reset(); - textureMap.Reset(); - pendingNotifies.clear(); + { + std::lock_guard guard(pendingReadMutex); + std::lock_guard guardW(pendingWriteMutex); + allocMap.Reset(); + suballocMap.Reset(); + writeMap.Reset(); + textureMap.Reset(); + pendingNotifies.clear(); + } + + if (flushThreadRunning.load()) { + std::lock_guard guard(flushLock); + flushThreadRunning = false; + flushThreadPending = true; + } + flushCond.notify_one(); + flushThread.join(); } void MemBlockInfoDoState(PointerWrap &p) {