Re: [Threads-devel] RFC: 1st pthread shared_mutex refactoring patch

Subject: Re: [Threads-devel] RFC: 1st pthread shared_mutex refactoring patch
From: Fredrik Orderud (forderud_at_[hidden])
Date: 2013-01-10 17:47:23

On Mon, Jan 7, 2013 at 11:47 PM, Vicente J. Botet Escriba <
vicente.botet_at_[hidden]> wrote:

> Le 07/01/13 20:32, Fredrik Orderud a écrit :
>
>  The attached patch is intended as a 1st step towards separation of the
>> "locking policy" parts from the "synchronization" parts of shared_mutex.
>> This separation is achieved by moving state_data member checks and
>> manipulation into new methods in state_data. The separation is currently
>> only done for exclusive and shared locking, but I will complete the
>> separation to also cover upgrade locking if there is consensus that this is
>> a good strategy. state_data can then be encapsulated with all data members
>> private.
>>
>> Advantages of the separation:
>> * Simplify implementation of different locking prioritization policies:
>> Only requires reimplementation of state_data, with (hopefully) no changes
>> to the outer shared_mutex class.
>> * Simplify implementation of "lock-free" synchronization (as in win32
>> counterpart): Can be done in shared_mutex without touching the "locking
>> policy" logic in state_data.
>>
>> Any comments or feedback will be highly appreciated.
>>
>>
>>  Thanks Fredrik for the patch. I think It goes on the good direction. Do
> you mind to finish the upgrade part before I apply it?
>

Attached is an updated patch that also updates the upgrade part. All
boost::thread unit-tests still pass, and I think the changes makes
shared_mutex easier to understand.

On first sight, it might appear like some of the upgrade-related methods
becomes less efficient due to state_data members being modified several
times redundantly (e.g. in try_unlock_shared_and_lock_upgrade), but that is
something the compiler should be able to optimize away. Also, I might have
introduced a slight change in behavior in unlock_upgrade_and_lock, due to
state.upgrade now being reset before upgrade_cond.wait(lk) (as opposed to
before), but my impression is that it shouldn't matter.

Please review the patch, and let me know if you have any questions.

Thanks in advance,
Fredrik