Merge pull request #2665 from unknownbrackets/perf

Improve performance of export resolving / module loading

Common/ChunkFile.h

@@ -201,7 +201,8 @@ public:
 				typename std::map<K, T>::iterator itr = x.begin();
 				while (number > 0)
 				{
-					Do(itr->first);
+					K first = itr->first;
+					Do(first);
 					Do(itr->second);
 					--number;
 					++itr;

Core/Debugger/SymbolMap.cpp

@@ -112,6 +112,7 @@ void SymbolMap::ResetSymbolMap()
 #endif
 	entries.clear();
 	uniqueEntries.clear();
+	entryRanges.clear();
 }
 
 
@@ -127,6 +128,7 @@ void SymbolMap::AddSymbol(const char *symbolname, unsigned int vaddress, size_t
 	{
 		entries.push_back(e);
 		uniqueEntries.insert((const MapEntryUniqueInfo)e);
+		entryRanges[e.vaddress + e.size] = e.vaddress;
 	}
 }
 
@@ -193,6 +195,7 @@ bool SymbolMap::LoadSymbolMap(const char *filename)
 			e.UndecorateName();
 			entries.push_back(e);
 			uniqueEntries.insert((const MapEntryUniqueInfo)e);
+			entryRanges[e.vaddress + e.size] = e.vaddress;
 		}
 	}
 	fclose(f);
@@ -238,6 +241,41 @@ int SymbolMap::GetSymbolNum(unsigned int address, SymbolType symmask) const
 	return -1;
 }
 
+bool SymbolMap::GetSymbolInfo(SymbolInfo *info, u32 address, SymbolType symmask) const
+{
+	// entryRanges is indexed by end.  The first entry after address should contain address.
+	// Otherwise, we have no entry that contains it, unless things overlap (which they shouldn't.)
+	const auto containingEntry = entryRanges.upper_bound(address);
+	if (containingEntry == entryRanges.end())
+		return false;
+
+	// The next most common case is a single symbol by start address.
+	// So we optimize for that by looking in our uniqueEntry set.
+	u32 start_address = containingEntry->second;
+	if (start_address <= address)
+	{
+		const MapEntryUniqueInfo searchKey = {start_address, start_address};
+		const auto entry = uniqueEntries.find(searchKey);
+		if (entry != uniqueEntries.end() && (entry->type & symmask) != 0)
+		{
+			info->address = entry->vaddress;
+			info->size = entry->size;
+			return true;
+		}
+	}
+
+	// Fall back to a slower scan.
+	int n = GetSymbolNum(address, symmask);
+	if (n != -1)
+	{
+		info->address = GetSymbolAddr(n);
+		info->size = GetSymbolSize(n);
+		return true;
+	}
+
+	return false;
+}
+
 const char* SymbolMap::getDirectSymbol(u32 address)
 {
 	for (auto it = entries.begin(), end = entries.end(); it != end; ++it)
@@ -572,6 +610,7 @@ void SymbolMap::UseFuncSignaturesFile(const char *filename, u32 maxAddress)
 					addr+=sig->size-4; //don't need to check function interior
 					entries.push_back(e);
 					uniqueEntries.insert((const MapEntryUniqueInfo)e);
+					entryRanges[e.vaddress + e.size] = e.vaddress;
 					break;
 				}
 				sig++;

Core/Debugger/SymbolMap.h

@@ -20,6 +20,7 @@
 #include "../../Globals.h"
 #include <vector>
 #include <set>
+#include <map>
 
 enum SymbolType
 {
@@ -27,6 +28,12 @@ enum SymbolType
 	ST_DATA=2
 };
 
+struct SymbolInfo
+{
+	u32 address;
+	u32 size;
+};
+
 #ifdef _WIN32
 struct HWND__;
 typedef struct HWND__ *HWND;
@@ -42,6 +49,7 @@ public:
 	void ResetSymbolMap();
 	void AnalyzeBackwards();
 	int GetSymbolNum(unsigned int address, SymbolType symmask=ST_FUNCTION) const;
+	bool GetSymbolInfo(SymbolInfo *info, u32 address, SymbolType symmask = ST_FUNCTION) const;
 	const char *GetDescription(unsigned int address) const;
 #ifdef _WIN32
 	void FillSymbolListBox(HWND listbox, SymbolType symmask=ST_FUNCTION) const;
@@ -96,6 +104,7 @@ private:
 
 	std::set<MapEntryUniqueInfo> uniqueEntries;
 	std::vector<MapEntry> entries;
+	std::map<u32, u32> entryRanges;
 };
 
 extern SymbolMap symbolMap;

Core/FileSystems/MetaFileSystem.cpp

@@ -383,7 +383,7 @@ int MetaFileSystem::RenameFile(const std::string &from, const std::string &to)
 	if (MapFilePath(from, of, &osystem))
 	{
 		// If it's a relative path, it seems to always use from's filesystem.
-		if (to.find(':/') != to.npos)
+		if (to.find(":/") != to.npos)
 		{
 			if (!MapFilePath(to, rf, &rsystem))
 				return -1;

Core/HLE/HLE.cpp

@@ -19,6 +19,7 @@
 #include "HLE.h"
 #include <map>
 #include <vector>
+#include <string>
 #include "../MemMap.h"
 #include "../Config.h"
 #include "Core/CoreTiming.h"
@@ -53,9 +54,12 @@ enum
 	HLE_AFTER_DEBUG_BREAK = 0x20,
 };
 
+typedef std::vector<Syscall> SyscallVector;
+typedef std::map<std::string, SyscallVector> SyscallVectorByModule;
+
 static std::vector<HLEModule> moduleDB;
-static std::vector<Syscall> unresolvedSyscalls;
-static std::vector<Syscall> exportedCalls;
+static SyscallVectorByModule unresolvedSyscalls;
+static SyscallVectorByModule exportedCalls;
 static int delayedResultEvent = -1;
 static int hleAfterSyscall = HLE_AFTER_NOTHING;
 static const char *hleAfterSyscallReschedReason;
@@ -83,9 +87,8 @@ void HLEInit()
 
 void HLEDoState(PointerWrap &p)
 {
-	Syscall sc = {""};
-	p.Do(unresolvedSyscalls, sc);
-	p.Do(exportedCalls, sc);
+	p.Do(unresolvedSyscalls);
+	p.Do(exportedCalls);
 	p.Do(delayedResultEvent);
 	CoreTiming::RestoreRegisterEvent(delayedResultEvent, "HLEDelayedResult", hleDelayResultFinish);
 	p.DoMarker("HLE");
@@ -156,14 +159,23 @@ const char *GetFuncName(const char *moduleName, u32 nib)
 	if (func)
 		return func->name;
 
+	SyscallModuleName searchModuleName = {0};
+	strncpy(searchModuleName, moduleName, KERNELOBJECT_MAX_NAME_LENGTH);
+	searchModuleName[KERNELOBJECT_MAX_NAME_LENGTH] = '\0';
+
 	// Was this function exported previously?
 	static char temp[256];
-	for (auto it = exportedCalls.begin(), end = exportedCalls.end(); it != end; ++it)
+	const auto exported = exportedCalls.find(searchModuleName);
+	if (exported != exportedCalls.end())
 	{
-		if (!strncmp(it->moduleName, moduleName, KERNELOBJECT_MAX_NAME_LENGTH) && it->nid == nib)
+		const SyscallVector &syscalls = exported->second;
+		for (auto it = syscalls.begin(), end = syscalls.end(); it != end; ++it)
 		{
-			sprintf(temp, "[EXP: 0x%08x]", nib);
-			return temp;
+			if (it->nid == nib)
+			{
+				sprintf(temp, "[EXP: 0x%08x]", nib);
+				return temp;
+			}
 		}
 	}
 
@@ -212,23 +224,29 @@ void WriteSyscall(const char *moduleName, u32 nib, u32 address)
 	}
 	else
 	{
+		Syscall sysc = {"", address, nib};
+		strncpy(sysc.moduleName, moduleName, KERNELOBJECT_MAX_NAME_LENGTH);
+		sysc.moduleName[KERNELOBJECT_MAX_NAME_LENGTH] = '\0';
+
 		// Did another module export this already?
-		for (auto it = exportedCalls.begin(), end = exportedCalls.end(); it != end; ++it)
+		const auto exported = exportedCalls.find(sysc.moduleName);
+		if (exported != exportedCalls.end())
 		{
-			if (!strncmp(it->moduleName, moduleName, KERNELOBJECT_MAX_NAME_LENGTH) && it->nid == nib)
+			const SyscallVector &syscalls = exported->second;
+			for (auto it = syscalls.begin(), end = syscalls.end(); it != end; ++it)
 			{
-				Memory::Write_U32(MIPS_MAKE_J(it->symAddr), address); // j symAddr
-				Memory::Write_U32(MIPS_MAKE_NOP(), address + 4); // nop (delay slot)
-				return;
+				if (it->nid == nib)
+				{
+					Memory::Write_U32(MIPS_MAKE_J(it->symAddr), address); // j symAddr
+					Memory::Write_U32(MIPS_MAKE_NOP(), address + 4); // nop (delay slot)
+					return;
+				}
 			}
 		}
 
 		// Module inexistent.. for now; let's store the syscall for it to be resolved later
 		INFO_LOG(HLE,"Syscall (%s,%08x) unresolved, storing for later resolving", moduleName, nib);
-		Syscall sysc = {"", address, nib};
-		strncpy(sysc.moduleName, moduleName, KERNELOBJECT_MAX_NAME_LENGTH);
-		sysc.moduleName[KERNELOBJECT_MAX_NAME_LENGTH] = '\0';
-		unresolvedSyscalls.push_back(sysc);
+		unresolvedSyscalls[sysc.moduleName].push_back(sysc);
 
 		// Write a trap so we notice this func if it's called before resolving.
 		Memory::Write_U32(MIPS_MAKE_JR_RA(), address); // jr ra
@@ -240,23 +258,28 @@ void ResolveSyscall(const char *moduleName, u32 nib, u32 address)
 {
 	_dbg_assert_msg_(HLE, moduleName != NULL, "Invalid module name.");
 
-	for (size_t i = 0; i < unresolvedSyscalls.size(); i++)
-	{
-		Syscall *sysc = &unresolvedSyscalls[i];
-		if (strncmp(sysc->moduleName, moduleName, KERNELOBJECT_MAX_NAME_LENGTH) == 0 && sysc->nid == nib)
-		{
-			INFO_LOG(HLE,"Resolving %s/%08x",moduleName,nib);
-			// Note: doing that, we can't trace external module calls, so maybe something else should be done to debug more efficiently
-			// Note that this should be J not JAL, as otherwise control will return to the stub..
-			Memory::Write_U32(MIPS_MAKE_J(address), sysc->symAddr);
-			Memory::Write_U32(MIPS_MAKE_NOP(), sysc->symAddr + 4);
-		}
-	}
-
 	Syscall ex = {"", address, nib};
 	strncpy(ex.moduleName, moduleName, KERNELOBJECT_MAX_NAME_LENGTH);
 	ex.moduleName[KERNELOBJECT_MAX_NAME_LENGTH] = '\0';
-	exportedCalls.push_back(ex);
+	exportedCalls[ex.moduleName].push_back(ex);
+
+	const auto unresolved = unresolvedSyscalls.find(ex.moduleName);
+	if (unresolved != unresolvedSyscalls.end())
+	{
+		const SyscallVector &syscalls = unresolved->second;
+		for (size_t i = 0; i < syscalls.size(); i++)
+		{
+			const Syscall *sysc = &syscalls[i];
+			if (sysc->nid == nib)
+			{
+				INFO_LOG(HLE,"Resolving %s/%08x",moduleName,nib);
+				// Note: doing that, we can't trace external module calls, so maybe something else should be done to debug more efficiently
+				// Note that this should be J not JAL, as otherwise control will return to the stub..
+				Memory::Write_U32(MIPS_MAKE_J(address), sysc->symAddr);
+				Memory::Write_U32(MIPS_MAKE_NOP(), sysc->symAddr + 4);
+			}
+		}
+	}
 }
 
 const char *GetFuncName(int moduleIndex, int func)

Core/HLE/HLE.h

@@ -48,9 +48,11 @@ struct HLEModule
 	const HLEFunction *funcTable;
 };
 
+typedef char SyscallModuleName[32];
+
 struct Syscall
 {
-	char moduleName[32];
+	SyscallModuleName moduleName;
 	u32 symAddr;
 	u32 nid;
 };

Core/MIPS/MIPSAnalyst.cpp

@@ -351,10 +351,10 @@ namespace MIPSAnalyst
 		u32 addr;
 		for (addr = startAddr; addr<=endAddr; addr+=4)
 		{
-			int n = symbolMap.GetSymbolNum(addr,ST_FUNCTION);
-			if (n != -1)
+			SymbolInfo syminfo;
+			if (symbolMap.GetSymbolInfo(&syminfo, addr, ST_FUNCTION))
 			{
-				addr = symbolMap.GetSymbolAddr(n) + symbolMap.GetSymbolSize(n);
+				addr = syminfo.address + syminfo.size;
 				continue;
 			}