John John - 3 months ago 18
C# Question

C#, multi-threading - Form not updating

I have a monte-carlo simulation running across multiple threads with a progress bar to inform the user how it's going. The progress bar management is done in a separate thread using Invoke, but the Form is not updating.

Here is my code:

Thread reportingThread = new Thread(() => UpdateProgress(iSims, ref myBag));
reportingThread.Priority = ThreadPriority.AboveNormal;
reportingThread.Start();`


and here is the function being called:

private void UpdateProgress(int iSims, ref ConcurrentBag<simResult> myBag)
{
int iCount;
string sText;

if (myBag == null)
iCount = 0;
else
iCount = myBag.Count;

while (iCount < iSims)
{
if (this.Msg.InvokeRequired)
{
sText = iCount.ToString() + " simultions of " + iSims + " completed.";
this.Msg.BeginInvoke((MethodInvoker) delegate() { this.Msg.Text = sText; this.Refresh(); });
}
Thread.Sleep(1000);
iCount = myBag.Count;
}
}


I have used both Application.DoEvents() and this.refresh() to try to force the form to update, but nothing happens.

UPDATE: Here is the procedure calling the above function

private void ProcessLeases(Boolean bValuePremium)
{
int iSims, iNumMonths, iNumYears, iIndex, iNumCores, iSimRef;
int iNumSimsPerThread, iThread, iAssets, iPriorityLevel;
string sMsg;
DateTime dtStart, dtEnd;
TimeSpan span;
var threads = new List<Thread>();
ConcurrentBag<simResult> myBag = new ConcurrentBag<simResult>();
ConcurrentBag<summaryResult> summBag = new ConcurrentBag<summaryResult>();

this.Msg.Text = "Updating all settings";
Application.DoEvents();
ShowProgressPanel();

iSims = objSettings.getSimulations();
iNumCores = Environment.ProcessorCount;

this.Msg.Text = "Initialising model";
Application.DoEvents();
iNumSimsPerThread = Convert.ToInt16(Math.Round(Convert.ToDouble(iSims) / Convert.ToDouble(iNumCores), 0));

this.Msg.Text = "Spawning " + iNumCores.ToString() + " threads";

for (iThread = 0; iThread < iNumCores; iThread++)
{
int iStart, iEnd;

if (iThread == 0)
{
iStart = (iThread * iNumSimsPerThread) + 1;
iEnd = ((iThread + 1) * iNumSimsPerThread);
}
else
{
if (iThread < (iNumCores - 1))
{
iStart = (iThread * iNumSimsPerThread) + 1;
iEnd = ((iThread + 1) * iNumSimsPerThread);
}
else
{
iStart = (iThread * iNumSimsPerThread) + 1;
iEnd = iSims;
}
}
Thread thread = new Thread(() => ProcessParallelMonteCarloTasks(iStart, iEnd, iNumMonths, iSimRef, iSims, ref objDB, iIndex, ref objSettings, ref myBag, ref summBag));
switch (iPriorityLevel)
{
case 1: thread.Priority = ThreadPriority.Highest; break;
case 2: thread.Priority = ThreadPriority.AboveNormal; break;
default: thread.Priority = ThreadPriority.Normal; break;
}
thread.Start();
threads.Add(thread);
}

// Now start the thread to aggregate the MC results
Thread MCThread = new Thread(() => objPortfolio.MCAggregateThread(ref summBag, (iSims * iAssets), iNumMonths));
MCThread.Priority = ThreadPriority.AboveNormal;
MCThread.Start();
threads.Add(MCThread);

// Here we review the CollectionBag size to report progress to the user
Thread reportingThread = new Thread(() => UpdateProgress(iSims, ref myBag));
reportingThread.Priority = ThreadPriority.AboveNormal;
reportingThread.Start();

// Wait for all threads to complete
//this.Msg.Text = iNumCores.ToString() + " Threads running.";
foreach (var thread in threads)
thread.Join();

reportingThread.Abort();

this.Msg.Text = "Aggregating results";
Application.DoEvents();

this.Msg.Text = "Preparing Results";
Application.DoEvents();
ShowResults();
ShowResultsPanel();
}


As you can see, there are a number of updates to the Form before my Invoked call and they all work fine - in each case, I am using Application.DoEvents() to update.

myBag is a ConcurrentBag into which each monte-carlo thread dumps it's results. By using the Count method, I can see how many simulations have completed and update the user.

Answer
foreach (var thread in threads)
    thread.Join();

This is your problem. You are blocking here so nothing will ever update in the UI thread until all your threads complete.

This is a critical point - .DoEvents() happens naturally and all by itself every time a block of code you have attached to a user interface event handler completes executing. One of your primary responsibilities as a developer is to make sure that any code executing in a user interface event handler completes in a timely manner (a few hundred milliseconds, maximum). If you write your code this way then there is never, ever, a need to call DoEvents()... ever.

You should always write your code this way.

Aside from performance benefits, a major plus of using threads is that they are asynchronous by nature - to take advantage of this you have to write your code accordingly. Breaking out of procedural habits is a hard one. What you need to do is to forget the .Join altogether and get out of your ProcessLeases method here - let the UI have control again.

You are dealing with updates in your threads already so all you need is completion notification to let you pick up in a new handler when all of your threads finish their work. You'll need to keep track of your threads - have them each notify on completion (ie: invoke some delegate back on the UI thread, etc) and in whatever method handles it you would do something like

 if (allThreadsAreFinished)  // <-- You'll need to implement something here
 {
      reportingThread.Abort();
      this.Msg.Text = "Preparing Results";         
      ShowResults();
      ShowResultsPanel();
 }

Alternatively, you could also simply call ProcessLeases in a background thread (making sure to correctly invoke all of your calls within it) and then it wouldn't matter that you are blocking that thread with a .Join. You could also then do away with all of the messy calls to .DoEvents().

Additionally, you don't nee the call to this.Refresh(); here :

 this.Msg.BeginInvoke((MethodInvoker) delegate() { 
      this.Msg.Text = sText; 
      this.Refresh(); 
 });

If you aren't blocking the UI thread the control will update just fine without it. If you are blocking the UI thread then adding the .Refresh() call won't help because the thread won't be free to execute it any more than it will be free to execute the previous line. This is programming chaotically - randomly adding code hoping that it will work instead of examining and understanding the reasons why it doesn't.

Comments