Is it ok to create a Simple memory pool using singleton and a queue of pointers?

[peter peterson]

I have been looking at memory pools and was wondering what would be wrong with creating a singleton which create a "pool" of (allocated) pointers to a given struct/class( in the case below Messages ), and then when a client needs a "new" one pass them one from your free message queue. Any thoughts, potential problems, etc, would be appreciated. Thanks.

class MsgPool
{
public:
	
	~MsgPool();
	
	static void recycleMem(Message* msg);
	static Message* newMessage();
	static MsgPool* createInstance();

private:

	MsgPool();

	static MsgPool* msgPoolInstance;
	static Message* msgPtr;
	static std::queue<Message*> msgPtrPool;
};

MsgPool* MsgPool::msgPoolInstance = NULL;
std::queue<Message*> MsgPool::msgPtrPool;
Message* MsgPool::msgPtr;

//CCONSTRUCTOR/DESTRUCTORS
//----------------------------------------------------------------------------------------------//

MsgPool::MsgPool()
{
	while (msgPtrPool.size() < INITPTRPOOL) {
		msgPtr = new Message();
		msgPtrPool.push(msgPtr);
	}
}
MsgPool::~MsgPool()
{
	if (msgPoolInstance != NULL) {
		while (!msgPtrPool.empty()) {
			msgPtr = msgPtrPool.front();
			msgPtrPool.pop();
			delete msgPtr;
		}
	
		delete msgPoolInstance;
		msgPoolInstance = NULL;
	}
}
//----------------------------------------------------------------------------------------------//

//INSTANCE CREATION
//----------------------------------------------------------------------------------------------//
//This function creates the one and only instance of the message pool
MsgPool* MsgPool::createInstance()
{
	if (msgPoolInstance == NULL) {
		msgPoolInstance = new MsgPool();
	}
		return msgPoolInstance;
}
//----------------------------------------------------------------------------------------------//

//"ALLOCATION/DEALLOCATION"
//----------------------------------------------------------------------------------------------//

//This function puts a given pointer back into the pool
void MsgPool::recycleMem(Message* msg)
{
	if (msg != NULL) {
		msgPtrPool.push(msg);
	}
	else
		std::cerr <<"Passed NULL ptr" << std::endl;
}

//This function returns a message pointer
Message* MsgPool::newMessage()
{
	//If out of memory in pool allocate new memory
	if (msgPtrPool.empty()) {
		return (new Message);
	}
	else
	{
		msgPtr = msgPtrPool.front();
		msgPtrPool.pop();
		return msgPtr;
	}
	
}

[Oralloy]

Of course it's ok to create such a pool.

In some instances, it's sometimes beneficial to application performance, too.

The question is rather - is it appropriate or necessary to create this memory pool to solve your higher level problem.

Also, you might search around for memory pool template classes that you might use. If you multiple pools to implement, it is best if they all behave similarly. You'll have a lot more confidence in your application.

Cheers!
Oralloy

[peter peterson]

Assuming we are strictly seeking performance, would the implementation used above work as well(better?) as overloading new and delete and creating a "real" memory pool?

[Oralloy]

Peter,

What new and [icode]delete[/icode[ will get you, of course, is dynamic C++ objects on top of your memory pool, which might be very valuable for an application that needs to do rapid recovery of pooled objects. (e.g. deleting all objects from a pool is simply accomplished by declaring the pool closed - no destructors get called or anything - a technique used in embedded systems).

Your design, as it stands, does one "important" thing - it prevents the need for continuous construction and destruction of Message objects. Since I don't know how expensive that operation is, I can't comment further.

You might get a bit more optimization by using a self-managed stack of available Message objects, rather than a std::queue<> object.

Also, have you considered what your code should do, when your pool runs out of objects? Or is this a case which cannot happen? [Object use being controlled by the enclosing application, which is beyond the pool's control].

Since you're optimizing time, you get a bit of an improvement by not calling the Message c'tor and d'tor. You will get a bit more by using a self-managed stack of messages without. Is that enough?

May I ask what your real problem is?

The reason I'm asking, is because replacing the C++ memory management mechanism is an expensive optimization that generally results in low gain. Which is not to say that there aren't times when it is appropriate. I would estimate a week or of man time to write, debug, and "prove" that your code is truly working as required. Not only do you have to write your optimized memory pool (which is mostly done), but you probably should write error detection into the Message class, which detects when the pool is not used (for the maintennece guys that come after you).

Luck!