Re: [Threads-devel] v2/shared_mutex bitmask refactoring

Subject: Re: [Threads-devel] v2/shared_mutex bitmask refactoring
From: Vicente J. Botet Escriba (vicente.botet_at_[hidden])
Date: 2013-03-26 00:57:01

Le 25/03/13 13:53, Fredrik Orderud a écrit :
> The "bitmask" manipulation in v2/shared_mutex and v2/upgrade_mutex 
> makes the code difficult to understand. The attached patch attempts to 
> replace the bitmasks with more manageable "bitfields". This leads to 
> shorter and more readable code with equivalent performance.
>
> All "thread" tests still pass on my Win7/x64 computer with 
> v2/shared_mutex.hpp manually included from thread/shared_mutex.hpp.
>
> If the patch is appreciated, then I can also send a similar patch for 
> v2/upgrade_mutex.
>
Hi,

I appreciate your efforts going to a more understandable code. I don't 
know why HH has written the code like that, but usually he has good 
reasons.

In order to ensure the same performance, what about a patch using 
getters/setters instead of the direct use of the bitfields, and define 
these functions either using the current code either using the bitfields 
so that we can have both and compare easily.

I wonder if this is not portable if unsigned is not 32 bits.

+      static const count_t N_READERS_MAX = 0x7FFFFFFF;

BTW,

do you understand why there is a test in (*)?

     void
     upgrade_mutex::unlock_shared()
     {
       boost::lock_guard<mutex_t> _(mut_);
       count_t num_readers = (state_ & n_readers_) - 1;
       state_ &= ~n_readers_;
       state_ |= num_readers;
       if (state_ & write_entered_)
       {
         if (num_readers == 0)
           gate2_.notify_one();
       }
       else
       {
         if (num_readers == n_readers_ - 1) // (*)
           gate1_.notify_one();
       }
     }

Best,
Vicente