Jump to content
  • Advertisement
Sign in to follow this  
spudmuncher

[Solved] Clean memory allocation

This topic is 3403 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hi, I have no idea what the best way to deal with memory allocation is. I'm using vc++ express. I want to be able to, CDevice * d = createDevice (); and then, d->drop (); and I want all devices to be automatically updated under a single function call run (), while (run ()) { /*u get the idea*/ } I've written my attempt at doing this, using c++ new and delete, but it's problematic, because it requires a huge tonne of code devoted to each and every object that I want to have those features. I can post it if anyone wants, but it's a bit long so I won't post it now. My method is to keep a list of pointers to all the objects of a type (type is passed using an enum) for every type. That list is updated dynamically so it can have an unlimited number of list items. Every time an object is created, the list is given the pointer and the type to add. Then the list will call the updt() member of every object in the list. There has to be a better way. A way that doesn't involve object type specific code. It would be great if I could just have a base class that deals with reference counting / dynamic memory. that way, class CDevice : public CRef will be just about sufficient. Sorry if I'm hard to understand, this is a hard topic to explain. A link to an article on this very topic would be tops! Thx [Edited by - spudmuncher on March 28, 2009 10:14:03 PM]

Share this post


Link to post
Share on other sites
Advertisement
Create a Device base-class, with a virtual Update() function (and a virtual destructor), and derive your specific devices from that class. Then, have a container of Device pointers and store all devices in it. For updating, you can iterate through the container and call the Update() functions.

As for memory management, if you want to delete a device, you can call delete on a Device pointer, and since the destructor is virtual, the destructor of the actual type will be called (for example, DerivedDevice::~DerivedDevice) - you don't need to do manual bookkeeping for that anymore.


The idea here is hat Device is a generic interface, that is the same for all device types. So, regardless of the actual type, you can all treat them the same. What they do under the hood depends on their actual type (and that's what the virtual keyword is for).

Does that solve your problem?

Share this post


Link to post
Share on other sites
Thanks a lot for your response.
I've tried to make a concept design of how it will work, based on what you said. But I'm new to these advanced C++ concepts because I haven't needed to use them until now, so please let me know if I've done something (or everything) erroneously. Until today I couldn't imagine a possible use for polymorphism (:



// generic object base class
class CRefObject
{
public:
virtual ~CRefObject ();
virtual void update ();
};

// holds a reference to the pointers of all objects (all are the same type, CRefObject)
class CPointerContainer
{
// list of pointers is dynamically allocated, so it can have unlimited items
CRefObject ** pointer; // ** because array of pointers
int nPointers;
public:
~CPointerContainer ()
{ // destroy all
for (int i; i<nPointers; ++i) delete pointer[];
}
add (CRefObject * ptr); // won't bother adding insides of this function, basically it adds the given pointer to the container
remove (CRefObject * ptr); // again, won't include insides of this function. //It searches for the pointer in the list, and then deletes that pointer, setting it to null
clean (); // cleans up the container, slides upper pointers in the list down over null pointers
updateAll ()
{ // this would be called in a main loop
for (int i; i<nPointers; ++i)
{
if (pointer != NULL) pointer->updt (); // a null pointer is null because it has been deleted
}
}
} container;


virtual void CRefObject:: update ()
{
// default update action when not specified
}
// allows derived classes to be cleaned, when the base class is destroyed
virtual CRefObject:: ~CRefObject ()
{
container.remove (this);
}


// a couple of derived devices
class CDeviceA : public CRefObject
{
public:
~CDeviceA () {};
void update () {/* do the update stuff for this device */};
};

class CDeviceB : public CRefObject
{
public:
~CDeviceB () {};
void update () {/* do the update stuff for this device */};
};


// when I want to create a device I would call this function, which returns a pointer to that device
CDeviceA * CreateDeviceA ()
{
CDeviceA * d;
d = new CDeviceA;
container.add (d);
return d;
}
CDeviceB * CreateDeviceB ()
{
CDeviceB * d;
d = new CDeviceB;
container.add (d);
return d;
}

// so in the main loop I can now do this
void run ()
{
CDeviceA * device;

device = CreateDeviceA ();
container.updateAll ();

delete device;
}




[Edited by - spudmuncher on March 15, 2009 5:37:21 AM]

Share this post


Link to post
Share on other sites
Quote:

Code: (grrr, neither tabs nor spaces seem to work here to indent stuff )

[code][/code]
or
[source][/source]


1) pure virtual is good for interfaces
class CRefObject
{
public:
virtual ~CRefObject (){};
virtual void update () = 0; // << pure virtual means you HAVE to implement this (helps prevent typos like "Update()" from working)
virtual bool dead() = 0;

void deleteMe() { delete this; }
};
class CDeviceA : public CRefObject
{
virtual CDeviceA( /* params */ ) { deviceResources->aquire(); }
virtual ~CDeviceA() { deviceResources->release(); }
virtual void update() { /* do stuff! */ }
virtual bool dead() { /*decide when we are good for removal*/ }
}


2) std::vector<> is your friend.

std::vector<CRefObject *> updateList;
updateList.push_back( new CDeviceA (/* device A params */) );
//...
while(running())
{
std::for_each( updateList.begin(), updateList.end(), std::mem_fun(&CRefObject::update) );
//...
std::vector<CRefObject *>::iterator i = std::remove_if(updateList.begin(), updateList.end(), std::mem_fun(&CRefObject::dead);
std::for_each(i, updateList.end(), std::mem_fun(&CRefObject::deleteMe);
updateList.erase( i, updateList.end() );
}

std::for_each(updateList.being(), updateList.end(), std::mem_fun(&CRefObject::deleteMe);
updateList.clear();













3) As in the source above, prefer a "is dead" and centralized removal. It allows you to avoid all the random edge cases that tend to pop up when you add/remove items during the update loop.

4) The CRefDevice should probably inherit from boost::noncopyable under the assumption it aquires some resource that needs removal on delete.

** NOTE!: there could be typos, I typed the code off the top of my head.

[Edited by - KulSeran on March 15, 2009 3:48:55 AM]

Share this post


Link to post
Share on other sites
Please excuse me for seeming noobish, but....
I didn't really understand what you said KulSeran. Could you try to relate it more specifically to what I wrote. I noticed you used a boost library command, but I'm not using boost. And how does the standard library vector have to do with what I'm doing. Do we have to use external stuff. I guess if it's really worthwhile using std::vector<>, could you please explain how it could fit in with the code I wrote. I'm also a bit confused about the way you changed my code in step 1. Sorry. Thx

Share this post


Link to post
Share on other sites
Well, for step 1, I added a "= 0" at the end of the definition of the virtual update function. This tells the compiler that CRefObject does not in fact implement update. So you can't make a CRefObject on its own, and whatever classes inherit from CRefObject are forced to implement update().

For step 2, I didn't use a single boost function. And i replaced your CPointerContainer with std::vector<>.
Your add, remove, and clean functions for that CPointerContainer all require from the looks of it
1) you allocated enough memory to start with
2) you have to iterate over all objects just to find some free space
3) you have to constantly check "is not null? ok do stuff." instead of knowing the container only holds valid pointers.

by replacing your container with the C++ standard container you get:
clarity.
1)speed (no more iterating compacting NULLs, or finding a NULL to use as free space)
2)no need to know how much memory to new/delete for your container.
3)access to the standard library algorithms and utilities.



I re-read your posts, maybe a full example of how I'd do it that you can actually run will help:

#include <vector>
#include <algorithm>
#include <functional>

//----------------------------------------------------------------------------
// Your device interface
class DeviceBase
{
public:
DeviceBase(){};
virtual ~DeviceBase(){};
virtual void update () = 0; // << pure virtual means you HAVE to implement this (helps prevent typos like "Update()" from working)
virtual bool isDead() = 0;
void deleteMe() { delete this; }
};

//----------------------------------------------------------------------------
static std::vector<DeviceBase *> updateList;
static std::vector<DeviceBase *> removeList;
//----------------------------------------------------------------------------
// Adds it to the update list
void AddForUpdate( DeviceBase *pDevice )
{
updateList.push_back( pDevice );
}
//----------------------------------------------------------------------------
// Flag it for later removal.
// Don't outright remove it, incase this is called durring update
void FlagForRemove( DeviceBase *pDevice )
{
removeList.push_back( pDevice );
}
//----------------------------------------------------------------------------
// Check if it is in the remove list
bool WasRemoved( const DeviceBase *pDevice )
{
return std::find( removeList.begin(), removeList.end(), pDevice ) != removeList.end();
}
void RemoveDeleted( )
{
std::vector<DeviceBase *>::iterator i; // Iterator
i = std::remove_if(updateList.begin(), updateList.end(), WasRemoved ); // quickly moves all items that "WereRemoved" to the end of the list
updateList.erase( i, updateList.end() ); // quickly pops all items past i
removeList.clear(); // clear our "deleted already" list
}
//----------------------------------------------------------------------------
// Clear out the list
void ClearDevices()
{
//_______________
// first kill all the stuff that was already deleted
RemoveDeleted();
//_______________
// now kill everything else
std::for_each(updateList.begin(), updateList.end(), std::mem_fun(&DeviceBase::deleteMe)); // destroy the objects
updateList.clear();
removeList.clear();
}
//----------------------------------------------------------------------------
// Update loop
void UpdateLoop( )
{
//_______________
// first kill all the deleted stuff
RemoveDeleted();
//_______________
// Check for things that died
std::vector<DeviceBase *>::iterator i; // Iterator
i = std::remove_if(updateList.begin(), updateList.end(), std::mem_fun(&DeviceBase::isDead));
std::for_each(i, updateList.end(), std::mem_fun(&DeviceBase::deleteMe));
updateList.erase( i, updateList.end() );
//_______________
// Now update
std::for_each( updateList.begin(), updateList.end(), std::mem_fun(&DeviceBase::update) );
}


//----------------------------------------------------------------------------
// Inherited device
class DeviceA : public DeviceBase
{
public:
DeviceA() : _dead( false )
{
AddForUpdate( this ); // add for update
}
virtual ~DeviceA()
{
FlagForRemove( this ); // add for remove
}
// virtual update function, so we don't have to know what type of device in the update loop
// but the right code gets called
virtual void update()
{
std::cout << "Device A Updating" << std::endl;
}
virtual bool isDead()
{
return _dead;
}
private:
bool _dead;
};


class DeviceB : public DeviceBase
{
public:
DeviceB() : _dead( false )
{
AddForUpdate( this ); // add for update
}
virtual ~DeviceB()
{
FlagForRemove( this ); // add for remove
}
// virtual update function, so we don't have to know what type of device in the update loop
// but the right code gets called
virtual void update()
{
std::cout << "Device B Updating" << std::endl;
}
virtual bool isDead()
{
return _dead;
}
private:
bool _dead;
};

//--------------------------------------------------------------
// Begin Test Func
//--------------------------------------------------------------
int main ( u32 argc, char *argv[] )
{
//----------------------------------------------------------------------------
// create some devices
DeviceBase *newDevice1 = new DeviceA;
DeviceBase *newDevice2 = new DeviceA;
DeviceBase *newDevice3 = new DeviceB;

// Test the update loop
UpdateLoop();
// Delete one
delete newDevice1;
// Test again
UpdateLoop();
// Clear out everything
ClearDevices();
//----------------------------------------------------------------------------
return 0;
}




* and all of this is just out-of-the-box C++. No boost:: or other libraries.

[Edited by - KulSeran on March 15, 2009 10:31:04 PM]

Share this post


Link to post
Share on other sites
Thanks KulSeran for explaining in more detail.
That is far more helpful (:
When I get over this annoying flu I'll have another attempt at it.

Judging by the way certain things looked slightly mysterious, I think I will have to revise my c++ skills.

I'll probably put in a using namespace std;
to make the code look cleaner too (;



A question, are

DeviceBase *newDevice1 = new DeviceA;

and
DeviceA *newDevice1 = new DeviceA;

both valid and equivalent

Share this post


Link to post
Share on other sites
Quote:
I have no idea what the best way to deal with memory allocation is.

RAII is the way to go in C++. Learn it, it's the basics.

Share this post


Link to post
Share on other sites
RAII seems very complex.
http://www.codeproject.com/KB/cpp/RAIIFactory.aspx
An OO Variant
Is the only information I've found that is even remotely applicable to my problem. I am struggling to see how to connect it to the work I've done.

So, as I understand it

I would have my generic base class for all devices
class DeviceBase;

Then I would declare the RAII factory
class DeviceFactory;

Then I would have all my little devices
class DeviceA : public DeviceBase
class DeviceB : public DeviceBase;


Also, is there a way I can manually control the factory life-time, rather than having it only be valid for a single portion of my code (i.e. my devices need to be accessible from a lot of places). I can't seem to find a way to manually remove specific items from the factory either, eg. DeviceA->Drop ();

In the code I wrote, I didn't use a constructor in the base class. And when I create a new DeviceA I would put in new DeviceA (arg1, arg2);. How is this still possible with RAII though.

in MyPolymorphicFactory

void myFun (int n)
{
MyPolymorphicFactory factory;

for (int i = 0; i < n; ++i)
{
MyBase* m = factory.create<MyBase> (n);
// ...

m = factory.create<MyDerived> (n);
// ...

}

what exactly is MyClass in RaiiFactoryImp<MyClass> imp;
Is that some sort of keyword.

Share this post


Link to post
Share on other sites
I think that article is confusing matters with their 'RAII factory', and the fact that it discourages the use of smart pointers does not speak in it's favor (smart pointers are a fine example of RAII!).

The point of RAII is not to create 'factories' and all that. It's simply to tie resource allocation/deallocation to the lifetime of objects. For your device classes, their constructors create or open the underlying device, and their destructors should ensure that the underlying device is properly disposed of. That's what RAII is about.


So it looks like you're already using RAII for your devices. But, RAII can also be applied to dynamically created objects, by using smart pointers to encapsulate them. If a pointer goes out of scope, it is destroyed, but the object it pointed to is not. A smart pointer makes sure that the object it points to does get destroyed (it does so in it's destructor). There are various types of smart pointers, too, for various kinds of behavior: boost::shared_ptr only removes the wrapped object once all copies of the shared pointer are destroyed, while std::auto_ptr simply removes it's wrapped object when it is destroyed (useful to tie a dynamically allocated object to a scope).

So, to fully automate the memory management, you could use a std::vector of boost::shared_ptr's to Devices:

std::vector< boost::shared_ptr<Device> >   devices;

devices.push_back(boost::shared_ptr<Device>(new Device(...)));

Upon removing a shared-pointer-to-Device from the vector, if that was the last shared pointer that pointed to that Device, the Device will automatically be deleted.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!