Simple C# application eating memory

Question

Alright so basicly I have this simple application running in system tray that has one timer. Every tick it performs a check to see if a given directory and file exists, and based on the result it changes its icon.

The problem is every single timer tick the memory for the application raises ~100kb. I currently have it running for about 5 mins and it already uses 40MB of memory, which is unacceptable for such "micro" application.

Here's my code:

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.IO;
using System.Reflection;
using System.Windows.Forms;

namespace Tray
{
    public partial class Main : Form
    {
        string drive = "C:\\";
        string file  = "test.txt";

        System.Drawing.Image imgRed     = Image.FromFile("res\\icon-red.png");
        System.Drawing.Image imgOrange  = Image.FromFile("res\\icon-orange.png");
        System.Drawing.Image imgGreen   = Image.FromFile("res\\icon-green.png");
        System.Drawing.Icon  icoRed      = System.Drawing.Icon.ExtractAssociatedIcon("res\\icon-red.ico");
        System.Drawing.Icon  icoOrange   = System.Drawing.Icon.ExtractAssociatedIcon("res\\icon-orange.ico");
        System.Drawing.Icon  icoGreen    = System.Drawing.Icon.ExtractAssociatedIcon("res\\icon-green.ico");

        public Main()
        {
            InitializeComponent();
        }

        public static string ShowPrompt(string text, string caption)
        {
            Form prompt = new Form();
            prompt.Width = 500;
            prompt.Height = 150;
            prompt.Text = caption;
            Label textLabel = new Label() { Left = 50, Top = 20, Text = text };
            TextBox textBox = new TextBox() { Left = 50, Top = 50, Width = 400 };
            Button confirmation = new Button() { Text = "Ok", Left = 350, Width = 100, Top = 70 };
            confirmation.Click += (sender, e) => { prompt.Close(); };
            prompt.Controls.Add(confirmation);
            prompt.Controls.Add(textLabel);
            prompt.Controls.Add(textBox);
            prompt.ShowDialog();
            return textBox.Text;
        }

        public void updateInfo(){
            this.statusDrive.Text = "Drive [" + drive + "]";
            this.statusFile.Text = "File [" + drive + file + "]";
        }

        public void exec(){
            int status = 0;

            this.trayIcon.Text = "[Drive - ";
            if (Directory.Exists(drive)){
                this.statusDrive.Text += " - OK";
                this.statusDrive.Image = imgGreen;
                status++;
                this.trayIcon.Text += "OK] ";
            } else{
                this.statusDrive.Text += " - FAIL";
                this.statusDrive.Image = imgRed;
                this.trayIcon.Text += "FAIL] ";
            }

            this.trayIcon.Text += "[File - ";
            if (File.Exists(drive + file))
            {
                this.statusFile.Text += " - OK";
                this.statusFile.Image = imgGreen;
                status++;
                this.trayIcon.Text += "OK] ";
            }
            else
            {
                this.statusFile.Text += " - FAIL";
                this.statusFile.Image = imgRed;
                this.trayIcon.Text += "FAIL] ";
            }

            switch (status)
            {
                case 2:
                    this.Icon = icoGreen;
                    this.trayIcon.Icon = icoGreen;
                    this.status.Image = imgGreen;
                    break;
                case 1:
                    this.Icon = icoOrange;
                    this.trayIcon.Icon = icoOrange;
                    this.status.Image = imgOrange;
                    break;
                case 0:
                default:
                    this.Icon = icoRed;
                    this.trayIcon.Icon = icoRed;
                    this.status.Image = imgRed;
                    break;
            }
        }

        private void Form1_Load(object sender, EventArgs e)
        {
            this.Hide();

        }

        private void timer1_Tick(object sender, EventArgs e)
        {
            updateInfo();
            exec();
        }

        private void chDrive_Click(object sender, EventArgs e)
        {
            this.drive = ShowPrompt("Enter drive path", "Change drive");
        }

        private void chFile_Click(object sender, EventArgs e)
        {
            this.file = ShowPrompt("Enter new file path:", "Change file");
        }

        private void exitToolStripMenuItem_Click(object sender, EventArgs e)
        {
            Application.Exit();
        }
    }
}

I already tried to optimize the app by preloading the icons and images into variables and assigning those to the appropriate properties, however this didn't solve my problem.

Also, note that I managed to hide my main window by doing this (in Program.cs):

using System;
using System.Collections.Generic;
using System.Linq;
using System.Windows.Forms;

namespace Tray
{
    static class Program
    {
        /// <summary>
        /// The main entry point for the application.
        /// </summary>
        [STAThread]
        static void Main()
        {
            Application.EnableVisualStyles();
            Application.SetCompatibleTextRenderingDefault(false);
            Main mForm = new Main();
            Application.Run();
        }
    }
}

UPDATE

I just noticed that the memory usage climbs up to 50MB and drops to 20MB afterwards (and goes up again). Is this something I can possibly address or is it a windows "issue"?

You can also consider using a `StringBuilder` here - instead of concatenating strings once a second. — Simon Whitehead, Jul 19 '13 at 15:01
Why is that unacceptable? Virtual memory is cheap. Unused pages will be paged out to disk so they're not consuming physical memory, and disk is cheap too. When virtual memory gets low the garbage collector will reclaim it. So what's the problem? — Eric Lippert, Jul 19 '13 at 15:02
Don't blame windows for silly reasons. you ask windows to allocate, it does. what is the bug in windows in this case? you don't say allocate. it will never! — Sriram Sakthivel, Jul 19 '13 at 15:03
@SimonWhitehead I'll see if it helps, is concatenating strings really such an expensive operation? — Vultour, Jul 19 '13 at 15:05
@SriramSakthivel I'm not blaming anyone, just saying that 40MB for an app with 6 3kB images is a lot. — Vultour, Jul 19 '13 at 15:11
This behavior depends upon machine. If you have enough RAM then your application may consume more memory because you have enough memory. GC runs when it sees you're running out of memory. so if you run your app in machine which has less RAM it will consume less memory since GC runs very often and claims unused memory. — Sriram Sakthivel, Jul 19 '13 at 15:13
@SriramSakthivel Not that I want to argue semantics.. but the GC doesn't run "when it sees you running out of memory".. it runs when it wants to, such as when generation thresholds are reached during an allocation. Also, those initial thresholds are nowhere near the amount of RAM in any modern computer. — Simon Whitehead, Jul 19 '13 at 15:17
since with every tick you are updating cant u replace in the exec() the += with = ? — terrybozzio, Jul 19 '13 at 15:31

score 2 · Answer 1 · answered Jul 19 '13 at 15:03

2

You never appear to be disposing your form correctly in ShowPrompt, so I'd imagine this is your problem.

Because a form displayed as a dialog box is not closed, you must call the Dispose method of the form when the form is no longer needed by your application.

ShowDialog

answered Jul 19 '13 at 15:03

Sayse

42,633
14
77
146

The prompt function is fired probably 2 times in the app's lifetime, I'll take a look at it but it shouldn't be the problem – Vultour Jul 19 '13 at 15:04
If not then at the very least its still a possible leak (maybe not to the extremes you are seeing), other than this I'd always look at the images.. – Sayse Jul 19 '13 at 15:07

score 2 · Accepted Answer · answered Jul 19 '13 at 15:03

2

I'm going to take a stab at it being the string concatenations happening once a second. Consider using a StringBuilder. 40MB is nothing though really.

RE: Your update. The Garbage Collector is reclaiming the memory as it sees fit.

answered Jul 19 '13 at 15:03

Simon Whitehead

63,300
9
114
138

Is there any option how to reduce it? I think the person I'm making it for might bring up why is it taking so much memory. Of course I could explain, but the again I'd like to prevent it. – Vultour Jul 19 '13 at 15:12
You can reduce the footprint a little by fixing up the string concatenations. Other than that it is such a waste of time. Besides.. what general users check memory usage? They won't even see an issue unless they purposely go looking in this scenario. When they see issues.. that's when you should worry about it. – Simon Whitehead Jul 19 '13 at 15:14
Let's just say that this guy is a little different. He thinks that he knows a lot (from IT), however from my experience... well, not so much. I'm pretty sure he'll be checking the memory consumption out. Whatever, I'm not gonna waste time. I'll do what I can with the concatenations and just call it a day. – Vultour Jul 19 '13 at 15:18

Ehsan · Answer 3 · 2013-07-19T15:05:35.653

Garbage Collector does all the work of memory management for you. Temporary rise in memory doesn't always mean that there is a memory leak. It may come down when the GC collects memory. In case you suspect that there are memory leaks you need to do memory profiling which is no easy job. You need to read into this article for steps that you can take to find out the problem in your code. Alternatively, there are multiple tools avaiable in the market to do this job for you. You can use Ants Profiler of Red Gate, memprofiler amongst others.

score 1 · Answer 4 · answered Jul 19 '13 at 15:05

Some points that could cut down on memory usage:

Try to prebuild all those strings you're building in exec(). It looks like they're all runtime constants, but you build them every tick instead of building them once when the application starts. If this isn't possible, use StringBuilder instead of +=.

Only change properties on controls (icon, trayText, etc) if there has been a change. I.E. if tray text is already "[Drive C:\ - OK]", don't set its value again to "[Drive C:\ - OK]" next tick.

Yup, these things were the second point in my quest for optimization, will definitely try. — Vultour, Jul 19 '13 at 15:06

CSharpYouDull · Answer 5 · 2013-07-19T16:56:03.477

One thing you might consider is rather than using a timer why not use the FileSystemWatcher and attach to events:

var watcher = new FileSystemWatcher("somepath");
watcher.Deleted += (sender, eventArgs) => { };
watcher.Changed += (sender, eventArgs) => { };
watcher.Error += (sender, eventArgs) => { };
watcher.Renamed += (sender, eventArgs) => { };

I also agree that you should be disposing of the forms once you're done with them.

Simple C# application eating memory

UPDATE

5 Answers5