Improve iteration in DumpMemoryObjects

Author: slurmlord

Core/GameEngine/Include/Common/GameMemory.h

@@ -285,7 +285,7 @@ class Checkpointable
 #ifdef RTS_ENABLE_CRASHDUMP
 struct MemoryPoolAllocatedRange
 {
-	char* allocationAddr;
+	const char* allocationAddr;
 	size_t allocationSize;
 };
 #endif
@@ -489,8 +489,9 @@ class DynamicMemoryAllocator
 
 	#endif	// MEMORYPOOL_DEBUG
 #ifdef RTS_ENABLE_CRASHDUMP
-		Int getRawBlockCount() const;
-		void fillAllocationRangeForRawBlockN(const Int n, MemoryPoolAllocatedRange& allocationRange) const;
+		MemoryPoolSingleBlock* getFirstRawBlock() const;
+		MemoryPoolSingleBlock* getNextRawBlock(MemoryPoolSingleBlock* block) const;
+		void fillAllocationRangeForRawBlock(const MemoryPoolSingleBlock*, MemoryPoolAllocatedRange& allocationRange) const;
 #endif
 };
 
@@ -665,8 +666,7 @@ class MemoryPoolFactory
 			return AllocationRangeIterator(NULL, NULL);
 		}
 
-		Int getMemoryPoolCount() const;
-		MemoryPool* getMemoryPoolN(const Int n) const;
+		MemoryPool* getFirstMemoryPool() const;
 		friend class AllocationRangeIterator;
 #endif
 };

Core/GameEngine/Include/Common/MiniDumper.h

@@ -41,6 +41,7 @@ enum MiniDumperExitCode CPP_11(: Int)
 
 enum DumpObjectsState CPP_11(: Int)
 {
+	BEGIN,
 	MEMORY_POOLS,
 	MEMORY_POOL_ALLOCATIONS,
 	DMA_ALLOCATIONS,
@@ -111,8 +112,9 @@ class MiniDumper
 #ifndef DISABLE_GAMEMEMORY
 	// Internal memory dumping progress state
 	DumpObjectsState m_dumpObjectsState;
-	Int m_dumpObjectsSubState;
 	DynamicMemoryAllocator* m_currentAllocator;
+	MemoryPool* m_currentPool;
+	MemoryPoolSingleBlock* m_currentSingleBlock;
 
 	AllocationRangeIterator m_rangeIter;
 #endif

Core/GameEngine/Source/Common/System/GameMemory.cpp

@@ -445,7 +445,7 @@ class MemoryPoolSingleBlock
 	void setNextRawBlock(MemoryPoolSingleBlock *b);
 
 #if defined(MEMORYPOOL_DEBUG) || defined(RTS_ENABLE_CRASHDUMP)
-	Int debugGetLogicalSize();
+	Int debugGetLogicalSize() const;
 #endif
 #ifdef MEMORYPOOL_DEBUG
 	void debugIgnoreLeaksForThisBlock();
@@ -651,7 +651,7 @@ inline const char *MemoryPoolSingleBlock::debugGetLiteralTagString()
 /**
 	accessor
 */
-inline Int MemoryPoolSingleBlock::debugGetLogicalSize()
+inline Int MemoryPoolSingleBlock::debugGetLogicalSize() const
 {
 	//USE_PERF_TIMER(MemoryPoolDebugging) not worth it
 	return m_logicalSize;
@@ -2591,24 +2591,19 @@ void DynamicMemoryAllocator::debugDmaInfoReport( FILE *fp )
 #endif
 
 #ifdef RTS_ENABLE_CRASHDUMP
-Int DynamicMemoryAllocator::getRawBlockCount() const
+MemoryPoolSingleBlock* DynamicMemoryAllocator::getFirstRawBlock() const
 {
-	Int count = 0;
-	for (MemoryPoolSingleBlock* block = m_rawBlocks; block; block = block->getNextRawBlock())
-	{
-		++count;
-	}
+	return m_rawBlocks;
+}
 
-	return count;
+MemoryPoolSingleBlock* DynamicMemoryAllocator::getNextRawBlock(MemoryPoolSingleBlock* block) const
+{
+	return block->getNextRawBlock();
 }
-void DynamicMemoryAllocator::fillAllocationRangeForRawBlockN(const Int n, MemoryPoolAllocatedRange& allocationRange) const
+
+void DynamicMemoryAllocator::fillAllocationRangeForRawBlock(const MemoryPoolSingleBlock* block, MemoryPoolAllocatedRange& allocationRange) const
 {
-	MemoryPoolSingleBlock* block = m_rawBlocks;
-	for (int i = 0; i < n; ++i)
-	{
-		block = block->getNextRawBlock();
-	}
-	allocationRange.allocationAddr = reinterpret_cast<char*>(block);
+	allocationRange.allocationAddr = reinterpret_cast<const char*>(block);
 	allocationRange.allocationSize = block->calcRawBlockSize(block->debugGetLogicalSize());
 }
 #endif
@@ -3273,30 +3268,9 @@ void MemoryPoolFactory::debugMemoryReport(Int flags, Int startCheckpoint, Int en
 #endif
 
 #ifdef RTS_ENABLE_CRASHDUMP
-Int MemoryPoolFactory::getMemoryPoolCount() const
+MemoryPool* MemoryPoolFactory::getFirstMemoryPool() const
 {
-	Int count = 0;
-	MemoryPool* current = m_firstPoolInFactory;
-	while (current != NULL)
-	{
-		++count;
-		current = current->getNextPoolInList();
-	}
-
-	return count;
-}
-
-MemoryPool* MemoryPoolFactory::getMemoryPoolN(const Int n) const
-{
-	Int count = 0;
-	MemoryPool* current = m_firstPoolInFactory;
-	while (count < n && current != NULL)
-	{
-		++count;
-		current = current->getNextPoolInList();
-	}
-
-	return current;
+	return m_firstPoolInFactory;
 }
 
 AllocationRangeIterator::AllocationRangeIterator(const MemoryPoolFactory* factory)
@@ -3334,7 +3308,6 @@ void AllocationRangeIterator::MoveToNextBlob()
 		m_currentBlobInPool = NULL;
 	}
 }
-
 #endif
 
 //-----------------------------------------------------------------------------

Core/GameEngine/Source/Common/System/MiniDumper.cpp

@@ -71,8 +71,9 @@ MiniDumper::MiniDumper()
 	m_dumpThreadId = 0;
 #ifndef DISABLE_GAMEMEMORY
 	m_dumpObjectsState = MEMORY_POOLS;
-	m_dumpObjectsSubState = 0;
 	m_currentAllocator = NULL;
+	m_currentSingleBlock = NULL;
+	m_currentPool = NULL;
 #endif
 	m_dumpDir[0] = 0;
 	m_dumpFile[0] = 0;
@@ -372,7 +373,7 @@ void MiniDumper::CreateMiniDump(DumpType dumpType)
 		return;
 	}
 
-	m_dumpObjectsState = MEMORY_POOLS;
+	m_dumpObjectsState = BEGIN;
 
 	PMINIDUMP_EXCEPTION_INFORMATION exceptionInfoPtr = NULL;
 	MINIDUMP_EXCEPTION_INFORMATION exceptionInfo = { 0 };
@@ -495,7 +496,7 @@ BOOL MiniDumper::CallbackInternal(const MINIDUMP_CALLBACK_INPUT& input, MINIDUMP
 		{
 			// DumpMemoryObjects will set outputMemorySize to 0 once it's completed, signalling the end of memory callbacks
 			DumpMemoryObjects(output.MemoryBase, output.MemorySize);
-		} while ((output.MemoryBase == 0 || output.MemorySize == 0) && m_dumpObjectsState != COMPLETED);
+		} while (output.MemorySize == 0 && m_dumpObjectsState != COMPLETED);
 #else
 		output.MemoryBase = 0;
 		output.MemorySize = 0;
@@ -506,6 +507,8 @@ BOOL MiniDumper::CallbackInternal(const MINIDUMP_CALLBACK_INPUT& input, MINIDUMP
 			input.ReadMemoryFailure.Offset, input.ReadMemoryFailure.Bytes, input.ReadMemoryFailure.FailureStatus));
 		break;
 	case CancelCallback:
+		// The cancel callback checks if the current minidump should be cancelled.
+		// Set output to never wish to cancel, nor to receive cancel callbacks in the future.
 		output.Cancel = FALSE;
 		output.CheckCancel = FALSE;
 		break;
@@ -521,119 +524,85 @@ void MiniDumper::DumpMemoryObjects(ULONG64& memoryBase, ULONG& memorySize)
 	// m_dumpObjectsSubState is used to keep track of the progress within each phase, and is reset when advancing on to the next phase
 	switch (m_dumpObjectsState)
 	{
+	case BEGIN:
+		m_dumpObjectsState = MEMORY_POOLS;
+		if (TheMemoryPoolFactory)
+		{
+			m_currentPool = TheMemoryPoolFactory->getFirstMemoryPool();
+		}
+		break;
 	case MEMORY_POOLS:
 	{
 		// Dump all the MemoryPool instances in TheMemoryPoolFactory
 		// This only dumps the metadata, not the actual MemoryPool contents (done in the next phase).
-		if (TheMemoryPoolFactory == NULL)
+		if (m_currentPool == NULL)
 		{
 			m_dumpObjectsState = MEMORY_POOL_ALLOCATIONS;
-			break;
-		}
-
-		Int poolCount = TheMemoryPoolFactory->getMemoryPoolCount();
-		//m_dumpObjectsSubState contains the index in TheMemoryPoolFactory of the MemoryPool that is being processed
-		if (m_dumpObjectsSubState < poolCount)
-		{
-			MemoryPool* pool = TheMemoryPoolFactory->getMemoryPoolN(m_dumpObjectsSubState);
-			if (pool != NULL)
-			{
-				memoryBase = reinterpret_cast<ULONG64>(pool);
-				memorySize = sizeof(MemoryPool);
-				++m_dumpObjectsSubState;
-			}
-			else
+			if (TheMemoryPoolFactory)
 			{
-				m_dumpObjectsSubState = poolCount;
+				m_rangeIter = TheMemoryPoolFactory->cbegin();
 			}
+			break;
 		}
 
-		if (m_dumpObjectsSubState == poolCount)
-		{
-			m_dumpObjectsSubState = 0;
-			m_dumpObjectsState = MEMORY_POOL_ALLOCATIONS;
-		}
+		memoryBase = reinterpret_cast<ULONG64>(m_currentPool);
+		memorySize = sizeof(MemoryPool);
+		m_currentPool = m_currentPool->getNextPoolInList();
 		break;
 	}
 	case MEMORY_POOL_ALLOCATIONS:
 	{
 		// Iterate through all the allocations of memory pools and containing blobs that has been done via the memory pool factory
 		// and include all of the storage space allocated for objects
-		if (TheMemoryPoolFactory == NULL)
+		if (m_rangeIter == TheMemoryPoolFactory->cend())
 		{
 			m_dumpObjectsState = DMA_ALLOCATIONS;
+			m_currentAllocator = TheDynamicMemoryAllocator;
+			m_currentSingleBlock = m_currentAllocator->getFirstRawBlock();
 			break;
 		}
 
-		//m_dumpObjectsSubState is used to track if the iterator needs to be initialized, otherwise just a counter of the number of items dumped
-		if (m_dumpObjectsSubState == 0)
-		{
-			m_rangeIter = TheMemoryPoolFactory->cbegin();
-			++m_dumpObjectsSubState;
-		}
-
-		// m_RangeIter should != cend() at this point before advancing, unless the memory pool factory is corrupted (or has 0 entries)
 		memoryBase = reinterpret_cast<ULONG64>(m_rangeIter->allocationAddr);
 		memorySize = m_rangeIter->allocationSize;
-		++m_dumpObjectsSubState;
 		++m_rangeIter;
-
-		if (m_rangeIter == TheMemoryPoolFactory->cend())
-		{
-			m_dumpObjectsState = DMA_ALLOCATIONS;
-			m_dumpObjectsSubState = 0;
-		}
 		break;
 	}
 	case DMA_ALLOCATIONS:
 	{
 		// Iterate through all the direct allocations ("raw blocks") done by DMAs, as these are done outside of the
 		// memory pool factory allocations dumped in the previous phase.
-		if (TheDynamicMemoryAllocator == NULL)
+		if (m_currentAllocator == NULL)
 		{
 			m_dumpObjectsState = COMPLETED;
 			break;
 		}
 
-		if (m_currentAllocator == NULL)
+		if (m_currentSingleBlock == NULL)
 		{
-			m_currentAllocator = TheDynamicMemoryAllocator;
-			// m_dumpObjectsSubState is used to track the index of the raw block in the allocator we are currently traversing
-			m_dumpObjectsSubState = 0;
+			// Iterated to a new allocator, start iterating over its blocks
+			m_currentSingleBlock = m_currentAllocator->getFirstRawBlock();
 		}
 
-		MemoryPoolAllocatedRange rawBlockRange = {0};
-		int rawBlocksInDma = m_currentAllocator->getRawBlockCount();
-		if (m_dumpObjectsSubState < rawBlocksInDma)
-		{
-			// Dump this block
-			m_currentAllocator->fillAllocationRangeForRawBlockN(m_dumpObjectsSubState, rawBlockRange);
-			memoryBase = reinterpret_cast<ULONG64>(rawBlockRange.allocationAddr);
-			memorySize = rawBlockRange.allocationSize;
-			++m_dumpObjectsSubState;
-		}
+		MemoryPoolAllocatedRange rawBlockRange = { 0 };
+		m_currentAllocator->fillAllocationRangeForRawBlock(m_currentSingleBlock, rawBlockRange);
+		memoryBase = reinterpret_cast<ULONG64>(rawBlockRange.allocationAddr);
+		memorySize = rawBlockRange.allocationSize;
+		m_currentSingleBlock = m_currentAllocator->getNextRawBlock(m_currentSingleBlock);
 
-		if (rawBlocksInDma == m_dumpObjectsSubState)
+		if (m_currentSingleBlock == NULL)
 		{
-			// Advance to the next DMA
 			m_currentAllocator = m_currentAllocator->getNextDmaInList();
-			m_dumpObjectsSubState = 0;
-
-			if (m_currentAllocator == NULL)
-			{
-				// Done iterating through all the DMAs
-				m_dumpObjectsState = COMPLETED;
-			}
 		}
 		break;
 	}
-	default:
+	case COMPLETED:
 		// Done, set "no more regions to dump" values
-		m_dumpObjectsState = COMPLETED;
-		m_dumpObjectsSubState = 0;
 		memoryBase = 0;
 		memorySize = 0;
 		break;
+	default:
+		DEBUG_CRASH(("Invalid object state"));
+		break;
 	}
 }
 #endif