Using ReaderWriterLock — are you getting what you think you’re getting?


Oh boy, more locking problems for the weekend!


Here’s a more complicated lock that often gets used when it shouldn’t and is avoided when it should be used.   Like the other articles in this series I’ll provide a bit of code and ask for comments, it’s more fun that way 🙂


OK, here’s the code:

    public class MixedUsers
{
private static System.Threading.ReaderWriterLock rw =
new System.Threading.ReaderWriterLock();

private static int val1 = 0;
private static int val2 = 0;

// this method is called by many threads
public static int ComputeSomethingUseful()
{
// disregarding timeout effects for now
rw.AcquireReaderLock(-1);

int result = val1 + val2;


rw.ReleaseReaderLock();

return result;
}

// this method is called by many threads
public static int UpdateUsefully(int v1, int v2)
{
rw.AcquireWriterLock(-1);

val1 += v1; // note coordination of updates
val2 += v2; // these two sums need to happen atomically

int result = val1 + val2;

rw.ReleaseWriterLock();

return result;
}
}


Now here are the things I want you to think about:


#1 Is this a good use of ReaderWriterLock?  What assumptions do you have to make about the frequency of the operations.


#2 If UpdateUsefully were the method that was called nearly always would you give the same answer?


#3 What if ComputeSomethingUseful were called almost exclusively instead, does that change your answer?


#4 Is there a different approach to solve this particular problem that might be more robust generally?


#5 What “tiny” change could I make in this problem that would make ReaderWriterLock virtually essential? 

Comments (14)

  1. To answer #1, there is no reason to EVER a good use of the ReaderWriterLock.

    My own personal tests (multiple threads, with varying percentages of concurrent read/writes) have shown it to be anywhere from 5-10x slower than using Monitor/lock.

    I believe Jeffery Richter pointed out some of the flaws as well, and has even implemented his own.

  2. Carlos says:

    1. No.  A ReaderWriterLock presupposes few writers and many readers, but even if this is true a ReaderWriterLock isn’t the best solution here.

    2. If UpdateUsefully is the most common method you might as well use a conventional lock (mutex), since there won’t be much contention amongst readers.

    3. If ComputeSomethingUseful is the most common method then it would be better to:

    4. cache the result calculated in UpdateUsefully in a volatile static member variable.  Since the result can be updated atomically you don’t need any reader lock.

    5. If the result couldn’t be precomputed (e.g. you’re calculating a*val1+val2 where a is a parameter to ComputeSomethingUseful) then a ReaderWriterLock would be appropriate.

  3. ricom says:

    Nicholas Paldino writes: “To answer #1, there is no reason to EVER a good [sic] use of the ReaderWriterLock” [emphasis in original]

    My experience has taught me that it is rarely possible to say anything absolute in the way of performance recommendations.  

    I won’t pretend to speak for Jeffery Richter, and I certainly won’t contest his results or yours.  However, I believe Mr. Richter would limit his conclusions as do I.

    It is unsurprising that ReaderWriterLock has raw performance that is worse than Monitor — it is after all a more complex beast.  It is lamentable that the existing implementation of ReaderWriterLock is not as good as we wish it was.  It is even more unfortunate that therefore we can’t use ReaderWriterLock in all the cases that we wish we could.

    However, a universal embargo on ReaderWriterLock, flaws and all, is not supportable.

  4. Vance Morrison says:

    While it is true that the current .NET implementation of a reader-writer lock is more expensive that you would like it to be, a reader writer lock does not inherentyly need to cost more than a standard lock.  See my post on just such implementation.  http://blogs.msdn.com/vancem/archive/2006/03/28/563180.aspx.  In the limit, the additional cost of a reader-writer lock over a normal lock is quite small (a few (cheap) instructions).  

    I happen to like reader-writer locks because they when they are useful (when readers dominate over writers), you get significantly more concurrency without complicating your design much at all (distiguishing read-only transactions from read-write ones).  

  5. Ken Overton says:

    Since this case deals with integer arithmetic, couldn’t we use Interlocked.Increment instead of locks?

  6. ricom says:

    We need some kind of locking because there are two resources (the integers) that need to be updated atomically.  We don’t want to observe half-updated states.

  7. Maurits says:

    UpdateUsefully could benefit from a rw.DowngradeToReaderLock() after the two += lines.

  8. Maurits says:

    Er, that is, a rw.DowngradeFromWriterLock()

  9. asqui says:

    What is the meaning of these two comments:

    val1 += v1; // note coordination of updates

    val2 += v2; // these two sums need to happen atomically

    Why do they need to happen automically — you’ve just aquired a writer lock which should mean that no-one else can access these variables (asuming, of course, that they’re using the same ReaderWriterLock to protect their use of these variables).

  10. ricom says:

    I only wanted to emphasize that there were two variables needing coordination and we didn’t want clients to observe one updated and the other not updated.

    The writer lock accomplishes this.

  11. asqui says:

    Ah okay… you were referring to the *pair* of sums hapenning atomically (sorry for the horrible misspelling of "atomically" in my original post).

    I thought you were referring to the atomicity of the increment itself (load, increment, write).

  12. Stan says:

    It isn’t the best example because the same result is calculated in both methods. As another person commented why not cache the result of the calculation in a volatile variable. That is especially true if ComputeSomethingUseful is called very often.

    If we can’t cache the result for whatever reason, at least the calculation part should be moved outside the lock scope. I’d copy the values of protected static variables to local variables on the stack; then release the lock and finally calculate the result.

  13. Well the trouble with simple samples like the one I provided in part 1 is that well… they’re too simple.

  14. Well I think it’s raining ReaderWriterLocks this month! Jeff Richter has an article on MSDN that…