views:

59

answers:

3

Hi all

I have a WinForm drawing a chart from available data.

I programmed it so that every 1 secong the Winform.Timer.Tick event calls a function that:

  • will dequeue all data available
  • will add new points on the chart

Right now data to be plotted is really huge and it takes a lot of time to be executed so to update my form. Also Winform.Timer.Tick relies on WM_TIMER , so it executes in the same thread of the Form.

Theses 2 things are making my form very UNresponsive.

What can I do to solve this issue?

I thought the following:

  • moving away from usage of Winform.Timer and start using a System.Threading.Timer
  • use the IsInvokeRequired pattern so I will rely on the .NET ThreadPool.

Since I have lots of data, is this a good idea? I have fear that at some point also the ThreadPool will be too long or too big.

Can you give me your suggestion about my issue?

Thank you very much!

AFG

+2  A: 

It is a good idea to move the fetching of the data to a Thread. You can use a BackgroundWorker that gets the data in an endless loop and

  • use the UpdateProgress event to update the chart. This takes care of the InvokeRequired business
  • Use a Sleep(remainingTime) inside the loop to get a desired frequency.
Henk Holterman
I agree, using a BackgroundWorker will take some of the stress of the main thread.
Jamie Keeling
Sorry I have to down-vote for suggesting to run a BackgroundWorker in an endless loop. It's a ThreadPool thread and shouldn't be active for the lifetime of the application.
ParmesanCodice
Generally you should not occupy a PoolThread too long, but keeping 1 or 2 around longer for this purpose isn't so bad.
Henk Holterman
+1  A: 

I would go with a System.Timers.Timer over a BackgroudWorker in an endless loop.
The BackgroundWorker is executed from a ThreadPool and is not meant to run for the lifetime of your application.

Motivation for System.Timers.Timer:

  • Each elapsed event is executed from a ThreadPool, won't hang your UI thread.
  • Using a combination of locks and enabling/disabling the timer we can get the same frequency as if we did a Thread.Sleep(xxx) in an endless loop.
  • Cleaner and more obvious as to what you are trying to achieve

Here's my suggestion:
Disabling the timer at the beginning of the method, then re-enabling it again at the end, will cater for the case where the amount of work done in the elapsed event takes longer than the timer interval. This also ensures the timer between updates is consistent. I've added a lock for extra precaution.

I used an anonymous method to update the UI thread, but you can abviously do that however you want, as long as you remember to Invoke, it's also a good idea to check the InvokeRequired property

private readonly object chartUpdatingLock = new object();

private void UpdateChartTimerElapsed(object sender, ElapsedEventArgs e)
{
    // Try and get a lock, this will cater for the case where two or more events fire
    // in quick succession.
    if (Monitor.TryEnter(chartUpdatingLock)
    {        
        this.updateChartTimer.Enabled = false;
        try
        {
            // Dequeuing and whatever other work here..

            // Invoke the UI thread to update the control
            this.myChartControl.Invoke(new MethodInvoker(delegate
                {
                    // Do you UI work here
                })); 
        }
        finally
        {
            this.updateChartTimer.Enabled = true;
            Monitor.Exit(chartUpdatingLock);
        }
    }
}
ParmesanCodice
+1  A: 

It is quite unlikely you'll be ahead by using a background timer. Your chart control almost certainly requires it to be updated from the same thread is was created on. Any kind of control that has a visible appearance does. Which requires you to use Control.BeginInvoke in the Elapsed event handler so that the update code runs on the UI thread. Dequeueing data isn't likely to be expensive, you will have actually have made it slower by invoking. And still not have taken the pressure off the UI thread.

You'll also have a potentially serious throttling problem, the timer will keep on ticking and pump data, even if the UI thread can't keep up. That will eventually crash your program with OOM.

Consider instead to make the code that updates the chart smarter. A chart can only display details of the data if such details are at least a pixel wide. Realistically, it can only display 2000 pixels with useful information. That's not much, updating 2000 data points shouldn't cause any trouble.

Hans Passant
Thanks nobugz; I don't know if I get entirely your response. Ok for charting 2000 points, but are you suggesting me to 1. avoid using System.Treading.Timers.Timer or BackgrounWork 2. keep using Winform.Timer because is in the same thread of the UI?
Abruzzo Forte e Gentile
I'm saying that System.Timers.Timer won't solve your problem, it will make it worse. It can't do a better job than a regular Timer. So keep using the regular timer, make your charting code smarter. Only chart what the user can see.
Hans Passant