Why does this Parallel.ForEach code freeze the program up?
Solution 1
You must not start the parallel processing in your UI thread. See the example under the "Avoid Executing Parallel Loops on the UI Thread" header in this page.
Update: Or, you can simply create a new thread manuall and start the processing inside that as I see you have done. There's nothing wrong with that too.
Also, as Jim Mischel points out, you are accessing the lists from multiple threads at the same time, so there are race conditions there. Either substitute ConcurrentBag
for List
, or wrap the lists inside a lock
statement each time you access them.
Solution 2
A good way to circumvent the problems of not being able to write to the UI thread when using Parallel statements is to use the Task Factory and delegates, see the following code, I used this to iterate over a series of files in a directory, and process them in a Parallel.ForEach
loop, after each file is processed the UI thread is signaled and updated:
var files = GetFiles(directoryToScan);
tokenSource = new CancellationTokenSource();
CancellationToken ct = tokenSource.Token;
Task task = Task.Factory.StartNew(delegate
{
// Were we already canceled?
ct.ThrowIfCancellationRequested();
Parallel.ForEach(files, currentFile =>
{
// Poll on this property if you have to do
// other cleanup before throwing.
if (ct.IsCancellationRequested)
{
// Clean up here, then...
ct.ThrowIfCancellationRequested();
}
ProcessFile(directoryToScan, currentFile, directoryToOutput);
// Update calling thread's UI
BeginInvoke((Action)(() =>
{
WriteProgress(currentFile);
}));
});
}, tokenSource.Token); // Pass same token to StartNew.
task.ContinueWith((t) =>
BeginInvoke((Action)(() =>
{
SignalCompletion(sw);
}))
);
And the methods that do the actual UI changes:
void WriteProgress(string fileName)
{
progressBar.Visible = true;
lblResizeProgressAmount.Visible = true;
lblResizeProgress.Visible = true;
progressBar.Value += 1;
Interlocked.Increment(ref counter);
lblResizeProgressAmount.Text = counter.ToString();
ListViewItem lvi = new ListViewItem(fileName);
listView1.Items.Add(lvi);
listView1.FullRowSelect = true;
}
private void SignalCompletion(Stopwatch sw)
{
sw.Stop();
if (tokenSource.IsCancellationRequested)
{
InitializeFields();
lblFinished.Visible = true;
lblFinished.Text = String.Format("Processing was cancelled after {0}", sw.Elapsed.ToString());
}
else
{
lblFinished.Visible = true;
if (counter > 0)
{
lblFinished.Text = String.Format("Resized {0} images in {1}", counter, sw.Elapsed.ToString());
}
else
{
lblFinished.Text = "Nothing to resize";
}
}
}
Hope this helps!
Solution 3
If anyone's curious, I kinda figured it out but I'm not sure if that's good programming or any way to deal with the issue.
I created a new thread like so:
Thread t = new Thread(do_checks);
t.Start();
and put away all of the parallel stuff inside of do_checks().
Seems to be doing okay.
Solution 4
One problem with your code is that you're calling FinishedProxies.Add
from multiple threads concurrently. That's going to cause a problem because List<T>
isn't thread-safe. You'll need to protect it with a lock or some other synchronization primitive, or use a concurrent collection.
Whether that causes the UI lockup, I don't know. Without more information, it's hard to say. If the proxies
list is very long and checkProxy
doesn't take long to execute, then your tasks will all queue up behind that Invoke
call. That's going to cause a whole bunch of pending UI updates. That will lock up the UI because the UI thread is busy servicing those queued requests.
Solution 5
This is what I think might be happening in your code-base.
Normal Scenario: You click on button. Do not use Parallel.Foreach
loop. Use Dispatcher class and push the code to run on separate thread in background. Once the background thread is done processing, it will invoke the main UI thread for updating the UI. In this scenario, the background thread(invoked via Dispatcher) knows about the main UI thread, which it needs to callback. Or simply said the main UI thread has its own identity.
Using Parallel.Foreach loop: Once you invoke Paralle.Foreach
loop, the framework uses the threadpool
thread. ThreadPool
threads are chosen randomly and the executing code should never make any assumption on the identity of the chosen thread. In the original code its very much possible that dispatcher thread invoked via Parallel.Foreach
loop is not able to figure out the thread which it is associated with. When you use explicit thread, then it works fine because the explicit thread has its own identity which can be relied upon by the executing code.
Ideally if your main concern is all about keeping UI responsive, then you should first use the Dispatcher class to push the code in background thread and then in there use what ever logic you want to speedup the overall execution.
dsp_099
Updated on July 09, 2022Comments
-
dsp_099 almost 2 years
More newbie questions:
This code grabs a number of proxies from the list in the main window (I couldn't figure out how to make variables be available between different functions) and does a check on each one (simple
httpwebrequest
) and then adds them to a list calledfinishedProxies
.For some reason when I press the start button, the whole program hangs up. I was under the impression that Parallel creates separate threads for each action leaving the UI thread alone so that it's responsive?
private void start_Click(object sender, RoutedEventArgs e) { // Populate a list of proxies List<string> proxies = new List<string>(); List<string> finishedProxies = new List<string>(); foreach (string proxy in proxiesList.Items) { proxies.Add(proxy); } Parallel.ForEach<string>(proxies, (i) => { string checkResult; checkResult = checkProxy(i); finishedProxies.Add(checkResult); // update ui /* status.Dispatcher.Invoke( System.Windows.Threading.DispatcherPriority.Normal, new Action( delegate() { status.Content = "hello" + checkResult; } )); */ // update ui finished //Console.WriteLine("[{0}] F({1}) = {2}", Thread.CurrentThread.Name, i, CalculateFibonacciNumber(i)); }); }
I've tried using the code that's commented out to make changes to the UI inside the
Parallel.Foreach
and it makes the program freeze after the start button is pressed. It's worked for me before but I used Thread class.How can I update the UI from inside the
Parallel.Foreach
and how do I makeParallel.Foreach
work so that it doesn't make the UI freeze up while it's working? -
dsp_099 over 12 yearsAlso, I load about 10 proxies each time to run the test so it's not too many; I added a line AFTER the parallel.foreach that changes a label box to 'checker is complete!' and when i press the check button the program WAITS until all of the processes are done before updating the box so it's like yes they all do run simultaneously but it's as if they run simultaneously in the same thread if that makes any sense, because simply doing the httpwebrequest from the same ui thread would cause it to hang up exactly the same way.
-
Fulproof about 11 yearsWhat do you mean under "substitute
ConcurrentBag
forList
"? ... Did you really mean "substituteList
toConcurrentBag
"? -
Waihon Yew about 11 years@Fulproof: I believe that correct English is "substitute A for B" == "substitute B with A".
-
Fulproof about 11 years"substitute one thing (A) for another (B)" == "substitute A with B" == (A) "takes place or performs the function of the other" (B). Can you give any reference to your usage?
-
Fulproof about 11 yearsAnyway, I'd like to understand better how to choose between .NET concurrent collections for multi-threading in WPF
-
Waihon Yew about 11 years@Fulproof: Reference, examples #2 and #3 are exactly what I 'm referring to. Your own ref does not mention "subtitute with" at all as far as I can see.
-
Simon MᶜKenzie almost 11 yearsYou might get a further performance boost using
BeginInvoke
onto the UI thread, then you don't have to wait while it's updating, as you currently do when usingInvoke
. Of course, this might require locks insideWriteProgress
... -
StevenVL almost 11 yearsThe UI thread currently isn't being locked, it stays fully responsive, so i'm not sure how this might help? But I'm gonna give it a try to see the difference, locking the objects themselves isn't a problem in my scenario.
-
Simon MᶜKenzie almost 11 yearsIgnore my comment about the lock - I was thinking about multiple threads running the
BeginInvoke
, but since they're all being invoked on the UI thread, there can't be any re-entrancy. What I was saying is that each thread in yourParallel.ForEach
has to wait for theInvoke
to complete, which will slow things down. WithBeginInvoke
, the UI updates will be queued onto the UI thread and will run asynchronously. -
StevenVL almost 11 yearsSetting up a test case right now to compare, I'll come back here when I got the results, thanks in advance!
-
StevenVL almost 11 yearsJust finished my tests, using the same amount of files to be resized for each call.
Invoke
gave me results from 9.6seconds to 8.2 fastest (only once), whereasBeginInvoke
was faster, going from 8.1 fastest several times to 8.5 slowest. Thanks for the tip @SimonMcKenzie! -
Simon MᶜKenzie almost 11 yearsNo worries. Of course, in a tighter loop or with more involved UI updates, the improvements can be a lot more substantial.
-
B Medeiros over 6 yearsJon, you're right, but as this is an international forum it would have been nicer if a more straightforward expression were used. "Either use ConcurrentBag instead of List or..." would have been a lot easier to read for everyone, I think. Lesson learnt, thought. :)
-
Davide Piras over 5 yearsVERY Helpful, did not compile first shot but of big help, i implemented it just now, THANKS!
-
Davide Piras over 5 yearsthis is very helpful i tried this one out, simple and effective, thanks!!
-
Theodor Zoulias over 2 yearsStartNew is Dangerous.
Task.Run
is preferable.