Prev: appropriate class variable usage?
Next: Must declare the scalar variable "@myDB@"
From: Hector Santos on 2 Jun 2010 10:23
Peter Duniho wrote:

> void RunWorkers(int count)
> {
> for (int i = 0; i < count; i++)
> {
> BackgroundWorker worker = new BackgroundWorker();
> int delay = i * 500;
>
> worker.DoWork += (sender, e) =>
> {
> // dummy work
> Thread.Sleep(delay);
> };
>
> worker.RunWorkerCompleted += (sender, e) =>
> {
> // Signal completion
> if (--count == 0)
> {
> AllWorkersDone();
> }
> };
>
> worker.RunWorkerAsync();
> }
> }
>
> void AllWorkersDone()
> {
> // do whatever here
> }
>
> • The "count" variable, used directly in RunWorkers() and as a
> captured variable in the RunWorkerCompleted event handlers, is only ever
> used in a single thread, ensuring synchronization.


Hi Pete,

I appreciate the example in using the anonymous functions! Something
I intend to master in C# :)

Just a few small notes:

1) As a principle, using time for synchronization is a engineering
taboo in thread designs. A Sleep() is not simulating work because
there is no work being done and hence no other promotions of system
overhead context switching and/or interrupts. Even then, inevitably,
it will provide unexpected behavior when using a "random" or unknown
thread residence time.

2) Using the loop limit in this example as a thread count "signal" is
not good here. Any delay within the loop that is longer than any
thread residency time will alter the captured count variable.

To illustrate, change the code to add a delay between thread startup
to see what I mean:

int delay = i*500;
if (i == 1) {
// i.e, Simulate Preparation work
Thread.Sleep(100);
}

Overall, from a software engineering standpoint, avoid thread designs
based on time syncing and avoid using the common resource such as loop
limit as the watch variable here.

Two simple solutions - set the thread count outside the loop, or first
prepare the threads, much like a "Suspended" concept and then start it
after the preparation.

void RunWorkers(int count)
{
int threadCount = count;
for (int i = 0; i < count; i++)
{
BackgroundWorker worker = new BackgroundWorker();
int delay = i * 500;
if (i == 1) {
// i.e, Simulate Preparation work
Thread.Sleep(100);
}

worker.DoWork += (sender, e) =>
{
// dummy work
Thread.Sleep(delay);
};

worker.RunWorkerCompleted += (sender, e) =>
{
// Signal completion
if (--threadCount == 0)
{
AllWorkersDone();
}
};

worker.RunWorkerAsync();
}
}

or

void RunWorkers(int count)
{
// prepare threads

BackgroundWorker[] workers = new BackgroundWorker[count];
for (int i = 0; i < count; i++)
{
worker[i] = new BackgroundWorker();
int delay = i * 500;
if (i == 1) {
// i.e, Simulate Preparation work
Thread.Sleep(100);
}

worker[i]DoWork += (sender, e) =>
{
// dummy work
Thread.Sleep(delay);
};

worker[i].RunWorkerCompleted += (sender, e) =>
{
// Signal completion
if (--Count == 0)
{
AllWorkersDone();
}
};
}

// start threads
foreach (var w in workers) w.RunWorkerAsync();
}


Anyway, I see your main layout method and it was very good to
illustrate this using anonymous functions - learned a lot. Thanks.


--
HLS
From: Peter Duniho on 2 Jun 2010 11:25
Hector Santos wrote:
> [...]
> Just a few small notes:

Hector, you have completely missed the point of the code example, both
in terms of what things in the code example are important, and in
comprehending how the example actually works.

> 1) As a principle, using time for synchronization is a engineering taboo
> in thread designs. A Sleep() is not simulating work because there is no
> work being done and hence no other promotions of system overhead context
> switching and/or interrupts. Even then, inevitably, it will provide
> unexpected behavior when using a "random" or unknown thread residence time.

As my comment that reads "dummy work" explains, the call to
Thread.Sleep() has nothing to do with the code example. It's there
simply to simulate the worker thread taking some time to do work. In a
real implementation, the call to Thread.Sleep() would not be present,
and its presence or lack of presence has no effect on whether the
technique I've illustrated works or not.

If you feel more comfortable replacing the call to Thread.Sleep() with a
busy loop, go right ahead. It doesn't change the validity of the code
example one bit, because the call to Thread.Sleep() has nothing to do
with what's being illustrated.

> 2) Using the loop limit in this example as a thread count "signal" is
> not good here. Any delay within the loop that is longer than any thread
> residency time will alter the captured count variable.

You are also mistaken on your second point. As I already pointed out,
the thread-count variable is only ever used in a single thread: the main
GUI thread. The RunWorkerCompleted event handlers cannot execute until
the method that is initializing the threads has returned, and only one
event handler at a time can execute, because they all execute on the
same thread.

> To illustrate, change the code to add a delay between thread startup to
> see what I mean:
>
> int delay = i*500;
> if (i == 1) {
> // i.e, Simulate Preparation work
> Thread.Sleep(100);
> }

It would have been nice if you'd actually tried adding that code
yourself. Had you done so, you would have seen that it has no effect on
the outcome of the code, other than to delay it of course.

> Overall, from a software engineering standpoint, avoid thread designs
> based on time syncing and avoid using the common resource such as loop
> limit as the watch variable here.

Again, there is nothing in my code that relies on "time synching". The
call to Thread.Sleep() is simply to simulate the worker thread doing
something. The code will work regardless of the time each worker thread
spends doing work.

> Two simple solutions - set the thread count outside the loop, or first
> prepare the threads, much like a "Suspended" concept and then start it
> after the preparation.

Neither of those are required. They only add complexity, for no benefit.

> [...]
> Anyway, I see your main layout method and it was very good to illustrate
> this using anonymous functions - learned a lot. Thanks.

I hope that you will go back and review the code example more carefully.
As I said at the outset, it is a good idea to become familiar with the
..NET-native techniques, and in this case that includes understanding
what BackgroundWorker actually does and how it works.

In particular, knowing that the BackgroundWorker events, except for
DoWork, always get raised on the same thread that created the
BackgroundWorker instance (assuming that thread is one with a
SynchronizationContext, such as a normal, Forms GUI thread) is critical
in understanding why the code example I posted is correct.

Without that understanding, one might get distracted nit-picking aspects
of the code that don't have anything to do with the actual example, and
making incorrect statements about why one believes the code doesn't work
as posted.

Pete
From: Jason Keats on 2 Jun 2010 11:49
Hector Santos wrote:
> For the longest, for my non-I/O based thread work, I used
> WaitForMultiObjects (WFMO) as a way to wait for a set of worker threads
> to complete, simpling by putting the thread handles in an array and
> passing it to WFMO, for example:
>
> // Wait (infinitely) for 10 threads to ALL complete.
>
> HANDLE hThreads[10] = {0};
>
> for(int i=0; i<10; i++) {
> hThreads[i] = CreateThread(.......);
> }
>
> WFMO(hThreads,10,TRUE,INFINITE);
>
> I wish to do the same in C#, I can start the threads, but I don't see a
> simple WFMO() idea.
>
> Ideally, I wish to have a callback version of WFMO() so that an event is
> called (i.e. a OnThreadsFinish() event) signally when all threads are
> finished.
>
> Note: I was able to come up with a method, but it seems there is a more
> natural way in .NET. What I do is:
>
> 1) Create X BackgroundWorkers and put it into an array Workers[]
>
> 2) Then have a DoEvents loop:
>
> int done = 0;
> while (done < X)
> {
> done = 0;
> for (int i=0; i < X; i++) {
> if (!Workers[i].IsBusy) done++;
> }
> Application.DoEvents();
> }
>
> // THREADS WE DONE - do whatever
>
> I only used a Thread Pool. But I don't see a function there to determine
> when all the queued threads are complete.
>
> Thanks
>

A completely different approach would be to use Retlang.

The following illustrates what you're after:
http://code.google.com/p/retlang/wiki/SummationExample

HTH
From: Hector Santos on 2 Jun 2010 13:31
Peter Duniho wrote:

> Hector Santos wrote:
>> [...]
>> Just a few small notes:
>
> Hector, you have completely missed the point of the code example, both
> in terms of what things in the code example are important, and in
> comprehending how the example actually works.


Really? I think you are assuming too to be saying that.

> As my comment that reads "dummy work" explains, the call to
> Thread.Sleep() has nothing to do with the code example. It's there
> simply to simulate the worker thread taking some time to do work. In a
> real implementation, the call to Thread.Sleep() would not be present,
> and its presence or lack of presence has no effect on whether the
> technique I've illustrated works or not.


Its an important concept to not mix up SLEEP with ACTUAL THREAD WORK
when it come to system stabilization.

>> 2) Using the loop limit in this example as a thread count "signal" is
>> not good here. Any delay within the loop that is longer than any
>> thread residency time will alter the captured count variable.
>
> You are also mistaken on your second point. As I already pointed out,
> the thread-count variable is only ever used in a single thread: the main
> GUI thread. The RunWorkerCompleted event handlers cannot execute until
> the method that is initializing the threads has returned, and only one
> event handler at a time can execute, because they all execute on the
> same thread.
>
>> To illustrate, change the code to add a delay between thread startup
>> to see what I mean:
>>
>> int delay = i*500;
>> if (i == 1) {
>> // i.e, Simulate Preparation work
>> Thread.Sleep(100);
>> }
>
> It would have been nice if you'd actually tried adding that code
> yourself. Had you done so, you would have seen that it has no effect on
> the outcome of the code, other than to delay it of course.


I did. This following illustrates the contention issue with count as
I described.

using System;
using System.Text;
using System.ComponentModel;
using System.Threading;

namespace ConsoleApplet {

class CoBackgroundWorker : BackgroundWorker
{
public int tid = 0;
}
class WorkerJobs
{
void BeepDone()
{
Console.Beep(2000, 200);
Console.Beep(1000, 50);
}

public void RunWorkers(int count)
{
Console.WriteLine("* Starting {0} Threads",count);
for (int i = 0; i < count; i++)
{
CoBackgroundWorker worker = new CoBackgroundWorker();
worker.tid = i;

int delay = i*500;
if (i == 1) Thread.Sleep(100);

worker.DoWork += (sender, e) =>
{
// dummy work
CoBackgroundWorker w = sender as CoBackgroundWorker;
Console.WriteLine("- tid: {0} delay={2} count={3}",
w.tid, delay ,count);
Thread.Sleep(delay);
};

worker.RunWorkerCompleted += (sender, e) =>
{
CoBackgroundWorker w = sender as CoBackgroundWorker;
WorkerDone(w.tid, count);
if (--count == 0)
{
AllWorkersDone(w.tid);
}
};
worker.RunWorkerAsync();
}
}

void WorkerDone(int tid, int count)
{
Console.WriteLine("- tid: {0} Worker Done! count={1}",tid,
count);
}
void AllWorkersDone(int tid)
{
BeepDone();
Console.WriteLine("- tid: {0} All Workers Done!",tid);
}
}

class Program
{

static void Main(string[] args)
{
WorkerJobs jobs = new WorkerJobs();

Console.WriteLine("- Press ESCAPE to Exit");

jobs.RunWorkers(5);

while (true)
{
Thread.Sleep(50);
if (Console.KeyAvailable &&
Console.ReadKey().Key == ConsoleKey.Escape) {
break;
}
}

}
}
}

--
HLS
From: Hector Santos on 2 Jun 2010 19:07
Peter Duniho wrote:

>> Overall, from a software engineering standpoint, avoid thread designs
>> based on time syncing and avoid using the common resource such as loop
>> limit as the watch variable here.
>
> Again, there is nothing in my code that relies on "time synching". The
> call to Thread.Sleep() is simply to simulate the worker thread doing
> something. The code will work regardless of the time each worker thread
> spends doing work.


The point is that a little testing yourself would of shown that the
using the count variable was not a good idea.

I naturally saw it even for I tried it to prove it was wrong because
as you indicated, in the world real there would be "real work" not the
poor man's sleep (which isn't any kind of simulated work whatsoever).
This is common mistake in thread designs.

>> Two simple solutions - set the thread count outside the loop, or first
>> prepare the threads, much like a "Suspended" concept and then start it
>> after the preparation.
>
> Neither of those are required. They only add complexity, for no benefit.


But your example had a bug as I illustrated.

> I hope that you will go back and review the code example more carefully.


I did even before I posted. But only tried it because it was obvious
there would a thread contention issue.


> As I said at the outset, it is a good idea to become familiar with the
> .NET-native techniques, and in this case that includes understanding
> what BackgroundWorker actually does and how it works.


I doubt you even know which a common mistake you showed. If you want
to be respected, then show some respect yourself.

> In particular, knowing that the BackgroundWorker events, except for
> DoWork, always get raised on the same thread that created the
> BackgroundWorker instance (assuming that thread is one with a
> SynchronizationContext, such as a normal, Forms GUI thread) is critical
> in understanding why the code example I posted is correct.


You see, you assumed that I was naive about thread, GUI designs or
designs in general I seriously doubt you can match my expertise in
this area. But that wasn't even the question - how GUI are blocks or
not. Yet, to show a solution, an able one, you did what experts
consider a common mistake in thread design 101.

> Without that understanding, one might get distracted nit-picking aspects
> of the code that don't have anything to do with the actual example, and
> making incorrect statements about why one believes the code doesn't work
> as posted.


Well, the fact was my statements are 100% correct and on the money,
and I am following up for the archives, just in case someone needs
something similar and uses your posted code as a framework.

You need to focus on what the questions and DO NOT PREASSUME too much
into what a poster knows or doesn't.

Thanks for your comments.

--
HLS
First  |  Prev  |  Next  |  Last
Pages: 1 2 3 4 5
Prev: appropriate class variable usage?
Next: Must declare the scalar variable "@myDB@"