c# memory leak unmanaged resources

Question

Hi all I know this question has been asked many times and I've googled it myself as well before asking but I just cannot seem to figure it out.

I've only just started learning C# through a half-built application by someone else and ran into this memory leak issue. I ran the .NET memory profiler (software from red-gate) and narrowed it down to a particular block of code in the application.

It is a small 2D based game, each monster lives in a map which is a list of Tiles. Each monster "Think()s" every second where to move next.

Think() gets a walkpath (List of Tiles) to a target.

Here's Monster.cs

public class Monster 
{
    NavMesh NavMesh;
    public List<World.Tile> WalkPath;

    public void Think(long tick) {
        LastWalk = ctick;
        var rndMoveTile = Position.CurrentTile.Neighbours[Dice.Random(Position.CurrentTile.Neighbours.Count - 1)];
        WalkPath = FindPath(NavMesh.Tiles[Position.CurrentTile.X,Position.CurrentTile.Y], NavMesh.Tiles[rndMoveTile.X, rndMoveTile.Y]); // <- culprit called here
        DoMove();
    }
}

NavMesh.cs

public class NavMesh
{
    public World.Tile[,] Tiles;
    public NavMesh(World.Map map)
    {
        Tiles = map.CopyTiles; //<- this is causing trouble i think
    }
}

Tile.cs

public class Tile : IAStarNode<Tile>
{
    public int X;
    public int Y;
    public int WalkFlags;
    public Portal Portal;
    public bool Occupied;

    public enum E_WalkFlags
    {
        Walkable = 1,
        Projectile = 2,
        Flyable = 4,
    }

    public List<Tile> Neighbours { get; set; }
    public double Cost { get; set; }
    public Tile ParentNode { get; set; }
    public bool inopenlist { get; set; }
    public bool inclosedlist { get; set; }
    //  public long runindex { get; set; }

    public Tile()
    {
        this.Neighbours = new List<Tile>();
    }

    public Tile(int x, int y, int WalkFlags)
    {
        X = x;
        Y = y;
        this.WalkFlags = WalkFlags;
        this.Neighbours = new List<Tile>();
    }
}

And finally the culprit (I think..)

Map.cs

    public Tile[,] CopyTiles
    {
        get
        {
            var ret = new Tile[sizeX, sizeY];
            for (int xx = 0; xx < sizeX; xx++)
            {
                for (int yy = 0; yy < sizeY; yy++)
                {
                    var oldtile = Tiles[xx, yy];
                    ret[xx, yy] = new Tile(xx, yy, oldtile.WalkFlags, oldtile.Occupied);
                }
            }
            for (int xx1 = 0; xx1 < sizeX; xx1++)
            {
                for (int yy1 = 0; yy1 < sizeY; yy1++)
                {
                    var oldtile = Tiles[xx1, yy1];
                    var newtile = ret[xx1, yy1];
                    for (int x = 0; x < oldtile.Neighbours.Count; x++)
                    {
                        var oldnei = oldtile.Neighbours[x];
                        newtile.Neighbours.Add(ret[oldnei.X, oldnei.Y]);
                    }
                }
            }
        }
    }

I've read that I have to implement IDisposable to dispose unmanaged resources which I did on Tile.cs and tried to do using(Tile[,] Tiles = new Tiles[,]) but I got a compiler error saying Tile[,] doesn't have IDisposable interface, I am not sure what that [*,*] type is though.

Can anyone please suggest how I can improve memory performance on this?

Where are your unmanaged resources? `Tile.cs` is a C# object, and unless `Portal` is using some external resources, then I don't see any unmanaged code here. — ProgrammingLlama, Apr 28 '18 at 07:38
Please read [this](https://stackoverflow.com/questions/334326/what-is-managed-unmanaged-code-in-c) — ProgrammingLlama, Apr 28 '18 at 07:40
@john thanks for the link. I guess I didn't need to implement `IDisposable` on `Tile.cs`. The profiler says that `Tile` object keeps increasing in number and size though.. is there a way I can get rid of the old tiles after `CopyTiles` ? — mkim, Apr 28 '18 at 07:45
What is the purpose of CopyTiles? And I'd recommend you make CopyTiles a method, not a property. — ProgrammingLlama, Apr 28 '18 at 07:46
@john This was the code given to me, I doubted that myself as well and tried to just do `return Tiles` as `CopyTiles` only seems to be copying the existing `Tiles` object, but when I do that, `Monster` only walks a couple tiles and just stops.. — mkim, Apr 28 '18 at 07:50
Growing memory does not necessary indicate a leak. GC might just think there is no reason to collect right now, since there is a plenty of available memory for example. — Evk, Apr 28 '18 at 11:24
@Evk I'm running this application on a VM with 2GB ram and it always maxes out.. — mkim, Apr 28 '18 at 12:17

score 1 · Answer 1 · answered Apr 28 '18 at 08:47

1

Can anyone please suggest how I can improve memory performance on this?

The overall data structure could use a lot of memory for a large map, as each tile is holding its own list of neighbors.

CopyTiles is making a complete copy of all of this. The implementation is OK. (It could be better, but it's not dreadfully inefficient).

I suspect that your code (somewhere not shown in the question) is probably calling CopyTiles a lot, and that is the reason your memory is blowing out. Put a breakpoint, or a counter, in there and see how often it is called.

answered Apr 28 '18 at 08:47

Peter Aylett

750
3
8

CopyTiles get called for each Monster every second and I've got lots of Monsters in a map and lots of maps as you suspected.. How would you change it to make it perform better? – mkim Apr 28 '18 at 09:58
Also shouldn't the GC disposing all the copied tiles once the next call is invoked? – mkim Apr 28 '18 at 10:38

c# memory leak unmanaged resources

1 Answers1