Take the 2-minute tour ×
Game Development Stack Exchange is a question and answer site for professional and independent game developers. It's 100% free, no registration required.

I'm making a top-down shoot-em-up in XNA, and I have placeholder blocks for my characters and bullets. Here's a picture:

image1

I have all the Bullets stored in a List<Bullet>, such that a Bullet is deleted whenever it collides with the window bounds. So I have a List<Bullet> called bullets, and I call bullets.RemoveAt(i) to delete a Bullet object.

However, if I keep on holding down Space to shoot Bullets, my "Mango Warfare" application starts "bleeding bullets," or bleeding memory. The app starts to climb up the memory rankings in Windows Task Manager:

image 2

Here's my code for removing out-of-window-bounds bullets:

private void CheckBulletCollisions()
{
    foreach (Bullet bullet in bullets)
    {
        if (bullet.X > GraphicsDevice.Viewport.Width ||
            bullet.X < -5 || 
            bullet.Y > GraphicsDevice.Viewport.Height ||
            bullet.Y < -10)
        {
            bullet.Enabled = false;
            continue;
        }
    }
}

private void RemoveDisabledBullets()
{
    int num_bullets = bullets.Count;
    int count = 0;
    int i = 0;
    while (count < num_bullets)
    {
        if (!bullets[i].Enabled)
        {
            bullets.RemoveAt(i);
            ++count;
            continue;
        }
        ++count;
        ++i;
    }
}

Clearly, RemoveAt() is not working. Is there anything else I can do to free up memory for out-of-window-bounds bullets?

share|improve this question
    
For which platform are you developing? PC, XBox, WP7? –  Hackworth May 23 '12 at 19:57
    
Just PC at the moment. –  Jason May 23 '12 at 19:59
1  
You're looking at the private working set of the process in the task manager there; it's not a very reliable metric for diagnosing memory issues like this. RemoveAt() works fine, and presuming there are no other outstanding references to your bullet objects (whether or not this is true isn't something that can be determined from just this code, unfortunately) they will eventually be reclaimed by the GC. –  Josh Petrie May 23 '12 at 21:24
    
As far addressing a solution, there are a number of ways you can have better control over the memory consumption of your bullets, but some of them depend on information you haven't provided. Do you have a fixed upper limit to the number of bullets you'll want on the screen at any one time? –  Josh Petrie May 23 '12 at 21:27
add comment

6 Answers

up vote 11 down vote accepted

Don't use the task manager to do memory profiling. It doesn't mean what you think it means, especially for C# or other managed applications. ANTS Memory Profiler is a wonderful managed memory profiler that I've used repeatedly. It is not free, but it does have a trial. As Jonathan Dickinson points out in the comments, the CLR itself also exposes several performance counters you can view in perfmon.exe, including several related to .NET memory and garbage collection.

There is a very good chance there's nothing actually rooting your bullet objects once they are removed from that list and that, upon a subsequent collection of the appropriate garbage generation, the memory consumed by the objects will be reclaimed. Without seeing more of your code I can't be sure, but I think it's a safe bet.

However, you are doing something that is, if nothing else, inefficient and you could improve it:

You're removing items from a list, which implies you are allocating them and putting them in the list in the first place -- in other words you are allowing allocation to occur during your game loop, and this means you'll eventually cause garbage collection (and more to the point, possibly cause more frequent collection of higher GC generations). Further, you're removing items from the middle of the list (most of the time), which means every time you remove something, the following elements of the list must be copied.

Consider instead partitioning your list into two halves, an "active" and a "disabled" half. You will need an additional integer variable somewhere to track the index of the first disabled bullet in the list.

When your game starts, you preallocate some reasonable number of bullets in the list and set the "first disabled" index to 0 (because all bullets in the list are currently disabled). Whenever you need to "create" a new bullet you just set the properties of bullets[firstDisabled] to whatever values the new bullet should have:

bullets[firstDisabled].X = desiredX;
bullets[firstDisabled].Y = desiredY;
bullets[firstDisabled].Speed = 100;

// The bullet is now active:
++firstDisabled;

In your main update loop, you only process bullets from 0 to firstDisabled:

for (var bulletIndex = 0; bulletIndex < firstDisabled; ++bulletIndex) {
  // Update the bullet here.
}

To handle the case where a bullet has left the screen during the update loop, you don't remove it from the list. Instead you swap that bullet's data with the last enabled bullet's data (which is at firstDisabled - 1 and decrement firstDisabled.

Obviously this technique implies two things: you have a fixed upper limit of live bullets and that the order of bullets in the list doesn't matter. The latter should not be a problem, however the former might be. Fortunately you can deal with this if you have to by adding a new bullet at the end of the list (not in the middle) and swapping that bullet's properties with the bullet at index firstDisabled. Then you increment firstDisabled -- much like how you handle "removing" a disabled bullet, really.

The end result is that you have a system that allows bullets to be activated and deactivated in linear time (your original method was more intensive) and has a fixed memory budget which can't leak or induce garbage collection during gameplay and is easily cleaned up post-gameplay by simply clearing the entire list (if need be).

share|improve this answer
2  
Also known as object pooling, good answer. +1 –  Danik May 23 '12 at 21:55
    
I changed the List<Bullet> to a 10-bullet array after reading some of the other answers, but I was still allocating objects and assigning these objects to array locations during the game loop. I'll have to give your advice a try, thanks! –  Jason May 23 '12 at 22:13
1  
+1. You might also want to look into adding perf counters into your answer: the .Net ones can give you an estimate of the memory usage (without the speed impact of profiling). –  Jonathan Dickinson May 24 '12 at 8:35
add comment

2 Things. First, it could be some kind of garbage collection issue. Just because you are removing the Bullet from the List doesn't mean it or some part of it still doesn't exist in memory. I don't know enough about the specifics or how Bullet is implemented, but it could be the Bullet object is creating something (A Texture2d?) and not properly destroying it?

2nd, For the loop in your second code, you have two variables (count and i) that grow exactly the same. Just pointing that out. See Hackerworth's method in the other answer.

Hope this is helpful! -Woody

share|improve this answer
    
It seems you're right Woody, the bug smells the memory leak due to not deleted bullets after being removed from the list. –  lvictorino May 23 '12 at 20:18
    
If OP comes back and shows some more of his Bullet code, we might be able to help better. I'm no C# expert. –  Woody Zantzinger May 23 '12 at 20:27
2  
Your for() loop is bugged - after it removes a bullet, it will not check the new bullets[count], which is the old bullets[count+1], so in the worst case, if all bullets are disabled, it will remove only half of them. –  Hackworth May 23 '12 at 20:29
    
I think your right. I'll edit it. Thanks. –  Woody Zantzinger May 23 '12 at 20:32
1  
The loop should work I think since i is not incremented when a bullet was removed, but it's definately a contrieved way of doing it. It would be much better to iterate through the list from the end. –  Danik May 23 '12 at 21:14
show 2 more comments

First, you don't need 2 indices i and count, one will suffice.

For clarity, I would rewrite the method:

private void RemoveDisabledBullets()
{
    int i = bullets.Count - 1;
    while (i >= 0)
    {
        if (!bullets[i].Enabled)
        {
            bullets.RemoveAt(i);
        }
        --i;
    }
}

Second, I have no reason to believe that RemoveAt(i) somehow doesn't work, it's a very basic method. Maybe there's a bug in your while loop, it's certainly convoluted enough to allow for that possibility, no offense, but if you call RemoveAt(i) and your program does not throw an exception, then the element at i has indeed been removed from that list.

If you are absolutely positive that the bullets are causing your memory leak, then the reason must be that your bullets are still being referenced somewhere else and are not getting garbage-collected.

Can you confirm that bullets is really the only active object that references bullets?

I haven't personally tried it, but have a look here to log whether or not the garbage collector has actually run.

share|improve this answer
    
A for loop would be even more clear. –  Amplify91 May 23 '12 at 21:32
add comment

Is continue even necessary? Not seeing how the program would get caught in that one line of the if statement.

I'm guessing RemoveDisabledBullet() is called in your Update method after the CheckBulletCollisions() method

maybe try a for loop. It's worked for me in the past.

for (int x = bullets.Count - 1; x >= 0; x--)
{
   if (!bullets[x].Enabled)
   {
      bullets.RemoveAt(x);
   }   
}
share|improve this answer
add comment

I know this is like the third or so answer with this suggestion, but I would rewrite your for loop like this:

private void RemoveDisabledBullets()
{
    for (int i = 0; i < bullets.Count;)
    {
        if (!bullets[i].Enabled)
        {
            bullets.RemoveAt(i);
        }
        else
        {
            i++;
        }
    }
}

This makes sure that count gets evaluated for every step and that every bullet is actually checked to see if it is disabled. It's how I do it for my own bullets. :)

share|improve this answer
add comment

Is it possible that even tho removeAt() is removing the reference to the object that the operating system is not reporting it as free (and may not report it as free until it decides to claim it for another activity)?

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.