Optimizing my application

[HaLo2FrEeEk]

I've gotten past the point of functionality in my program, now I'm trying to optimize. When I started writing I was still learning about a lot of things, so I didn't quite do things the "right" way. Everything works, but I'm sure I could make it use less memory. I've been getting pretty conscious about this lately, so I've been opening up Task Manager and running my app, putting it through it's various functions to see how the memory is affected.

I've noticed that when I use images and open files, even after I close the filestream and binaryreader accessing the file, it still doesn't release the memory resources.

I have a program that opens a sort of archive file, a file containing a theme for the Xbox 360. The program reads all the information about the theme and lets the user preview individual files, save those individual files, and dump all the files. The theme file is only 12.5MB. My program's idle memory usage is just under 11MB. When I click the "Open" button to show the openFileDialog, the memory usage jumps to 21MB. When I actually open the theme file, it jumps up to just under 50MB of memory usage. Why is it going up 30MB just to open a 12.5MB file? There are various pieces of information I retrieve, and I loop through a 4096 byte block 64 times (reading 64 bytes at a time) to get the information about the individual files. I just create a byte array and pass it to a struct, then print the resulting information to a listView.

I'm sure I've disposed of all the things I can, but disposing my objects doesn't seem to release the resources. I have a "Close File" button that disposes and closes all the various files and readers and images I'm using, but the memory usage stays the same. Why is this?

Is there a simple way to optimize my program, and if there's no simple way, is there a way I can print some soft of debug info to the program telling me how much memory it's using and for what resources?

[tlhintoq]

'simple' and 'windows memory' never go together.

The theme file is only 12.5MB. My program's idle memory usage is just under 11MB. When I click the "Open" button to show the openFileDialog, the memory usage jumps to 21MB. When I actually open the theme file, it jumps up to just under 50MB of memory usage. Why is it going up 30MB just to open a 12.5MB file?

Maybe you are making multiple copies of your images without realizing it. You have an image in a variable, and a copy of it in a picturebox.imag e property... and you make a copy of it as part of a thumbnail generator...

I just create a byte array and pass it to a struct, then print the resulting information to a listView.

you have an image in a byte[] *and* in a struct *and* in a listview... there's three times the usage right there.

[HaLo2FrEeEk]

I didn't realize setting the picture in the picturebox made a copy of it, is there a way around that?

And as for the byte[], struct, listview, I'm passing 64 bytes of data, non image, to the struct for parsing, at most 64 times (64x64 byte blocks in a 4096 byte block.) I set the byte[] in the loop everytime, so it should only ever have 64 bytes of data in it, and if anything it would only be reading 4096 bytes. I just don't get why when I close the file and dispose the filestream, it doesn't release the resources. I even tried calling GC.Collect(), nothing.

[tlhintoq]

Then it is still within scope someplace.

[HaLo2FrEeEk]

I'd like to have someone look at it for me, but I don't know who I can trust with my source, I worked really hard on this and I don't want to have it ripped. I've done everything I can, I have a close method that closes and disposes of all the filestreams and binaryreaders and images that I use, or so I think. How can I find out what's still being used AFTER clicking the close file button, is that possible?

[tlhintoq]

MSDN magazine advertises a couple tools specifically for memory tracking purposes including object uses.

[HaLo2FrEeEk]

I know that I'm closing and disposing of all my resources, at least I'm pretty sure I am, I just don't understand this! I've looked through everything I can and I either end up disposing of something I need to stay open, or it doesn't seem to do anything at all. You'd think disposing of the filestream and binaryreader acessing the file would at least release SOME of the memory, but it doesn't release anything at all.

For example, I know there has to be better ways to do these things. There's an embedded 64x64 PNG image in the file I'm opening. At offset 0x1712 is an Int32 that represents the size in bytes of the PNG, then the PNG starts at 0x171A. Here's the method I'm using to get the information and extract the PNG:

private MemoryStream getGameTile()
{
    br.BaseStream.Position = 0x1712; // Set the position of the binaryreader
    byte[] gameTileSizeBytes = br.ReadBytes(4); // Read the 4 bytes that represent the PNG size
    Array.Reverse(gameTileSizeBytes); // Reverse the array since it uses Big Endian byte order
    br.BaseStream.Position = 0x171A; // Set the position of the binaryreader
    int gameTileSize = BitConverter.ToInt32(gameTileSizeBytes, 0); // convert the array to get the integer value for the PNG size
    byte[] gameTileBytes = br.ReadBytes(gameTileSize); // Read "gameTileSize" bytes, the PNG image
    MemoryStream gameTileMem = new MemoryStream(gameTileBytes); // Set the memoryStream
    return gameTileMem; // And return it.
}

There has to be a simpler way to do that without making all those intermediate variables. And that's just ONE example.

[GaryTexmo]

Sorry to jump in here so late, a thought just occurred to me.

Are you expecting to see your memory free up as soon as a particular component of your program that was using it is closed? An example of what I mean by this is say you have a window that appears and shows a bunch of images. When you close that window (and the underlying code has those images go out of scope), are you expecting to see the memory deallocate?

If this is the case, simply put, you won't. The garbage collector is a picky thing... I like to say that it does it's job on it's own terms, not anybody else's! :P It will free that memory when it is needed or if it feels like it would be a good time to do so, otherwise it just hangs out. For the most part, I wouldn't worry about it. If your program or another process comes along and wants to allocate that memory, the garbage collector should release it and all is well.

However, if you're running on a limited memory environment or you otherwise absolutely require that memory to be released, you can tweak the garbage collector settings (or call garbage collection manually, which is generally not recommended). Search MSDN for the "GC" class.

My advice to you though would be to follow T's suggestion and look for wasted allocation if you can. It's always good to make sure you don't get into any bad habits :D That said, instead of worrying about how much memory your program has allocated at any given time, look for a trend. Does your program's memory footprint constantly grow? If so, that might indicate a memory leak, which I suppose in C# terms means you're always allocating, never letting variables go out of scope even when you intend to. A case of that might be adding an item to a local list and a global list. If the local list goes out of scope, the object is still allocated.

</brain_dump>

Good luck!

[HaLo2FrEeEk]

Well I'm communicatiing with tlhintoq via email and he told me that generally global variables should be avoided. The thing is, I don't know how to access a variable in multiple methods if it's not global. I have a lot of global variables because I have a lot of information that needs to be shared between methods. I thought that constantly passing the variables as parameterswould get a little tedious, and also make multiple instances of the variable.

And yes, I do notice a trend, when I open the program it uses 10MB of memory, when I click the button to open the openfiledialog is jumps to 20MB (I haven't opened any files yet though) and when I actually open the file it jumps to 50MB. Even if I click close, or just click the openfiledialog again to open a different file, it adds 15 to 20MB on top of the 50, and that seems to grow forever.

I have a few intermediary variables whose value is only temporary, since the variable I need the data to be in doesn't allow for a certain thing or something. Like I need to read data from a stream and into a byte array in a loop, but since (as far as I know) byte arrays can't be successively written to, I use a list<byte> and at the end of the loop, I do List.ToArray(), but then I have this List and a byte[] with the same data. I've tried List.Clear() but it doesn't seem to actually work for releasing the resources.

That's just one example, there are more times that I do things like that, like reading 4 bytes to a byte array so I can get a Big Endian Int32, since C# only reads in Little Endian (and not all my values are Big Endian, some are Little.)

[GaryTexmo]

Yea, the globals may get you since they'll hold the memory until it's cleared. Now you mention you are doign a List.Clear(), which is correct, but that brings me back to what I mentioned before. You won't actually see the memory go away when the clear is called... it hangs out until the garbage collector decides it should go away.

Here's a little demo program I made a while back, play around with this. When I do it and click the Dallocate (shush, typo :P) button the footprint in taskmanager doesn't actually change much. Set the point count to something high to see better results. If you deallocate with the GC box checked, things clean themselves up right away.

[HaLo2FrEeEk]

Thanks, I'll check out that app. However, at the end of my close file method, I tried calling GC.Collect() and I think now I'm using GC.GetTotalMemo ry(true). Neither really seemed to help, I'm assuming that's because of the global variables. I also tried waiting, about an hour, the program's memory usage never changed.

So I guess the question is how can I take a global variable out of scope, if it's even possible, and how can I avoid global variables and still have universal access to the information from multiple methods.

[GaryTexmo]

You can't take them out of scope, that's when you need to clear your lists, which you sound like you were doing. T's suggestion of using some memory tracking tools might be in order here :)

As for removing things from the global scope, it depends. While globals are kind of taboo, I've felt they've got a time and a place. Still, if you want to remove them, try putting them somewhere that they make sense. You sound like you're making a game... instead of making them a global, make them a private member of the main game class (whatever runs the game loop). Maybe make your game objects know how to draw and update themselves so that you don't have to pass object references all over, just a reference to whatever they need to draw.

Most of my recent game experience is in XNA, so I typically do something like this...

public class MainGame : Game
{
  private List<GameObject> m_gameObjects;
  private List<GameObject> m_objectsInView;

  ...

  protected override void Update(GameTime gameTime)
  {
    for (int i = 0; i < m_gameObjects.Count; i++)
    {
      m_gameObjects[i].Update(gameTime, <other parameters for updating>);
    }
  }

  protected override void Draw(GameTime gameTime)
  {
    m_objectsInView.Clear();
    m_objectsInView.Add(GetObjectsInView(m_gameObjects, <view bounds>));
    for (int i = 0; i < m_objectsInView.Count; i++)
    {
      m_objectsInView.Draw(gameTime, <other parameters for drawing>);
    }
  }
}

I don't know if you're using XNA and I know your problem isn't the game loop, but maybe that gives you some ideas? In this case my object list is local to my private class, so all the game knows about is the objects. Hopefully that gives you some ideas, I dunno.

If you've got unrestricted memory growth, look at methods where that memory is allocated and put some debugging info (output or breakpoints) in there and see when it's called and by where. This should help you determine whether or not you're making needless calls to that method.

Oh, another tip... watch your memory allocation in your draw and update loops. It's a bad thing to allocate new memory in these methods as they run quite often. For example, in the above notice I have my draw list as a member variable instead of a local variable to the method because instantiating a new object here takes processing power away from the draw loop. All I add to that list is references. It will still allocate some memory when I add to the list, but at least I save on instantiating a new object every loop, and in my experience every little bit counts.

Again, hopefully that gives you some more ideas...

[HaLo2FrEeEk]

Well I read your post, but unfortunately I'm not making a game. You sound like you know about the Xbox 360, having used XNA, so you'll probably know what premium themes are? If not, it's basically just a theme for your dashboard. Wallpapers, styles, and little pictures that sit behind the people in your friends list. If you DO know what they are, then...well, I'll continue.

I've been learning all about the Xbox 360's file structure and figuring out how they store stuff in their proprietary file format, which is sort of like an archive, but not compressed. At offset 0xC000 there is a list of all the individual files packed inside the archive, sort of like an index. Each list item is 64 bytes long and has this structure:

public struct fileEntry
{
    public string fileName; // 40 bytes
    public byte fileNameLength; // The length of the filename
    public short fileSizeBlocks; // The number of 4096-byte blocks the file takes up (Little Endian)
    public byte nullByte1; // Not used
    public short fileSizeBlocksCopy; // Exact copy of the first fileSizeBlocks
    public byte nullByte2; // Not used
    public short fileStart; // The 4096-byte block where the file starts (Little Endian)
    public byte nullByte3; // Not used
    public short pathIndicator; // Tells if the file should be in a folder, FF FF means it's just a file
    public int fileSizeBytes; // the length of the file in bytes
    public int updateTime; // Update DOS date/time of the file
    public int accessTime; // Access DOS date/time of the file
}

The block count starts at 0xC000 as well, so if a file is located at block 1, then it's really at 0xD000. What my program does is load in a theme file and read that file list, calculate the total content size, number of files, and offset of each file, then write that information to a listView. It's not that simple, though. The Xbox 360 format (it's called the CON format, by the way) has security in the form of hash blocks. It took me a while to understand how they worked, but I have it now.

There are master hashes, which take up 2 blocks, and sub hashes, which only take up 1. These hashes do not count as a block in the file list, so they need to be removed so that my offset points to the right place and so that I'm not reading the hash block into the file I'm trying to get at.

Starting at offset 0xC000, there is a master hash 170 4096-byte blocks ahead, so the first master hash is at 0xB6000 (or 0xC000 + (170 * 0x1000)). That's 2 blocks, or 8192 bytes, and needs to be removed. After that master hash there are another 170 blocks of data and the 171st block is a sub hash. Then another 170 data blocks, then a sub hash block. If the file is long enough you can have more than one master hash, but theme files are never long enough (the file would have to be at least 119,115,777 bytes, or 113.6MB to include another master hash table (and the second master hash table would include a certificate extension as well as the master hash, so it would be 3 blocks long, taking up 0x719A000, 0x719B000, and 0x719C000.))

The point is, I don't need those hash tables, just the data. So, knowing that there would only be one master hash, I wrote this quick loop:

private void removeHash()
{
    hashlessList = new List<byte>();
    br.BaseStream.Position = 0xC000;
    bool masterHash = false;
    int subHash = 0;
    while (br.BaseStream.Position < br.BaseStream.Length)
    {
        if (subHash == 170)
        {
            br.BaseStream.Position += 4096;
            if (!masterHash)
            {
                br.BaseStream.Position += 4096;
                masterHash = true;
            }
            subHash = 0;
        }
        else
        {
            hashlessList.AddRange(br.ReadBytes(4096));
            subHash++;
        }
    }
    hashlessFile = hashlessList.ToArray();
    hashlessList.Clear();
    hashlessList = null; // Releasing the resource
}

Basically, I loop and increase the subHash counter until it equals 170, at which point I know I've hit a hash block. so I skip 4096 bytes. I check if masterHash is set to false, which means I haven't encountered the master hash yet. If it is false, I skip another 4096 bytes and set masterHash to true, so it won't happen again. I then set subHash to 0. If subHash isn't equal to 170 (so I'm at a data block) I copy the data into my List, hashlessList, using AddRange() and BinaryReader.Re adBytes(), then I increase the subHash counter. At the end of the loop I copy the entire contents of my hashlessList to a byte array, clear the List, then set the List to null.

Before I started setting the List to null I noticed that even though I cleared it, it still wouldn't release the memory. Once I started setting it to null also, it would release the memory and I noticed quite a drop when monitoring it in the Task Manager.

Sorry, I know this is a really long post and I hope you'll read it all. I know that this is probably a bad way to be skipping those hash blocks, but I can't figure out a way to note where they would be on the fly...unless, maybe if I calculated where they'd be based on the length of the file and stored THAT information in a list, then as I extracted out the files I could simply run the loop, skipping the hash blocks, without having to copy the entire hashless file into a byte array. The problem with that is I'd also have to recalculate the starting position of each file based on how many hash blocks there are before that file starts, since hash blocks don't count as data blocks. So basically if there's a data block at 0, a hash block at 1, and a data block at 2, and you need the second data block, as far as the filesystem is concerned, the second block is block 1, the hash block is skipped.

[GaryTexmo]

Sorry, I guess I was way off base :D

I don't think there's anything fundamentally wrong with what you're doing. If you didn't want to do it this way you could use an offset mapping mechanism to store where the data is for each file (sorry if I get some terminology incorrect, I won't understand this nearly as well as you do :P). Then when you're processing a file you could use your map to go straight to the index.

Still though, what you have should be ok, but what are you doing with the hashless file and the original file once you're finished? How is the data for those released, or are they released at all? These questions are rhetorical, just giving you thinks to think about when you look for memory usage.

What are you doing with these theme files anyway? Just curious :)

[HaLo2FrEeEk]

The hashlessList is cleared, then set to null. The hashlessFile (the byte array) is set to null in my closeFile method. The filestream and binaryreader for the original file are both closed right after the removeHash loop completes. Closing a filestream and binaryreader don't seem to do anything though...