Sign in to follow this  

Memory allocation/freeing problem. (Particles,)

This topic is 3467 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

Well it's not particles exactly but bullets. Pretty similar. At any point I have hundreds of them flying around all over the place. This is how I currently manage them: 1. At creation, the constructor pushes the (this) pointer it in a vector of bullet pointers 2. The following code is what puts the objects in the scene:
	bullet_iter = bulletQ.begin();
	if ( bulletQ.size() > 0 )
	{
		for ( int i=0; i<bulletQ.size(); i++)
		{
		if(bullet_iter[i]->active)
			bullet_iter[i]->render();
		else
		{
			bulletQ.erase( &bullet_iter[i] );
		}
		}
	}
I thought the bulletQ.erase( &bullet_iter[i] ) would take the object out of scope and the destructor would be called thus freeing the memory. Apparently it's not the case. I've seen the memory it occupies shoot up to ~180 megs from the initial 11 in about 3mins. I tried simply freeing the memory by 'free( bullet_iter[i] )' but it throws errors. What's the proper way to free the memory - or even better, reuse it - without having to use a vector of static length? [BTW, the active member of the class goes to 0 when it hits something or expires.]

Share this post


Link to post
Share on other sites
I would use something like this

std::vector<Particle> m_ParticleList(MAX_PARTICLES);

When a particle dies, swap it with the last element and then pop that element. The vector will always remain the same size (MAX_PARTICLES).. don't ever call push_back on it, just use the space that you gave yourself and it'll never resize (call resize(0) if you need to "clear" it).

Manage the end of the vector yourself with a number of elements used counter.

Also, your active state and the "render" function should probably not be coupled. Anything in the list is considered "active" therefore you can render the list all at once, in some more efficient manner.

Share this post


Link to post
Share on other sites
erase needs an iterator, you're passing it a pointer. Also, you're skipping elements every time you delete one (since you increment the index). Also, erase does not remove the element from scope, it removes it from the vector (which, if the vector contains pointers, will not delete the pointed-at element).

Correct practice:
  • You store elements in the vector instead of pointers.
  • You iterate once to remove dead elements.
  • You iterate once to render.


Then, the container will take care of all memory issues for you.

Typical example of remove-render function:


class bullet
{
public:
void render();
bool inactive() const;
};

void remove_and_render(std::vector<bullet> &bullets)
{
bullets.erase( std::remove_if( bullets.begin(), bullets.end(),
std::mem_fun_ref( &bullet::inactive ) ),
bullets.end());

std::for_each( bullets.begin(), bullets.end(),
std::mem_fun_ref( &bullet::render ) );
}


Share this post


Link to post
Share on other sites
Hey thanks, that was quick!

Sorry, I was just going over the methods carefully.

For the 1st method by bzroom:

I understand what you're saying and I think I know how I can implement it properly. Is this viable that I use a stack of empty pointers (in the vector) and in the constructor of the object simply pop that pointer and assign "this" to it (i.e. if (emp_stack.size()) emp_stack.front()=this;)? Can I do that?

For the 2nd method by ToohrVyk:

This is something like what I initially went for but I didn't know much about std::remove_if. The reason I'm using pointers instead of objects is because if I push back (*this) it makes a copy and gets messy with garbage pointers and the rest. Nothing draws. Is using pointers inherently bad for this purpose? Either way, I'll try to implement either methods now.

Thanks :)

Share this post


Link to post
Share on other sites
Quote:
Original post by glopen
This is something like what I initially went for but I didn't know much about std::remove_if. The reason I'm using pointers instead of objects is because if I push back (*this) it makes a copy and gets messy with garbage pointers and the rest. Nothing draws. Is using pointers inherently bad for this purpose? Either way, I'll try to implement either methods now.


Don't. Use. Pointers. If you don't use pointers, you won't get any pointer-related mess. It's that simple. Just push_back a local instance and let it destroy itself when it goes out of scope. Its copy will remain in the vector.

The ultimate point of storing instances instead of pointers is precisely that the vector will now manage your memory for you completely. This will save you hours of work.

Share this post


Link to post
Share on other sites
Yeah, I'd much rather use the instance itself (or its copy) but the thing is that some objects have pointers and arrays in them which become garbage once a copy is made, which means I'll have to define a copy constructor and do all the loading all over again.

I actually used instances initially but had to switch to pointers because all the info loaded into arrays would disappear and nothing would draw. I guess I'll have to define a copy constructor (if there's no better way). I wish there was some way of using references within the constructor (pushing itself), if that makes any sense.. Thanks again.

Share this post


Link to post
Share on other sites
Quote:
Original post by glopen
Yeah, I'd much rather use the instance itself (or its copy) but the thing is that some objects have pointers and arrays in them which become garbage once a copy is made, which means I'll have to define a copy constructor and do all the loading all over again.


The bad news is, for your C++ objects to behave in a sane way, you will need to define the copy constructor, assignment operator and destructor (this is known as the rule of three).

The good news is, your C++ compiler will provide default copy constructors, assignment operators and destructors which will work for most objects—as long as the members of those objects work correctly.

The only thing that usually causes problems with the default functions is the use of pointers. But, as I've mentioned earlier, you shouldn't be using naked pointers anyway, so that's a moot point.

Arrays, however, should never cause any problem: the default copy constructor copies them around correctly.

Quote:
I actually used instances initially but had to switch to pointers because all the info loaded into arrays would disappear and nothing would draw. I guess I'll have to define a copy constructor (if there's no better way). I wish there was some way of using references within the constructor (pushing itself), if that makes any sense.. Thanks again.


Switching to pointers means less work in the short term, but more work on the long term as you will have to continuously reinvent pointer-friendly functionality already present in the standard library. Defining a copy constructor is the better way.

Now, of course, there's the question of big, fat objects. Those that require a large loading time and will seldom be copied around, deallocated and reconstructed. These are nothing like bullets, however. They are, indeed, better manipulated by reference (so you don't incur copies everywhere), but those references should be wrappers that implement correct copy-assign-destruct semantics. One example is boost::shared_ptr, which is a garbage-collected reference-counted smart pointer. Another example is boost::ptr_vector, which is a vector that manipulates its contents by pointer (and therefore never requires copies) but also manages the pointed-to memory.

But bullets should not be big, fat objects. If they are, apply the Flyweight design pattern.

Share this post


Link to post
Share on other sites
I was just checking out everything that you mentioned and wow! There's a lot I should know more about (:|

Quote:
Switching to pointers means less work in the short term, but more work on the long term as you will have to continuously reinvent pointer-friendly functionality already present in the standard library. Defining a copy constructor is the better way.


My bullets are fairly basic objects, though inherited from a much bigger base. These won't require any copy constructor. Can't say the same about the bigger objects because for whatever reason (probably bad implementation) the arrays are becoming empty.

I don't really know much about the Boost libraries or know how to use them. Just never had to go any further than STL. Basically I'd gladly give up pointers if I just get to use the '.' operator more often :)

Share this post


Link to post
Share on other sites
(I'm still learning C++ so sorry if this is a stupid question)

To all those suggesting "just keep the objects in the list, rather than pointers", how do you do this without totally removing all polymorphism?! Or should you keep a list for each different concrete type of particle? Either option seems really stupid..

Share this post


Link to post
Share on other sites
Quote:
Original post by raigan
To all those suggesting "just keep the objects in the list, rather than pointers", how do you do this without totally removing all polymorphism?! Or should you keep a list for each different concrete type of particle? Either option seems really stupid..

Well, in the cases where you store the object you're typically dealing with the most-specific type anyway that you create. You should never use this to store copies of objects passed to you by pointers, e.g.,

BaseObject* base = CreateSomeSpecificObject();
m_objects.push_back(*base);

Of course, you can still take the address of the objects in the vector and pass those around as pointers to base classes and what not. Just keep in mind adding or removing elements from the vector might cause a reallocation and thus invalidate all pointers handed out so far.

Share this post


Link to post
Share on other sites

This topic is 3467 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.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this