Merge pull request #15377 from unknownbrackets/memblock-detail

Debugger: Avoid mem write tag lookup on small alloc
hrydgard · Feb 6, 2022 · 9eeea5a · 9eeea5a
2 parents 51c21ce + 80e054b
commit 9eeea5a
Show file tree

Hide file tree

Showing 10 changed files with 125 additions and 67 deletions.
diff --git a/Core/Debugger/Breakpoints.cpp b/Core/Debugger/Breakpoints.cpp
@@ -22,6 +22,7 @@
 #include "Common/Log.h"
 #include "Core/Core.h"
 #include "Core/Debugger/Breakpoints.h"
+#include "Core/Debugger/MemBlockInfo.h"
 #include "Core/Debugger/SymbolMap.h"
 #include "Core/Host.h"
 #include "Core/MemMap.h"
@@ -389,15 +390,19 @@ void CBreakPoints::AddMemCheck(u32 start, u32 end, MemCheckCondition cond, Break
 		check.result = result;
 
 		memChecks_.push_back(check);
-		anyMemChecks_ = true;
+		bool hadAny = anyMemChecks_.exchange(true);
+		if (!hadAny)
+			MemBlockOverrideDetailed();
 		guard.unlock();
 		Update();
 	}
 	else
 	{
 		memChecks_[mc].cond = (MemCheckCondition)(memChecks_[mc].cond | cond);
 		memChecks_[mc].result = (BreakAction)(memChecks_[mc].result | result);
-		anyMemChecks_ = true;
+		bool hadAny = anyMemChecks_.exchange(true);
+		if (!hadAny)
+			MemBlockOverrideDetailed();
 		guard.unlock();
 		Update();
 	}
@@ -413,7 +418,9 @@ void CBreakPoints::RemoveMemCheck(u32 start, u32 end)
 	if (mc != INVALID_MEMCHECK)
 	{
 		memChecks_.erase(memChecks_.begin() + mc);
-		anyMemChecks_ = !memChecks_.empty();
+		bool hadAny = anyMemChecks_.exchange(!memChecks_.empty());
+		if (hadAny)
+			MemBlockReleaseDetailed();
 		guard.unlock();
 		Update();
 	}
@@ -441,6 +448,9 @@ void CBreakPoints::ClearAllMemChecks()
 	if (!memChecks_.empty())
 	{
 		memChecks_.clear();
+		bool hadAny = anyMemChecks_.exchange(false);
+		if (hadAny)
+			MemBlockReleaseDetailed();
 		guard.unlock();
 		Update();
 	}

diff --git a/Core/Debugger/MemBlockInfo.cpp b/Core/Debugger/MemBlockInfo.cpp
@@ -390,7 +390,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 (size >= 0x100 || MemBlockInfoDetailed()) {
+	if (MemBlockInfoDetailed(size)) {
 		PendingNotifyMem info{ flags, start, size };
 		info.ticks = CoreTiming::GetTicks();
 		info.pc = pc;

diff --git a/Core/Debugger/MemBlockInfo.h b/Core/Debugger/MemBlockInfo.h
@@ -25,6 +25,8 @@
 
 class PointerWrap;
 
+static constexpr uint32_t MEMINFO_MIN_SIZE = 0x100;
+
 enum class MemBlockFlags {
 	ALLOC = 0x0001,
 	SUB_ALLOC = 0x0002,
@@ -75,3 +77,11 @@ void MemBlockInfoDoState(PointerWrap &p);
 void MemBlockOverrideDetailed();
 void MemBlockReleaseDetailed();
 bool MemBlockInfoDetailed();
+
+static inline bool MemBlockInfoDetailed(uint32_t size) {
+	return size >= MEMINFO_MIN_SIZE || MemBlockInfoDetailed();
+}
+
+static inline bool MemBlockInfoDetailed(uint32_t size1, uint32_t size2) {
+	return size1 >= MEMINFO_MIN_SIZE || size2 >= MEMINFO_MIN_SIZE || MemBlockInfoDetailed();
+}
diff --git a/Core/HLE/ReplaceTables.cpp b/Core/HLE/ReplaceTables.cpp
@@ -156,14 +156,16 @@ static int Replace_memcpy() {
 	}
 	RETURN(destPtr);
 
-	const std::string tag = "ReplaceMemcpy/" + GetMemWriteTagAt(srcPtr, bytes);
-	NotifyMemInfo(MemBlockFlags::READ, srcPtr, bytes, tag.c_str(), tag.size());
-	NotifyMemInfo(MemBlockFlags::WRITE, destPtr, bytes, tag.c_str(), tag.size());
-
-	// It's pretty common that games will copy video data.
-	if (tag == "ReplaceMemcpy/VideoDecode" || tag == "ReplaceMemcpy/VideoDecodeRange") {
-		if (bytes == 512 * 272 * 4) {
-			gpu->NotifyVideoUpload(destPtr, bytes, 512, GE_FORMAT_8888);
+	if (MemBlockInfoDetailed(bytes)) {
+		const std::string tag = "ReplaceMemcpy/" + GetMemWriteTagAt(srcPtr, bytes);
+		NotifyMemInfo(MemBlockFlags::READ, srcPtr, bytes, tag.c_str(), tag.size());
+		NotifyMemInfo(MemBlockFlags::WRITE, destPtr, bytes, tag.c_str(), tag.size());
+
+		// It's pretty common that games will copy video data.
+		if (tag == "ReplaceMemcpy/VideoDecode" || tag == "ReplaceMemcpy/VideoDecodeRange") {
+			if (bytes == 512 * 272 * 4) {
+				gpu->NotifyVideoUpload(destPtr, bytes, 512, GE_FORMAT_8888);
+			}
 		}
 	}
 
@@ -206,14 +208,16 @@ static int Replace_memcpy_jak() {
 	currentMIPS->r[MIPS_REG_A3] = destPtr + bytes;
 	RETURN(destPtr);
 
-	const std::string tag = "ReplaceMemcpy/" + GetMemWriteTagAt(srcPtr, bytes);
-	NotifyMemInfo(MemBlockFlags::READ, srcPtr, bytes, tag.c_str(), tag.size());
-	NotifyMemInfo(MemBlockFlags::WRITE, destPtr, bytes, tag.c_str(), tag.size());
+	if (MemBlockInfoDetailed(bytes)) {
+		const std::string tag = "ReplaceMemcpy/" + GetMemWriteTagAt(srcPtr, bytes);
+		NotifyMemInfo(MemBlockFlags::READ, srcPtr, bytes, tag.c_str(), tag.size());
+		NotifyMemInfo(MemBlockFlags::WRITE, destPtr, bytes, tag.c_str(), tag.size());
 
-	// It's pretty common that games will copy video data.
-	if (tag == "ReplaceMemcpy/VideoDecode" || tag == "ReplaceMemcpy/VideoDecodeRange") {
-		if (bytes == 512 * 272 * 4) {
-			gpu->NotifyVideoUpload(destPtr, bytes, 512, GE_FORMAT_8888);
+		// It's pretty common that games will copy video data.
+		if (tag == "ReplaceMemcpy/VideoDecode" || tag == "ReplaceMemcpy/VideoDecodeRange") {
+			if (bytes == 512 * 272 * 4) {
+				gpu->NotifyVideoUpload(destPtr, bytes, 512, GE_FORMAT_8888);
+			}
 		}
 	}
 
@@ -242,9 +246,11 @@ static int Replace_memcpy16() {
 	}
 	RETURN(destPtr);
 
-	const std::string tag = "ReplaceMemcpy16/" + GetMemWriteTagAt(srcPtr, bytes);
-	NotifyMemInfo(MemBlockFlags::READ, srcPtr, bytes, tag.c_str(), tag.size());
-	NotifyMemInfo(MemBlockFlags::WRITE, destPtr, bytes, tag.c_str(), tag.size());
+	if (MemBlockInfoDetailed(bytes)) {
+		const std::string tag = "ReplaceMemcpy16/" + GetMemWriteTagAt(srcPtr, bytes);
+		NotifyMemInfo(MemBlockFlags::READ, srcPtr, bytes, tag.c_str(), tag.size());
+		NotifyMemInfo(MemBlockFlags::WRITE, destPtr, bytes, tag.c_str(), tag.size());
+	}
 
 	return 10 + bytes / 4;  // approximation
 }
@@ -281,9 +287,11 @@ static int Replace_memcpy_swizzled() {
 
 	RETURN(0);
 
-	const std::string tag = "ReplaceMemcpySwizzle/" + GetMemWriteTagAt(srcPtr, pitch * h);
-	NotifyMemInfo(MemBlockFlags::READ, srcPtr, pitch * h, tag.c_str(), tag.size());
-	NotifyMemInfo(MemBlockFlags::WRITE, destPtr, pitch * h, tag.c_str(), tag.size());
+	if (MemBlockInfoDetailed(pitch * h)) {
+		const std::string tag = "ReplaceMemcpySwizzle/" + GetMemWriteTagAt(srcPtr, pitch * h);
+		NotifyMemInfo(MemBlockFlags::READ, srcPtr, pitch * h, tag.c_str(), tag.size());
+		NotifyMemInfo(MemBlockFlags::WRITE, destPtr, pitch * h, tag.c_str(), tag.size());
+	}
 
 	return 10 + (pitch * h) / 4;  // approximation
 }
@@ -310,9 +318,11 @@ static int Replace_memmove() {
 	}
 	RETURN(destPtr);
 
-	const std::string tag = "ReplaceMemmove/" + GetMemWriteTagAt(srcPtr, bytes);
-	NotifyMemInfo(MemBlockFlags::READ, srcPtr, bytes, tag.c_str(), tag.size());
-	NotifyMemInfo(MemBlockFlags::WRITE, destPtr, bytes, tag.c_str(), tag.size());
+	if (MemBlockInfoDetailed(bytes)) {
+		const std::string tag = "ReplaceMemmove/" + GetMemWriteTagAt(srcPtr, bytes);
+		NotifyMemInfo(MemBlockFlags::READ, srcPtr, bytes, tag.c_str(), tag.size());
+		NotifyMemInfo(MemBlockFlags::WRITE, destPtr, bytes, tag.c_str(), tag.size());
+	}
 
 	return 10 + bytes / 4;  // approximation
 }

diff --git a/Core/HLE/sceDeflt.cpp b/Core/HLE/sceDeflt.cpp
@@ -58,9 +58,11 @@ static int CommonDecompress(int windowBits, u32 OutBuffer, int OutBufferLength,
 		*crc32Addr = crc32(crc, outBufferPtr, stream.total_out);
 	}
 
-	const std::string tag = "sceDeflt/" + GetMemWriteTagAt(InBuffer, stream.total_in);
-	NotifyMemInfo(MemBlockFlags::READ, InBuffer, stream.total_in, tag.c_str(), tag.size());
-	NotifyMemInfo(MemBlockFlags::WRITE, OutBuffer, stream.total_out, tag.c_str(), tag.size());
+	if (MemBlockInfoDetailed(stream.total_in, stream.total_out)) {
+		const std::string tag = "sceDeflt/" + GetMemWriteTagAt(InBuffer, stream.total_in);
+		NotifyMemInfo(MemBlockFlags::READ, InBuffer, stream.total_in, tag.c_str(), tag.size());
+		NotifyMemInfo(MemBlockFlags::WRITE, OutBuffer, stream.total_out, tag.c_str(), tag.size());
+	}
 
 	return hleLogSuccessI(HLE, stream.total_out);
 }

diff --git a/Core/HLE/sceDmac.cpp b/Core/HLE/sceDmac.cpp
@@ -51,8 +51,12 @@ static int __DmacMemcpy(u32 dst, u32 src, u32 size) {
 	}
 	if (!skip) {
 		currentMIPS->InvalidateICache(src, size);
-		const std::string tag = "DmacMemcpy/" + GetMemWriteTagAt(src, size);
-		Memory::Memcpy(dst, src, size, tag.c_str(), tag.size());
+		if (MemBlockInfoDetailed(size)) {
+			const std::string tag = "DmacMemcpy/" + GetMemWriteTagAt(src, size);
+			Memory::Memcpy(dst, src, size, tag.c_str(), tag.size());
+		} else {
+			Memory::Memcpy(dst, src, size, "DmacMemcpy");
+		}
 		currentMIPS->InvalidateICache(dst, size);
 	}
 

diff --git a/Core/HLE/sceKernelInterrupt.cpp b/Core/HLE/sceKernelInterrupt.cpp
@@ -655,9 +655,11 @@ static u32 sceKernelMemcpy(u32 dst, u32 src, u32 size)
 		}
 	}
 
-	const std::string tag = "KernelMemcpy/" + GetMemWriteTagAt(src, size);
-	NotifyMemInfo(MemBlockFlags::READ, src, size, tag.c_str(), tag.size());
-	NotifyMemInfo(MemBlockFlags::WRITE, dst, size, tag.c_str(), tag.size());
+	if (MemBlockInfoDetailed(size)) {
+		const std::string tag = "KernelMemcpy/" + GetMemWriteTagAt(src, size);
+		NotifyMemInfo(MemBlockFlags::READ, src, size, tag.c_str(), tag.size());
+		NotifyMemInfo(MemBlockFlags::WRITE, dst, size, tag.c_str(), tag.size());
+	}
 
 	return dst;
 }
@@ -688,9 +690,11 @@ static u32 sysclib_memcpy(u32 dst, u32 src, u32 size) {
 	if (Memory::IsValidRange(dst, size) && Memory::IsValidRange(src, size)) {
 		memcpy(Memory::GetPointer(dst), Memory::GetPointer(src), size);
 	}
-	const std::string tag = "KernelMemcpy/" + GetMemWriteTagAt(src, size);
-	NotifyMemInfo(MemBlockFlags::READ, src, size, tag.c_str(), tag.size());
-	NotifyMemInfo(MemBlockFlags::WRITE, dst, size, tag.c_str(), tag.size());
+	if (MemBlockInfoDetailed(size)) {
+		const std::string tag = "KernelMemcpy/" + GetMemWriteTagAt(src, size);
+		NotifyMemInfo(MemBlockFlags::READ, src, size, tag.c_str(), tag.size());
+		NotifyMemInfo(MemBlockFlags::WRITE, dst, size, tag.c_str(), tag.size());
+	}
 	return dst;
 }
 
@@ -789,9 +793,11 @@ static u32 sysclib_memmove(u32 dst, u32 src, u32 size) {
 	if (Memory::IsValidRange(dst, size) && Memory::IsValidRange(src, size)) {
 		memmove(Memory::GetPointer(dst), Memory::GetPointer(src), size);
 	}
-	const std::string tag = "KernelMemmove/" + GetMemWriteTagAt(src, size);
-	NotifyMemInfo(MemBlockFlags::READ, src, size, tag.c_str(), tag.size());
-	NotifyMemInfo(MemBlockFlags::WRITE, dst, size, tag.c_str(), tag.size());
+	if (MemBlockInfoDetailed(size)) {
+		const std::string tag = "KernelMemmove/" + GetMemWriteTagAt(src, size);
+		NotifyMemInfo(MemBlockFlags::READ, src, size, tag.c_str(), tag.size());
+		NotifyMemInfo(MemBlockFlags::WRITE, dst, size, tag.c_str(), tag.size());
+	}
 	return 0;
 }
 

diff --git a/Core/MemMapHelpers.h b/Core/MemMapHelpers.h
@@ -58,21 +58,25 @@ inline void Memcpy(void *to_data, const u32 from_address, const u32 len, const c
 
 inline void Memcpy(const u32 to_address, const u32 from_address, const u32 len, const char *tag, size_t tagLen) {
 	u8 *to = GetPointer(to_address);
-	if (to) {
-		const u8 *from = GetPointer(from_address);
-		if (from) {
-			memcpy(to, from, len);
-			char tagData[128];
-			if (!tag) {
-				const std::string srcTag = GetMemWriteTagAt(from_address, len);
-				tag = tagData;
-				tagLen = snprintf(tagData, sizeof(tagData), "Memcpy/%s", srcTag.c_str());
-			}
-			NotifyMemInfo(MemBlockFlags::READ, from_address, len, tag, tagLen);
-			NotifyMemInfo(MemBlockFlags::WRITE, to_address, len, tag, tagLen);
+	// If not, GetPointer will log.
+	if (!to)
+		return;
+	const u8 *from = GetPointer(from_address);
+	if (!from)
+		return;
+
+	memcpy(to, from, len);
+
+	if (MemBlockInfoDetailed(len)) {
+		char tagData[128];
+		if (!tag) {
+			const std::string srcTag = GetMemWriteTagAt(from_address, len);
+			tag = tagData;
+			tagLen = snprintf(tagData, sizeof(tagData), "Memcpy/%s", srcTag.c_str());
 		}
+		NotifyMemInfo(MemBlockFlags::READ, from_address, len, tag, tagLen);
+		NotifyMemInfo(MemBlockFlags::WRITE, to_address, len, tag, tagLen);
 	}
-	// if not, GetPointer will log.
 }
 
 template<size_t tagLen>

diff --git a/GPU/GPUCommon.cpp b/GPU/GPUCommon.cpp
@@ -2767,11 +2767,15 @@ void GPUCommon::DoBlockTransfer(u32 skipDrawReason) {
 		framebufferManager_->NotifyBlockTransferAfter(dstBasePtr, dstStride, dstX, dstY, srcBasePtr, srcStride, srcX, srcY, width, height, bpp, skipDrawReason);
 	}
 
-	const uint32_t src = srcBasePtr + (srcY * srcStride + srcX) * bpp;
 	const uint32_t srcSize = height * srcStride * bpp;
-	const std::string tag = "GPUBlockTransfer/" + GetMemWriteTagAt(src, srcSize);
-	NotifyMemInfo(MemBlockFlags::READ, src, srcSize, tag.c_str(), tag.size());
-	NotifyMemInfo(MemBlockFlags::WRITE, dstBasePtr + (dstY * dstStride + dstX) * bpp, height * dstStride * bpp, tag.c_str(), tag.size());
+	const uint32_t dstSize = height * dstStride * bpp;
+	if (MemBlockInfoDetailed(srcSize, dstSize)) {
+		const uint32_t src = srcBasePtr + (srcY * srcStride + srcX) * bpp;
+		const uint32_t dst = dstBasePtr + (dstY * dstStride + dstX) * bpp;
+		const std::string tag = "GPUBlockTransfer/" + GetMemWriteTagAt(src, srcSize);
+		NotifyMemInfo(MemBlockFlags::READ, src, srcSize, tag.c_str(), tag.size());
+		NotifyMemInfo(MemBlockFlags::WRITE, dst, dstSize, tag.c_str(), tag.size());
+	}
 
 	// TODO: Correct timing appears to be 1.9, but erring a bit low since some of our other timing is inaccurate.
 	cyclesExecuted += ((height * width * bpp) * 16) / 10;
@@ -2784,17 +2788,23 @@ bool GPUCommon::PerformMemoryCopy(u32 dest, u32 src, int size) {
 			// We use a little hack for PerformMemoryDownload/PerformMemoryUpload using a VRAM mirror.
 			// Since they're identical we don't need to copy.
 			if (!Memory::IsVRAMAddress(dest) || (dest ^ 0x00400000) != src) {
-				const std::string tag = "GPUMemcpy/" + GetMemWriteTagAt(src, size);
-				Memory::Memcpy(dest, src, size, tag.c_str(), tag.size());
+				if (MemBlockInfoDetailed(size)) {
+					const std::string tag = "GPUMemcpy/" + GetMemWriteTagAt(src, size);
+					Memory::Memcpy(dest, src, size, tag.c_str(), tag.size());
+				} else {
+					Memory::Memcpy(dest, src, size, "GPUMemcpy");
+				}
 			}
 		}
 		InvalidateCache(dest, size, GPU_INVALIDATE_HINT);
 		return true;
 	}
 
-	const std::string tag = "GPUMemcpy/" + GetMemWriteTagAt(src, size);
-	NotifyMemInfo(MemBlockFlags::READ, src, size, tag.c_str(), tag.size());
-	NotifyMemInfo(MemBlockFlags::WRITE, dest, size, tag.c_str(), tag.size());
+	if (MemBlockInfoDetailed(size)) {
+		const std::string tag = "GPUMemcpy/" + GetMemWriteTagAt(src, size);
+		NotifyMemInfo(MemBlockFlags::READ, src, size, tag.c_str(), tag.size());
+		NotifyMemInfo(MemBlockFlags::WRITE, dest, size, tag.c_str(), tag.size());
+	}
 	InvalidateCache(dest, size, GPU_INVALIDATE_HINT);
 	GPURecord::NotifyMemcpy(dest, src, size);
 	return false;

diff --git a/GPU/Software/SoftGpu.cpp b/GPU/Software/SoftGpu.cpp
@@ -745,9 +745,11 @@ void SoftGPU::Execute_BlockTransferStart(u32 op, u32 diff) {
 		memcpy(dstp, srcp, width * bpp);
 	}
 
-	const std::string tag = "GPUBlockTransfer/" + GetMemWriteTagAt(src, srcSize);
-	NotifyMemInfo(MemBlockFlags::READ, src, srcSize, tag.c_str(), tag.size());
-	NotifyMemInfo(MemBlockFlags::WRITE, dst, dstSize, tag.c_str(), tag.size());
+	if (MemBlockInfoDetailed(srcSize, dstSize)) {
+		const std::string tag = "GPUBlockTransfer/" + GetMemWriteTagAt(src, srcSize);
+		NotifyMemInfo(MemBlockFlags::READ, src, srcSize, tag.c_str(), tag.size());
+		NotifyMemInfo(MemBlockFlags::WRITE, dst, dstSize, tag.c_str(), tag.size());
+	}
 
 	// TODO: Correct timing appears to be 1.9, but erring a bit low since some of our other timing is inaccurate.
 	cyclesExecuted += ((height * width * bpp) * 16) / 10;