Keeping UI Responsive with foreach loop in WinForms

Question

I have created a Winforms App that uses the Open Hardware Monitor to display PC temps in a gauge format using Live Charts. I am aware that the following code causes the UI to become unresponsive as the charts are updated but I am not able to figure out how to implement any form of threading to work with this or how to change how I have coded the application to keep the UI responsive. The timer will tick every two seconds which fetches the values and updates the gauges.

References added to the project are:

OpenHardwareMonitorLib
LiveCharts.Winforms + Dependencies

See my code below, any advice would be appreciated.

UserControls.TempGauge

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Drawing;
using System.Data;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows.Forms;
using System.Windows.Media;

namespace PCDashboard.UserControls
{
public partial class TempGauge : UserControl
{
    public TempGauge(string name)
    {
        try
        {
            //Initialize
            InitializeComponent();

            //Gauge Properties
            this.Name = name;
            gaugeName.Text = name;
            gauge.Uses360Mode = true;
            gauge.From = 0;
            gauge.To = 110;
            gauge.Value = 0;
            gauge.HighFontSize = 60;
            gauge.Base.Foreground = System.Windows.Media.Brushes.White;
            gauge.InnerRadius = 0;
            gauge.GaugeBackground = Classes.Colours.Blue;
        }
        catch (Exception)
        {

        }
    }
  }
}

MainForm

using OpenHardwareMonitor.Hardware;
using PCDashboard.Classes;
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows.Forms;

namespace PCDashboard.Forms
{
    public partial class MainForm : Form
    {
        public MainForm()
        {
            InitializeComponent();
        }

        private void MainForm_Load(object sender, EventArgs e)
        {
            try
            {
                GetSystemTemps(false);
                timer.Enabled = true;
            }
            catch (Exception)
            {
            }
        }

        private void exitToolStripMenuItem_Click(object sender, EventArgs e)
        {
            Close();
        }

        private void GetSystemTemps(bool UpdateGauge)
        {
            try
            {
                UpdateVisitor updateVisitor = new UpdateVisitor();
                Computer computer = new Computer();
                computer.Open();

                //Which components to read?
                computer.CPUEnabled = true;
                computer.GPUEnabled = true;
                computer.MainboardEnabled = true;
                computer.RAMEnabled = true;

                computer.Accept(updateVisitor);

                foreach (IHardware hardware in computer.Hardware)
                {
                    foreach (ISensor sensor in hardware.Sensors.Where(
                        x => x.SensorType == SensorType.Temperature))
                    {
                        if (UpdateGauge)
                        {
                            UserControls.TempGauge tempGauge =
                                ((UserControls.TempGauge)gaugeContainer
                                    .Controls[sensor.Name]);

                            if (tempGauge != null)
                            {
                                tempGauge.gauge.Value = Math.Round(
                                    double.Parse(sensor.Value.ToString()), 0);
                            }
                        }
                        else
                        {
                            UserControls.TempGauge tempGauge =
                                new UserControls.TempGauge(sensor.Name);
                            gaugeContainer.Controls.Add(tempGauge);
                        }
                    }
                }

                computer.Close();
            }
            catch (Exception)
            {
            }
        }

        private void timer_Tick(object sender, EventArgs e)
        {
            try
            {
                GetSystemTemps(true);
            }
            catch (Exception)
            {
            }
        }
    }
}

You don't seem to be doing anything that would prevent the UI from being unresponsive and unless I'm missing something, a timer is perfectly acceptable. Simply polling for values and updating a control should be quick. The adding of controls shouldn't be a major use case. Is OpenHardwareMonitorLib particularly slow in polling for values? — , Apr 26 '20 at 01:46
It's not particularly slow but it's noticeable. For example trying to move the form, instead of being able to drag it around freely there is considerable delay, same with accessing the file menu, you have the time it takes for the timer to tick again. I was hoping to somehow do the work on a different thread and update the gauge if possible? — Jack Foulkes, Apr 26 '20 at 02:06
You could try using async/await which might help with the stuttering of the form. Here's [some docs](https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/concepts/async/) on it. — li223, Apr 26 '20 at 02:25
What `Interval` are you using. You know its _milliseconds_ not seconds — , Apr 26 '20 at 02:35
@li223 it's not immediately necessary for what the OP is doing — , Apr 26 '20 at 02:36
It is possible that the slow part is not getting the values of the sensors, but creating the `ISensor` objects in the first place. In that case you could retrieve the sensors once, and store them in an array or list. Then on every `timer_Tick` enumerate the array. — Theodor Zoulias, Apr 26 '20 at 04:07
_"The timer will tick every two seconds"_ - 1) is your timer `Interval` set to `2` or `2000`? 2) Have you tried measuring how long each operation takes? Use `Stopwatch` and in `timer_Tick`, measure exactly how long the entire call to `GetSystemTemps` takes. As I said, either **your timer interval is too short** or `OpenHardwareMonitorLib` is too slow. Looking at your code you shouldn't require threads....yet — , Apr 26 '20 at 11:22

score 1 · Accepted Answer · answered Apr 26 '20 at 02:59

My suggestion is to separate the data retrieval from the presentation. This way you will be able to offload the heavy work of data retrieval to a background thread, without having to worry how you will update the UI controls at the same time. Here is how you could make a GetSensorData method that fetches the data as an array of ValueTuple<string, double> elements, representing the name and the value of each sensor:

private (string, double)[] GetSensorData()
{
    var list = new List<(string, double)>();
    Computer computer = new Computer();
    computer.Open();
    //...
    foreach (IHardware hardware in computer.Hardware)
    {
        foreach (ISensor sensor in hardware.Sensors.Where(
            x => x.SensorType == SensorType.Temperature))
        {
            list.Add(sensor.Name, double.Parse(sensor.Value.ToString()));
        }
    }
    computer.Close();
    return list.ToArray();
}

Then you could use the Task.Run method to offload the heavy work of retrieving the data to a background thread (a ThreadPool thread). This method returns a Task, that could be awaited asynchronously so that the code below the await has available data to work with.

private async Task UpdateSystemTempsAsync(bool updateGauge)
{
    var data = await Task.Run(() => GetSensorData()); // Offload to thread-pool

    // While awaiting the UI remains responsive

    // After the await we are back in the UI thread

    foreach (var (sensorName, sensorValue) in data)
    {
        if (updateGauge)
        {
            UserControls.TempGauge tempGauge =
                ((UserControls.TempGauge)gaugeContainer.Controls[sensorName]);
            if (tempGauge != null)
            {
                tempGauge.gauge.Value = Math.Round(sensorValue, 0);
            }
        }
        else
        {
            var tempGauge = new UserControls.TempGauge(sensorName);
            gaugeContainer.Controls.Add(tempGauge);
        }
    }
}

Finally you'll have to make the event handlers async:

private async void MainForm_Load(object sender, EventArgs e)
{
    try
    {
        await UpdateSystemTempsAsync(false);
        timer.Enabled = true;
    }
    catch { }
}

private async void timer_Tick(object sender, EventArgs e)
{
    timer.Enabled = false;
    try
    {
        await UpdateSystemTempsAsync(true);
        timer.Enabled = true;
    }
    catch { }
}

In case the slow part is updating the UI controls, a solution you could try is injecting an `await Task.Yield();` in every loop, so that the UI thread has time to process UI messages between redrawing each `TempGauge`. — Theodor Zoulias, Apr 26 '20 at 04:14
Thank you for this, using this method has worked! I just had to add to the list using a new ValueTuple: list.Add(new ValueTuple(sensor.Name, double.Parse(sensor.Value.ToString()))); — Jack Foulkes, Apr 26 '20 at 11:17
@JackFoulkes yeap, you need C# 7.1 or later to enjoy all the available [syntax sugar](https://learn.microsoft.com/en-us/dotnet/csharp/tuples) for tuples! — Theodor Zoulias, Apr 26 '20 at 12:12

Joel Coehoorn · Answer 2 · 2020-04-26T03:48:46.630

Using a timer for this is perfectly reasonable. But there are three things you can do to help your app stay responsive.

First, I'd check the Interval property on the timer. Anything less than 100 is probably too small, and you can probably go quite a bit higher. Even 250 is still 4 times per second, and 400 more than twice per second.

Second, I'd bracket the loop with calls to SuspendLayout() and ResumeLayout(). This will help your form be more efficient about redrawing in a batch.

Finally, IIRC you can set the timer's Enabled property to false for form Move events, and back to true again for ResizeEnd events, to disable updates while someone is moving the form around. This is from way back for me, though, so you'll want to test this one.

Okay, a quick test shows the third option works in a basic way, but doesn't cover every scenario. You'll also need to set to true on (at minimum) window state changes (detect via `SizeChanged` event). — Joel Coehoorn, Apr 26 '20 at 03:45

Keeping UI Responsive with foreach loop in WinForms

2 Answers2