Share via

Using ReaderWriterLock -- are you getting what you think you're getting?

  • Article

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

  • Anonymous
    May 05, 2006
    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.
  • Anonymous
    May 05, 2006
  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.
  • Anonymous
    May 05, 2006
    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.
  • Anonymous
    May 05, 2006
    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).  
  • Anonymous
    May 09, 2006
    Since this case deals with integer arithmetic, couldn't we use Interlocked.Increment instead of locks?
  • Anonymous
    May 09, 2006
    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.
  • Anonymous
    May 09, 2006
    UpdateUsefully could benefit from a rw.DowngradeToReaderLock() after the two += lines.
  • Anonymous
    May 09, 2006
    Er, that is, a rw.DowngradeFromWriterLock()
  • Anonymous
    May 10, 2006
    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).
  • Anonymous
    May 10, 2006
    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.
  • Anonymous
    May 10, 2006
    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).
  • Anonymous
    May 11, 2006
    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.
  • Anonymous
    May 12, 2006
    Well the trouble with simple samples like the one I provided in part 1 is that well... they're too simple.
  • Anonymous
    May 15, 2006
    Well I think it's raining ReaderWriterLocks this month! Jeff Richter has an article on MSDN that...