Search code examples
c#garbage-collectionpictureboxwindows-forms-designerdoubly-linked-list

Doubly linked list consuming too much memory


I`m making an application that opens an image in a WindowsForm PictureBox and by clicking left arrow or right arrow, it will change to previous or next image in the same folder.

In order to change image the image faster without having to wait to load, I made the program load the images in advance but not from the entire folder, only 16 of them : the current one being displayed, 5 previous ones and 10 next ones.

The images are stored in a Doubly Linked List but for some reason the program is not discarding the images in the classes that are not being used anymore so the program keeps using more and more memory the more images i see.

How can i solve this issue ?

I tried using "System.GC.Collect()" at the end of NextImage and PreviousImage functions but it didn`t work.

This is the code of the class used to manage this :

 public class ImageController
{
    private string imageFolder = "";

    // List with the paths of every image in the folder
    private List<string> imageFiles;

    private LinkedList<Bitmap> imageLinkedList;
    private LinkedListNode<Bitmap> currentNode;

    // List of supported extensions
    private HashSet<string> supportedExtensions = new HashSet<string>() { "bmp", "gif", "exif", "jpg", "png", "tiff" };

    // Index in the imageFiles that correspond to currentImage
    private IndexObject currentIndex;

    // Index in the imageFiles that correspond to oldestLoadedImage
    private IndexObject oldestIndex;

    // Index in the imageFiles that correspond to newestLoadedImage
    private IndexObject newestIndex;

    // Limits of linked list
    private int oldCache = 5;
    private int newCache = 10;

    // Returns current image
    public Bitmap Image { get { return currentNode.Value; } }

    // Returns path of current image
    public string ImagePath { get { return imageFiles[currentIndex.Index]; } }

    public ImageController(string fileName)
    {
        imageFolder = Path.GetDirectoryName(fileName);
        imageFiles = Directory.EnumerateFiles(imageFolder).ToList();
        imageFiles.RemoveAll(t => !supportedExtensions.Contains(Path.GetExtension(t).Replace(".", "").ToLower()));

        currentIndex    = new IndexObject(imageFiles.IndexOf(fileName), imageFiles.Count);
        oldestIndex     = new IndexObject(currentIndex.Index, imageFiles.Count);
        newestIndex     = new IndexObject(currentIndex.Index, imageFiles.Count);

        imageLinkedList = new LinkedList<Bitmap>();
        LoadCache();
    }

    private void LoadCache()
    {
        currentNode = imageLinkedList.AddFirst(new Bitmap(imageFiles[currentIndex.Index]));

        // Load left side
        for (int i = 0; i < oldCache; i++)
        {
            oldestIndex.Index--;
            imageLinkedList.AddFirst(new Bitmap(imageFiles[oldestIndex.Index]));
        }

        // Load right side
        for (int i = 0; i < newCache; i++)
        {
            newestIndex.Index++;
            imageLinkedList.AddLast(new Bitmap(imageFiles[newestIndex.Index]));
        }
    }
    public Bitmap NextImage()
    {
        currentIndex.Index++;
        oldestIndex.Index++;
        newestIndex.Index++;

        // Delete oldest image
        imageLinkedList.First.Value.Dispose();
        imageLinkedList.RemoveFirst();

        // Add new image to Linked List
        imageLinkedList.AddLast(new Bitmap(imageFiles[newestIndex.Index]));

        currentNode = currentNode.Next;

        return currentNode.Value;
    }
    public Bitmap PreviousImage()
    {
        currentIndex.Index--;
        oldestIndex.Index--;
        newestIndex.Index--;

        // Delete newest image
        imageLinkedList.Last.Value.Dispose();
        imageLinkedList.RemoveLast();

        // Add new image to Linked List
        imageLinkedList.AddFirst(new Bitmap(imageFiles[oldestIndex.Index]));

        currentNode = currentNode.Previous;

        return currentNode.Value;
    }

}

public class IndexObject
{
    private int _index;
    private int _limit;
    public IndexObject(int index, int limit)
    { 
        _index = index;
        _limit = limit;
    }
    public int Index
    {
        get
        {
            if (_index >= 0)
                return _index % _limit;
            else
                return _limit + _index;
        }
        set
        {
            if (value >= 0)
                _index = value % _limit;
            else
                _index = _limit + value;
        }
    }
}

Edits : I was using my own linked list class that I created but @cup gave me the suggestion of using C# LinkedList which I was not aware of. I changed the code to use this LinkedList because it made the code look cleaner than my own linked list class. I also added @marsh-wiggle solution which was actually to call the dispose method for the Bitmap.


Solution

  • It looks like you never dispose old images. Try to call something this before loading a new image. This may reduce your memory consumption.

    DisposeLinkedImage(oldestLoadedImage);
    
    private void DisposeLinkedImage(LinkedImage linkedImage)
    {
        linkedImage.Image.Dispose();
        linkedImage.Image = null;
    }