Memory Manager woes

Started by
7 comments, last by bkt 19 years, 2 months ago
I just recently had enough time to rewrite my memory manager (was using Engunity's original implementation) and I've hit a few snags. I haven't worked on getting reference counting setup yet, I just wanted to make sure that everything work, and at first it seemed like it did. The problem I was having though is that my debug information for allocation (overloaded 'new' operator) doesn't seem to be writing. I know the log system works; and I know that the overloaded delete operator is reporting deletion. On top of all of that, it seems that I'm stuck in a loop of some inifinte loop somewhere (which wasn't there before). Here are two examples of the log files that are being produced: log.html dbg.html Here's the code: Memory.cpp

void MemoryManager::Create(void) {
	m_LiveObjects.clear();
	m_DeadObjects.clear();

	m_lTopHash 	= 0;
	m_lHeapAllocSize = 0;
}

void MemoryManager::Destroy(void) {
	// this is a direct-kill; nothing in between ;)
	m_LiveObjects.Release();
	m_DeadObjects.Release();

	#ifdef _DEBUG
		if(m_lHeapAllocSize > 0)
			SysMem(__FUNCTION__,"Heap allocation size is wrong; %d",m_lHeapAllocSize);

		SysMem(__FUNCTION__,"Total memory objects this run %d",m_lTopHash-1);
	#endif

	m_LiveObjects.clear();
	m_DeadObjects.clear();
}

void MemoryManager::KillObject(ulong lHash) {
	// when this is called; we're moving an object from the "live" list to
	// the "dead" list. So we need to check to make sure it's here first!
	if(m_LiveObjects[lHash] != NULL) {
		IMemoryObject* p = m_LiveObjects[lHash];

		// make sure to get rid of her
		m_LiveObjects.remove(p);

		// houston - we have a problem!
		if(m_DeadObjects[lHash] != NULL) {
			// assign the object a new hash id
			p->m_lHashID = NextHashID();

			#ifdef _DEBUG
				SysMem(__FUNCTION__,"Memory object with hash %d already exists on dead list; reassigning hash to %d",
					lHash,p->m_lHashID);
			#endif
		}
		m_DeadObjects[p->m_lHashID] = p;

		// flag the object as killed incase we need to bring it back onto the live list
		p->m_bKilled = true;

		#ifdef _DEBUG
			SysMem(__FUNCTION__,"Memory object moved to dead list; %d",p->m_lHashID);
		#endif
		return;
	}
	SysWarn(__FUNCTION__,"Memory object with hash %d does not exist",lHash);
	return;
}

//j: this is how we're going to allocate blocks of memory; this placed in the
//"" overloaded "new" operators of all devrived IMemoryObject objects
IMemoryObject* MemoryManager::AllocBlock(size_t size) {
	IMemoryObject* p = (IMemoryObject*)malloc(size);
	*p = IMemoryObject();		// invoke the constructor

	m_lHeapAllocSize += static_cast<ulong> (size);

	#ifdef _DEBUG
		SysMem(__FUNCTION__,"Memory block allocated with a size of %d",size);
	#endif
	return p;
}

//j: deallocation of memory blocks (otherwise 'freeing')
void MemoryManager::DeallocBlock(IMemoryObject* p) {
	m_lHeapAllocSize -= p->m_lAllocSize;

	free(p);

	#ifdef _DEBUG
		SysMem(__FUNCTION__,"Memory block deallocated with size of %d; heap %d",p->m_lAllocSize,m_lHeapAllocSize);
	#endif
}

//j: the good ole 'collect garbage' function; woo
void MemoryManager::CollectGarbage(void) {
	for(CMemoryMap::iterator it=m_DeadObjects.begin();
		it != m_DeadObjects.end(); it++) {
			IMemoryObject* p = it->second; // store pointer
			it = m_DeadObjects.remove( it );	 // remove from list

			DeallocBlock( p );				 // deallocate
	}
	m_DeadObjects.clear();						 // start fresh
}

void MemoryManager::AddObject(IMemoryObject* p) {
	if(m_LiveObjects[p->m_lHashID] != NULL) {
		SysWarn(__FUNCTION__,"Memory object with hash %d already in live list",p->m_lHashID);
		p->m_lHashID = NextHashID();
	}
	m_LiveObjects[p->m_lHashID] = p;
	#ifdef _DEBUG
	SysMem(__FUNCTION__,"Memory block with hash %d added to live list",p->m_lHashID);
	#endif
}

IMemoryObject::IMemoryObject(void) {
	MemoryManager mem;

	m_lAllocSize=sizeof(*this);
	m_lHashID=mem.NextHashID();
	m_bKilled=false;

	mem.AddObject( this );
 }

//j: overload the new operator with the memory manager's allocation method
void* IMemoryObject::operator new(size_t size) {
	MemoryManager mem;
	return ( mem.AllocBlock(size) );
}

//j: overload the delete operator with the memory manager's deallocation method
void IMemoryObject::operator delete(void* p) {
	MemoryManager mem;
	mem.KillObject( ((IMemoryObject*)p)->m_lHashID );
}


Memory.hpp & MemoryObject.hpp

//j: I have decided to make the Memory Manager a Monostate instead of a
//"" singleton; this can always be changed in the future if deemed needed
class MemoryManager : public IMonoState {
	public:
		MemoryManager(void) { }

		~MemoryManager(void) { }

		static void Create(void);
		static void Destroy(void);

		void KillObject(ulong);
		void AddObject(IMemoryObject*);

		IMemoryObject* AllocBlock(size_t);
		void  DeallocBlock(IMemoryObject*);
		IMemoryObject* ReallocBlock(IMemoryObject*,size_t);

		void CollectGarbage(void);

		inline ulong NextHashID(void) const { return m_lTopHash++; }
	private:
		//j: we're going to have our hash map an extension of std::map
		class CMemoryMap : public map_t<ulong,IMemoryObject*> {
			public:
				// physically dump all of the objects onto the garbage collector
				inline void Release(void) {
					MemoryManager mem;

					for(iterator it=begin(); it != end(); it++)
						mem.DeallocBlock( it->second );
					clear();
				}

				// I don't know why the STL doesn't return the previous iterator;
				// but it just sticks me off when I need to create another instance
				// just to delete a damn object ; so here we are 
				inline iterator remove( iterator it ) {
					iterator i = it--;
					erase( i );
					return it;
				}

				// if we want to remove something by having a pointer of the obj
				inline iterator remove( IMemoryObject* p ) {
					ulong hash=p->GetHash();
					for(iterator it=begin(); it != end(); it++) {
						if(hash==it->first) {
							if(p==it->second) {
								it=remove(it);
								return it;
							}
						}
					}
					return NULL;
				}						
		};

		static CMemoryMap m_LiveObjects;
		static CMemoryMap m_DeadObjects;

		static ulong m_lTopHash;
		static ulong m_lHeapAllocSize;
};

//j: this is the base memory object that can be devrived from anything
//"" that we wish to utilize the memory manager interface
class IMemoryObject {
	friend class MemoryManager;
	public:
		IMemoryObject(void);
		virtual ~IMemoryObject(void) { }

		void* operator new(size_t);
		void  operator delete(void*);
		virtual void Release(void);

		// this is how we deal with objects in the DeadObjects list
		inline bool IsKilled(void) const { return m_bKilled; }
		inline ulong GetHash(void) const { return m_lHashID; }
	private:
		ulong m_lAllocSize;
		ulong m_lHashID;
		bool m_bKilled;

};


I know it's a lot of code but I didn't include all of it because I didn't think it would be relevant. If you think you need more/all of it; just ask. There is no compile errors; and like I said I haven't setup memory reference counting yet. I have a feeling that this may be happening because I'm still using a modified version of the "smart pointer" class that Superpig had in his tutorial.
-John "bKT" Bellone [homepage] [[email=j.bellone@flipsidesoftware.com]email[/email]]
Advertisement
I would suggest you look at an industrial strength memory manager such as the one Paul Nettle has written (http://www.fluidstudios.com/). There are some bugs in it which probably won't be apparent until you use templates. I've emailed him about it but I don't know if it has been fixed yet.

There are several problems with your solution, for instance you invoke the ctor completly wrong. To invoke the ctor on an allocated object you need to call placement new:
unsigned char *buf = (unsigned char *)malloc(size);new (buf)IMemoryObject;

But I think a better way for a memory manager would be to overload new globally in your app, then the invocation of the ctor is done automagically for you. A no-throw new could look like this then:
void *operator new(size_t bytes){  return malloc(bytes);}

Also I think it's a mess if everything that should be allocatable must inherit from IMemoryObject. I promise if you think about it (or start using this seriously) you'll realise there are loads of problems with this design. I'm sorry to say this but I'm afraid you're in a dead end here.

One of the big gains of using a globally overloaded new/delete is other stuff like STL will use your memory manager too. This makes it ideal to track memory leaks and illegal accesses.

Good luck!
Originally I only wanted certian objects to be handled by the memory manager; but I thought I would eventually be changing it. I took a look at Fluid Studios' manager, it looked great but I've always been a person that wants to write my own things (that's how I learn the best). So if you know of any white papers that I can read on some good design methods for a memory manager that would be great.

I'm definately going to change the manager; I don't need it to be industrial strength just something that works at first. If you have any suggestions or resources for me to use definately let me know. Thanks a lot man!

[Edited by - bkt on February 22, 2005 8:19:24 AM]
-John "bKT" Bellone [homepage] [[email=j.bellone@flipsidesoftware.com]email[/email]]
I don't know any white papers for writing a memory manager, but to get started with something really simple, skip classes (ctor/dtor problems) and just create your own wrappers around *alloc/free. In those wrappers you can do the proper handling to keep track of allocations. To get started write something like:
struct mem_header{  size_t size;  void *block;  mem_header *next;};namespace {  mem_header *first_block = 0;};bool mem_alloc(void **buf, size_t size){  (*buf) = malloc(size);  if(*buf == 0)    return false;  mem_header *mh = static_cast<mem_header *>(malloc(sizeof(mem_header)));    if(mh == 0)  {    free(*buf);    return false;  }  mh->size = size;  mh->block = *buf;  mh->next = first_block;  first_block = mh;  return true;}

Then you add stuff (some sort of header, either separately allocated or allocated *before* the real allocation) to keep track of all allocations. Then you write tests that only use your own memory functions to verify that they work correctly. After that you can replace new/delete and *alloc/free. Be sure to read about the semantics of each function you want to "overload".

A good read to just get some basics in a simple memory manager is "Writing Solid Code" by Steve Maguire (ISBN: 1556155514).
Otherwise reading through the sources of the Fluid Studios memory manager will definitely be a useful read. I have to say I find them quite straight forward and not as complex as one might think. So even if you want to roll your own for the case of learning, reading through those sources and trying to understand them is definitely worth the time (besides, it's free).

Good luck!
What I was considering doing (let me know what you think) is the following:
I was going to keep the basic structure of devriving all "managed" objects with IMemoryObject; this would allow me to keep a hash table with the data inside it for quick lookups. But then for the rest of the code; I would just have an unmanaged list with a structure (actually similar to what you just posted) holding the pointer, size, and maybe reference count (haven't decided if I'll do reference counting on unmanaged objects).

But let me know what you think! Thanks for the input man, I really appreciate it!
-John "bKT" Bellone [homepage] [[email=j.bellone@flipsidesoftware.com]email[/email]]
Hey, sure, memory management is fun.
I have some questions you might want to answer for yourself as much as for me...
* What's the difference between "managed" and "unmanaged" data/objects in your code? Are managed objects garbage collected or is it something else?

* About keeping a hash table for quick look ups, when will you need quick look ups? Are you sure a hash table really is what you need?

I'm sorry if I ask a lot of stuff but I have a tendency towards liking simplicity and not to optimize (code or design) prematurely.

Cheers,
Andreas
"Managed" objects are devrived objects that have been created by the engine; i.e. let's say:
class CVertex : public IMemoryObject

Managed objects (because they are devrived from IMemoryObject have their hash id number, size of the object, and number of references (they are reference dependant). When they are deleted; they are thrown into the same bucket as the unmanaged objects (when deleted).

Unmanaged objects are everything else: strings, vectors (arrays), link lists, etc. They are not reference counted, and do not have a hash id number. Basically what the "hash" number (it's actually a map) allows for a quick lookup of the object for deletion. Unmanged objects will be stored like this:
struct unmanaged_memalloc_t {  void* m_Pointer;  unsigned long m_lSize;};

Basically the global new and delete operators are overloaded and when something is deleted the pointer can be searched for and then the object can be removed from the list and sent to the dead objects list (which has both managed and unmanaged objects).

Everything is garbage collected; but managed objects have reference counting and have a lot quicker lookup when deleting/modifying/referencing. Quick lookups will be needed because "game objects" are probably going to more likely be needed to be referenced (and since I want reference counting it's probably more efficient to store those objects in a std::map).

**Update: I was just thinking about referencing counting and the managed object; I could also make the interface more like a smart pointer object thus allowing a bit more resistance to calling dead object pointers later on in the code.

Just a few thoughts; let me know what you think about it.
-John "bKT" Bellone [homepage] [[email=j.bellone@flipsidesoftware.com]email[/email]]
Hmm, I'm sorry but I think it all sounds quite complex. Why would you want to have two different types of memory allocation? Why not have the faster for strings and other stuff as well? You can hash on the address of the allocated block.

The only reason I can see to have two different versions of memory allocations (and I'm a bit dubious about that too) is the one given in "Modern C++ Design" by Alexandrescu. In that book he presents a version that's fast for small objects (smaller than some predefined size). I don't remember the deal now, but he still built it on top of a "real" memory manager (be it your own or the built in).

Have you thought about how you will handle reference counting?

There's an old article here at GameDev that presents a referenced counted object (I'm sorry I don't have the URL). It is to be used on top of a real memory manager, maybe not what you want?
Anyway in this article basically any reference counted object needs to derive from a ReferenceCounter base class and then there's a templated class that wraps it and provides accessor methods (some that calls AddRef/SubRef in the ReferenceCounter class).

I think it feels a bit like you're mixing two different concepts together, memory management and game objects. Or maybe it's just me who have misunderstood. But anyway, game objects should not depend on the memory manager to do something for them (except allocate memory). You should be able to switch memory managers under the feet of the game objects and they shouldn't care. Instead you should have a game object manager for handling all game objects.

Cheers!
You're right; I realized this after I wrote out the design for the code (always seems to happen for me). I reorganized my approach and now only have one object and overload the global operators for new and delete; something I probably should have done from the start. The code compiles file; but nothing is booting in the engine (yay!) I'm going to step through it with a debugger later; here's what I have so far.
class CMemBucket {	friend class CMemBucket;	friend class MemoryManager;	public:		CMemBucket(void* pointer,size_t size) {			m_Pointer	= pointer;			m_Size		= size;			m_RefCount	= 1;		}		~CMemBucket(void) { }		// for the sorting algorithm that is processed in Sort()		inline bool operator() (CMemBucket*& cmp) {			return ( m_Size < cmp->m_Size );		}		// we need to overload the operators because we're overloading		// the global operators - we don't want these buckets being sent		// to the pail		inline void* operator new(size_t bucketSize) {			return (void*)malloc( bucketSize );		}		inline void operator delete(void* p) {			free( p );			return;		}	private:		void* 	m_Pointer;		size_t 	m_Size;		uint	m_RefCount;};class MemoryManager : public IMonoState {	public:		MemoryManager(void) { }		~MemoryManager(void) { }		// call these when we want to create the monostate		static void Create(void);		static void Destroy(void);		inline void CollectGarbage(void) { m_DeadBuckets.Release(); }		void* allocBucket(size_t);		void  deallocBucket(CMemBucket*);		void  addBucket(void*);		void  remBucket(void*);		inline size_t GetAllocSize(void) const { return m_HeapAllocSize; }	private:		// the basic run of the mills overloaded link list object		class CMemoryList : public array_t<CMemBucket*> {			public:				// we can run this at the end and dump all the objects to				// automagically deallocate them! No overhead! Yay?1				inline void Release(void) {					MemoryManager mem;					// run through and send it to deallocate					for(iterator it=begin();						it != end(); it++)							mem.deallocBucket( (*it) );				}				// we need to be able to sort things if need be				inline void Sort(void) {					std::sort( begin(),end() );				}		};		static CMemoryList			m_LiveBuckets;		static CMemoryList			m_DeadBuckets;		static size_t				m_HeapAllocSize;};


Memory.cpp
void* MemoryManager::allocBucket(size_t bucketSize) {	void* p = (void*) malloc(bucketSize);	// add size to the heap so we can track	m_HeapAllocSize += sizeof(p);	return p;}//j: we're going to "deallocate" a bucket nowvoid MemoryManager::deallocBucket(CMemBucket* bucket) {	bucket->m_RefCount--;	if(bucket->m_RefCount == 0) {		// remove the sizes from the heap		m_HeapAllocSize -= bucket->m_Size;		// free up the actual memory object		free( bucket->m_Pointer );		// now free up the footprint		delete bucket;	}	return;}void MemoryManager::addBucket(void* p) {	// check the pointer	if(!p) {		SysWarn(__FUNCTION__,"Invaild memory pointer passed",0);		return;	}	size_t bucketSize = sizeof(p);	CMemBucket* bucket = new CMemBucket( p, bucketSize );	m_LiveBuckets.push_back( bucket );	#ifdef _DEBUG		SysMem(__FUNCTION__,"Memory bucket added to live list; size %d/%d, address %s",			bucketSize, m_HeapAllocSize, &p );	#endif	return;}void MemoryManager::remBucket(void* p) {	// check the pointer	if(!p) {		SysWarn(__FUNCTION__,"Invaild memory pointer passed",0);		return;	}	int iMin = 1,iMax=(int)m_LiveBuckets.size(),iMid=0;	CMemBucket* bucket = m_LiveBuckets[0];	size_t bucketSize = sizeof(p);	// let's check the first bucket in the pool	if(bucket->m_Size == bucketSize) {		// we need to make sure the pointer equals		if(bucket->m_Pointer == p) {			m_LiveBuckets[0] = NULL;			m_DeadBuckets.push_back( bucket );			return;		}	}	// we're going to do a binary searching algorithm to work the	// quickest way possible to find the buckets because they are	// going to be sorted by the size of the memory allocated	while(iMin <= iMax) {		iMid = ( (iMax + iMin) / 2 );		bucket = m_LiveBuckets[iMid];		// let's check the first bucket in the pool		if(bucket->m_Size == bucketSize) {			// we need to make sure the pointer equals			if(bucket->m_Pointer == p) {				m_LiveBuckets[iMid] = NULL;				m_DeadBuckets.push_back( bucket );				return;			}		}		else if(bucket->m_Size > bucketSize)			iMax = iMid-1;		else if(bucket->m_Size < bucketSize)			iMin = iMid+1;	}	// well this would definately be rather bad!	SysWarn(__FUNCTION__,"Memory object does not exist in list; size %d, address %s",		bucketSize,&p);	return;}//j: now we're going to globally overload the new and delete operatorsvoid* operator new(size_t bucketSize) {	MemoryManager mem;	void* p	 = mem.allocBucket( bucketSize );	mem.addBucket( p );	return p;}void operator delete(void* p) {	MemoryManager mem;	mem.remBucket( p );	return;}


I haven't added reference counting so that may be the problem; I'll work on it later when this snow storm decides to come through and shut my whole town down.
-John "bKT" Bellone [homepage] [[email=j.bellone@flipsidesoftware.com]email[/email]]

This topic is closed to new replies.

Advertisement