C# WPF Threading

Question

I've been following this guide to the WPF threading model to create a little application that will monitor and display current and peak CPU usage. However, when I update my current and peak CPU in the event handler, the numbers in my window don't change at all. When debugging, I can see that the text fields do change but don't update in the window.

I've heard it's bad practice to go about building an application like this and should go for a MVVM approach instead. In fact, a few have been surprised that I've been able to run this without a runtime exception. Regardless, I'd like to figure out the code example/guide from the first link.

Let me know what your thoughts are!

And here is my xaml:

<Window x:Class="UsagePeak2.MainWindow"
    xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
    Title="CPU Peak" Height="75" Width="260">
<StackPanel Orientation="Horizontal" VerticalAlignment="Center" >
    <Button Content="Start"  
        Click="StartOrStop"
        Name="startStopButton"
        Margin="5,0,5,0"
        />
    <TextBlock Margin="10,5,0,0">Peak:</TextBlock>
    <TextBlock Name="CPUPeak" Margin="4,5,0,0">0</TextBlock>
    <TextBlock Margin="10,5,0,0">Current:</TextBlock>
    <TextBlock Name="CurrentCPUPeak" Margin="4,5,0,0">0</TextBlock>
</StackPanel>

Here is my Code

public partial class MainWindow : Window
{
    public delegate void NextCPUPeakDelegate();

    double thisCPUPeak = 0;

    private bool continueCalculating = false;

    PerformanceCounter cpuCounter;

    public MainWindow() : base()
    {
        InitializeComponent();
    }

    private void StartOrStop(object sender, EventArgs e)
    {
        if (continueCalculating)
        {
            continueCalculating = false;
            startStopButton.Content = "Resume";
        }
        else
        {
            continueCalculating = true;
            startStopButton.Content = "Stop";
            startStopButton.Dispatcher.BeginInvoke(
                DispatcherPriority.Normal, new NextCPUPeakDelegate(GetNextPeak));
            //GetNextPeak();
        }
    }

    private void GetNextPeak()
    {

        cpuCounter = new PerformanceCounter("Processor", "% Processor Time", "_Total");

        double currentValue = cpuCounter.NextValue();

        CurrentCPUPeak.Text = Convert.ToDouble(currentValue).ToString();

        if (currentValue > thisCPUPeak)
        {
            thisCPUPeak = currentValue;
            CPUPeak.Text = thisCPUPeak.ToString();
        }

        if (continueCalculating)
        {
            startStopButton.Dispatcher.BeginInvoke(
                System.Windows.Threading.DispatcherPriority.SystemIdle,
                new NextCPUPeakDelegate(this.GetNextPeak));
        }
    }
}

Answer 1

There's a few problems here. First, you are doing work on the main thread, but you're doing it in a very roundabout way. It's this line right here that allows your code to be responsive:

startStopButton.Dispatcher.BeginInvoke(
    System.Windows.Threading.DispatcherPriority.SystemIdle,
    new NextCPUPeakDelegate(this.GetNextPeak));

From the BeginInvoke method documentation (emphasis mine):

Executes the specified delegate asynchronously at the specified priority on the thread the Dispatcher is associated with.

Even though you're sending this at a high rate, you're queuing work on the UI thread by having the post back into the message loop occur on a background thread. This is what allows your UI to be at all responsive.

That said, you want to restructure your GetNextPeak method like this:

private Task GetNextPeakAsync(CancellationToken token)
{
    // Start in a new task.
    return Task.Factory.StartNew(() => {
        // Store the counter outside of the loop.
        var cpuCounter = 
            new PerformanceCounter("Processor", "% Processor Time", "_Total");

        // Cycle while there is no cancellation.
        while (!token.IsCancellationRequested)
        {
            // Wait before getting the next value.
            Thread.Sleep(1000);

            // Get the next value.
            double currentValue = cpuCounter.NextValue();

            if (currentValue > thisCPUPeak)
            {
                thisCPUPeak = currentValue;
            }

            // The action to perform.
            Action<double, double> a = (cv, p) => {
                CurrentCPUPeak.Text = cv.ToString();
                CPUPeak.Text = p.ToString();
            };

            startStopButton.Dispatcher.Invoke(a, 
                new object[] { currentValue, thisCPUPeak });
        }
    }, TaskCreationOptions.LongRunning);
}

Notes about the above:

• The call to the NextValue method on the PerformanceCounter class must have some time expire before values start coming through, as per Dylan's answer.

• A CancellationToken structure is used to indicate if the operation should be stopped. This drives the loop that will continuously run in the background.

• Your method now returns a Task class which represents the background information.

• Synchronization around the thisCPUPeak value is not needed, as the singular background thread is the only place where it is read and written to; the call to the Invoke method on the Dispatcher class has copies of the currentValue and thisCPUPeak values passed into it. If you want to access the thisCPUPeak value in any other thread (UI thread included) then you need to synchronize access to the value (most likely through the lock statement).

Now, you'll have to hold onto the Task on the class level as well and have a reference to a CancellationTokenSource (which produces the CancellationToken):

private Task monitorTask = null;
private CancellationTokenSource cancellationTokenSource = null;

And then change your StartOrStop method to call the task

private void StartOrStop(object sender, EventArgs e)
{
    // If there is a task, then stop it.
    if (task != null)
    {
        // Dispose of the source when done.
        using (cancellationTokenSource)
        {
            // Cancel.
            cancellationTokenSource.Cancel();
        }

        // Set values to null.
        task = null;
        cancellationTokenSource = null;

        // Update UI.
        startStopButton.Content = "Resume";
    }
    else
    {
        // Update UI.
        startStopButton.Content = "Stop";

        // Create the cancellation token source, and
        // pass the token in when starting the task.
        cancellationTokenSource = new CancellationTokenSource();
        task = GetNextPeakAsync(cancellationTokenSource.Token);
    }
}

Note that instead of the flag, it checks to see if the Task is already running; if there is no Task then it starts up the loop, otherwise, it cancels the existing loop using the CancellationTokenSource.

Answer 2

You are not seeing the UI update because you aren't using the performance counter correctly. The first time you query the processor time performance counter, it will always be 0. See this question.

cpuCounter = new PerformanceCounter("Processor", "% Processor Time", "_Total");

double currentValue = cpuCounter.NextValue();

Thread.Sleep(1000);

currentValue = cpuCounter.NextValue();

Something as simple as this will fix the problem, but you probably want to develop a more robust solution, taking into consideration some of the remarks in the comments above.