Locking -- Isolation -- Unit of Work -- Performance
I've had a bit of a locking/threading theme in some of my recent postings so I thought I'd continue that with a little problem. Here's a (simplified) set of operations that is sort of an example of what you might need to do in a free threaded class. Let's assume there is one instance of this class that is to be shared between multiple threads any of which might call the DoIt() function at any time.
Here are two possible implementations that are like things I've seen in the past. The question is: Which of these is "better?" Is there something that is (in some sense) better still? What are the factors that influence this decision (Hint: see the title of this posting)
You might have to scroll down to see the code depending on your browser because the <PRE> formatting seems to come out weird with the stylesheets in use here.
// it's easy to explain what this does
class ExampleCoarse
{
public ExampleCoarse() {}
private Object myLock = new Object();
private int count;
public void DoIt(StreamWriter sw)
{
lock(myLock)
{
long ticks = DateTime.Now.Ticks;
int count = IncrementCount();
LogToStream(sw, ticks, count);
}
}
private int IncrementCount()
{
return ++count;
}
private void LogToSteam(StreamWriter sw, long ticks, int count)
{
sw.WriteLine("{0} {1}", ticks, count);
}
}
// it's not so easy to explain this... or how to use it.. but is it better?
class ExampleFine
{
public ExampleFine() {}
private Object myLock = new Object();
private int count;
public void DoIt(StreamWriter sw)
{
long ticks = DateTime.Now.Ticks;
int count = IncrementCount();
LogToStream(sw, ticks, count);
}
private int IncrementCount()
{
int c = System.Threading.Interlocked.Increment(count);
return c;
}
private void LogToSteam(StreamWriter sw, long ticks, int count)
{
lock (myLock)
{
sw.WriteLine("{0} {1}", ticks, count);
}
}
}
Comments
- Anonymous
April 24, 2006
The comment has been removed - Anonymous
April 25, 2006
The comment has been removed - Anonymous
April 25, 2006
The comment has been removed - Anonymous
April 25, 2006
Scott writes:
>>I don't know if it was your intent with this example, but it occurs to me that trying to synchronize access to the externally provided StreamWriter instance is incorrect.
Critical observation there.
And what else do we not know along those lines?
(Btw this is basically an example I saw "in the wild", I just simplified it) - Anonymous
April 25, 2006
You probably have to look at the purpose behind that kind of logging.
Do you need a strictly increasing serial number (regardless of whether the output streams are different), or would a unique ID do just as well? In the latter case, you can have thread-private data that lets you manage a chunk of IDs per thread (thread 1 gets IDs 0-999, thread 2 gets IDs 1000-1999, etc.). Then you only need locking when you must grab a new chunk of IDs; the version with Interlocked.Increment() freezes the memory bus for all the other processors while the counter is being incremented.
Accessing the stream is way more expensive than bumping a counter. Can you buffer the information to be logged (i.e. store the <counter, timestamp, stream> tuples somewhere) and write to the streams later? - Anonymous
April 25, 2006
"And what else do we not know along those lines?"
We don't know who's responsible for instantiating the classes. If you have multiple instances the locks are useless. They should be implemented with a singleton pattern (and with the StreamWriter moved into the class as mentioned above.) - Anonymous
April 25, 2006
The fastest locking is no locking at all. To get maximum speed we could make the data thread local with the [ThreadStatic] attribute. If the counter is meant to be AppDomain global then it should be put into a static class and not in an instance one. Thread thrashing should be avoided if you only want to print out the current time and a number which does not serve a real purpose but degrades performance. If you eliminate the counter and leave only the time formatting then you should be done with a static function.
public static void DoIt(StreamWriter sw)
{
long ticks = DateTime.Now.Ticks;
sw.WriteLine("{0} ", ticks);
}
By the way my performance contest to format the time in the most performant way did lead to some interesting results: http://geekswithblogs.net/akraus1/archive/2006/04/23/76146.aspx
The coarse and fine grained examples should not differ so much in terms of performance since the formatting takes quite some time. Although I would vote for the second fine grained example to be a little more performant since it does block for a shorter time which does reduce thread contention.
Yours,
Alois Kraus - Anonymous
April 25, 2006
Oh and I forgot to mention that if the writing to the stream is slow and highly concurrent you should definitely check out its asynchronous version BeginWrite to benefit from the .NET thread pool to get a scalable solution.
Yours,
Alois Kraus - Anonymous
April 25, 2006
I think the ultimate answer to the question is, how exactly does this work in a practical scenario? Both of your examples have pros and cons, depending on how they are used and what the expectations are. Both also have flaws that could be corrected and make them better performing and more useful for different situations.
Locking an external resource (in this case the StreamWriter) seems like a bad idea, as you don't know where it came from or who else might already be locking it (which leads to the question how long will they hold the lock and degrade the performance of your logger). There are no guarantees that each time DoIt() is called that the stream will be the same as other calls, which was also mentioned previously.
(Which makes me wonder, lock() is an object-specific call, is it not? If two different streams are passed in to DoIt() from different callers, lock will not block if its called on a different object the second time, if memory serves. If that is truely the case, then I think that changes the scenario quite a bit.) - Anonymous
April 26, 2006
A few days ago I posted a concurrency problem for commentary and I got a very nice set of responses... - Anonymous
May 20, 2006
This is just a trivial one.
Interlocked increment takes an int as ref parameter which is missing in your sample code.
int c = System.Threading.Interlocked.Increment(count);