Chris Mullins said:
I scanned through your article and it seemed very well written. I have a
little bit of feedback on a few sections that you may be interested in:
The section on volitile and stale data, I though the example and text was
good for beginners, but didn't touch on what was (is!) the biggest issue
there for me - that is the problem of "it runs great in debug mode, but when
I turn on optimizations my code doesn't work any more." issue. The way the
various optimizers can and do isolate variables inside loops and other very
subtle things has caught me by surprise on more than one occasion. Your
suggestion of wrapping things in a lock in order to avoid the 'volitile'
issue is excellent - especially as it'll work in VB.NET, which lacks the
volitile keyword.
I can't work out whether I'm surprised that VB.NET lacks volatile or
not - I think I *may* have tried to find an equivalent and failed, but
I'm not sure.
It's a good point though - I think I've specified somewhere that "it
works for me" isn't a good enough reason to believe the code is valid,
but I'll emphasise it much more. Good catch.
The code samples in the "Stopping" section, I don't agree with. First the
Nitpicks:
You are using variables "stopping" and "stopped", and properties "Stopping"
and "Stopped". This naming convention shoud, I believe, be frowned upon.
Relying on case to differentiate members and properties is just asking for
trouble. It also violates the naming convention MS spells out in the MSDN
documentation for .NET.
It doesn't - the naming convention MS uses specifically doesn't mention
private members. It's not a problem for VB users because anyone outside
the class doesn't see the variables at all.
I've used the convention for years in both Java and C# and *never* had
a problem with it as far as I can remember. I find code written with my
convention significantly easier to use than code which uses underscores
all over the place - they just look horribly ugly to me, and disturb my
train of thought when I'm reading.
I'm afraid I'll stick with my convention as I do for all my code
samples.
I typically write this code as:
Private _stop As New Threading.ManualResetEvent(False)
Private _runningThread As Threading.Thread
Public Sub StartTheThread()
_stop.Reset()
_runningThread = New Threading.Thread(AddressOf MyThread)
_runningThread.Start()
End Sub
Public Sub StopTheThread()
_stop.Set()
_runningThread.Join()
End Sub
Private Sub MyThread()
While Not _stop.WaitOne(0, False)
' Do Work Here
End While
End Sub
I prefer this over your sample for a few reasons
1 - It is my belief (I have not benchmarked it, although I should) that
testing an event for Set/Not-Set is much cheaper than acquiring and
releasing a monitor (potentially) several millions times per second.
That's a heck of an assumption, in my view. Rather than making any
assumptions, I decided to test it. The main code in both my tests was
the same:
using System;
using System.Threading;
public class LockTest
{
static void Main()
{
Worker worker = new Worker();
new Thread(new ThreadStart(worker.Run)).Start();
Thread.Sleep(10000);
worker.Stop();
}
}
The worker object for my version is as on the website (except with
stopLock declared as object, not bool - oops! Fixed the website now)
but with a long counter variable which is just incremented in the loop
in the same way as with the code below, which is a slightly simplified
version of what you posted (just for the purposes of this test):
public class Worker
{
long counter=0;
ManualResetEvent stop = new ManualResetEvent(false);
public void Stop()
{
stop.Set();
}
public void Run()
{
DateTime start = DateTime.Now;
while (!stop.WaitOne(0, false))
{
counter++;
}
DateTime end = DateTime.Now;
Console.WriteLine
("Count per second: {0}", counter/(end-start).TotalSeconds);
}
}
Results (3 tests each):
Using a lock:
Count per second: 28038249.614792
Count per second: 28113891.7558685
Count per second: 27467033.4397496
Using a manual reset event:
Count per second: 1087057.6
Count per second: 1086289.82785602
Count per second: 1082087.3
So on average, using a lock we're able to count about 25 times as fast.
Just another example of threading not being a good thing to guess about
I'd be very interested to see whether you get the same kind of results
- it may *very* well depend to a very large extent on the CPU(s) of the
test machines. Mine is a P4/3.06GHz laptop; no hyperthreading or
anything like that. I'd be particularly interested to know the numbers
on a dual processor machine...
2 - If my thread loop doesn't have to run "as fast as it can", I can easilyl
change the to "While Not _stop.WaitOne(1000, False)" to make my loop run
once per second, while at the same time being responsive to shutdown events.
In your sample code, I would need to use a Sleep to slow down the loop,
which makes the thread unresponsive to shutting down.
Well, I'd probably use Monitor.Wait in that case, myself. I'm happier
with Monitor than reset events, but either would work fine here. I
wouldn't usually limit it artificially like that though - normally if
there's a reason to slow something down, that's because it hasn't got
work the whole time - and there I'd use a work queuing system. If I
want a thread to essentially be at a lower priority, I'll just lower
the priority. (Ah - another thing I haven't covered
3 - Your code is, in essece, creating a Manual Reset Event. Why create
something that's already there?
It's not, really. There's much more to events than just testing and
setting, as you've already shown with WaitOne - there are also calls to
wait for potentially many events at a time, etc. I believe testing a
boolean (even in a lock) is significantly simpler and easier to
understand than ManualResetEvents, which is why I stick with them in
general.
There were a few sections that I would have liked to see in there - mostly
these are questions that I've had for a while, and haven't ever been able to
get good ansers to..
I'll present quick answers here (to a couple of them), and see if there
are other comments in this thread (no pun intended). I'll then see
which bits are appropriate for inclusion in the article itself.
1 - Something that I've looked for information on, and found absolutly
nothing, is the "Synchronized" version of the various collection classes. I
have ended up having to roll my own collections in many cases, as I need to
access them from multiple threads (I am sooooo looking forward to
Generics!). With the lack of documentation on the synchronized collection
classes, I was never comfortable using them. What about them is thread safe?
I'm pretty sure Iteration is not (although it could be - the iterator could
grab a lock, and prevent other iterators from proceeding until it's
complete). Hashtables say (somewhere) that "one writer and multiple readers"
is thread safe, but I've never been quite sure of that. What about the other
collections?
I believe the idea of synchronized collections is that each individual
operation automatically takes out a lock, but that in order to
synchronize a whole series of operations, you need to lock on the
SyncRoot of the collection.
I've seen the same statement about multiple readers and a single writer
for hashtable, but I haven't tried to verify it with stress testing or
anything like that.
2 - Some of the Win32 threading constructs that people have been using for
so long are not present in .NET. Specifically Semaphores and (I think) Named
Events. Any idea why?
I really don't know. Semaphores are handy constructs, and fairly easily
built out of monitors (so long as you're careful
I may present a
sample semaphore class for those who are interested. I wouldn't be at
all surprised if they made it into .NET v2 anyway though.
3 - Some suggestions on how to start debugging threaded code. These could be
simple suggestions like "Give all your threads unique names, so that then
you're looking at the Threads window in the debugger, you can quickly tell
which thread is which", or complex suggestions like "Use SoS to determine
which thread is holding a lock".
Right, that's a good idea.
4 - You don't have a section on Events (either Manual or AutoReset). Along
with Monitors, these are the most used threading constructs.
Funnily enough, I've virtually never used them - except to build a
fuller Monitor implementation for the Compact Framework
I suspect it's my Java background showing through to some extent, but I
more naturally think in terms of monitors, and they tend to do
everything I need them to. I think they're generally simpler, which is
another reason I use them
(I do use Events when I want to wait for
multiple conditions etc.)
5 - ReaderWriterLocks are handy at times, but are probably beyond the scope
of your article.
Not sure - I think that's a very good idea. I haven't had to use them
myself, but almost certainly will at some time. I'll take a bit of a
breather and then probably add a section about them.
6 - Another advanded topic that I've seen little on, but seems as if it
could be really usefull, is the ability of the Threadpool to have a Handle
bound to it, and provide callbacks when this handle changed. Can I bind
standard wait handles to the thread pool so that I can callbacks whenever a
manual or auto-reset even is changed? It seems as if there would probably be
some fascinating use cases for this...
Mmm... I really don't know *anything* about that topic. Sounds like
something it would be worth investigating...
