Re: [Threads-devel] Extend shared_mutex with support for priority policies?

Subject: Re: [Threads-devel] Extend shared_mutex with support for priority policies?
From: Vicente J. Botet Escriba (vicente.botet_at_[hidden])
Date: 2012-12-20 17:44:56

Le 20/12/12 17:24, Fredrik Orderud a écrit :
> On Mon, Dec 17, 2012 at 9:38 PM, Vicente J. Botet Escriba 
> <vicente.botet_at_[hidden] <mailto:vicente.botet_at_[hidden]>> wrote:
>
>     Le 15/12/12 19:16, Fredrik Orderud a écrit :
>
>         The current "fair" shared_mutex implementation is great in the
>         sense that it neither favors readers nor writers. This is
>         probably a good property in most situations.
>
>         However, there are situations where either reader or writer
>         prioritization can be beneficial. In particular, I'm working
>         on a project where the locking could be significantly stricter
>         without risking introducing deadlocks if read-locks could be
>         guaranteed to have priority over write-locks. I fully
>         understand that a read- or write-priority implementation might
>         have a bigger overhead compared to the existing "fair"
>         shared_mutex implementation, but that is unlikely to be a
>         problem for me.
>
>         Is anyone in the boost community experiencing similar problems
>         and/or is there any interest in extending shared_mutex with
>         support for different priority policies?
>
>     I think that it will be better to create specific shared mutex
>     classes with read/write/fifo priorities. The interface of these
>     classes must mimic boost::shared_mutex so that we are able to use
>     them with the shared_lock and upgrade_lock.
>
>     I have no time to work on this now but if you provide a first
>     implementation I could help you to finish it so that it can be
>     introduced in Boost.Thread.
>
>
>  Thank you Vicente.
>
> Attached is an early "draft" implementation of "shared_pri_mutex" with 
> support for the following prioritization policies: 
> UNSPECIFIED_PRIORITY, EXCLUSIVE_PRIORITY and SHARED_PRIORITY. 
> Associated test code is attached in main.cpp. The implementation is 
> based on a stripped-down version of 
> boost/thread/pthread/shared_mutex.hpp, and implements policies through 
> an enum template argument.
Hi,

this is a good starting point. Do you expect that shared_pri_mutex< 
UNSPECIFIED_PRIORITY> behaves as shared_mutex?
> The policies could probably also be implemented with a class hierarchy 
> if preferred.
The enum seems ok to me, but maybe, having specific classes at the user 
level could introduce less constraints: shared_mutex, 
shared_mutex_prio_shared, , shared_mutex_prio_exclusive. This doesn't 
means that the implementation could not use a unique implementation as 
far as there is no loss on performances.
> For simplicity, neither upgrade-locking nor lock_for/until calls are 
> yet implemented.
I understand it as probe of concept.

> FIFO-prioritization have also been skipped, since I suspect it would 
> require a more advanced "queue" datastructure with higher complexity.
Yes, fifo priority needs a different implementation. This could be done 
later on.
>
> Could you (or someone else) please provide some feedback on the 
> "shared_pri_mutex" concept, and realism of getting a revised/extended 
> implementation incorporated into boost::thread at some point?
I would prefer to integrate into Boost an implementation that takes in 
account upgrade-locking and timed operations. As soon as we have a 
complete and correct implementation, with enough test and documentation 
we could integrate them in a release. The documentation will need a 
motivation, why do we need them? If you can work on some of these 
subjects the integration could be do sooner.

Anyway, for the time been I have somme minor remarks:

* the enum literals should be in lowercase and I would prefer a enum class.

     //enum class shared_mutex_priority
     BOOST_SCOPED_ENUM_DECLARE_BEGIN(shared_mutex_priority)
     {
         unspecified,
         exclusive,
         shared
     }

* mutex are not copiable

         BOOST_THREAD_NO_COPYABLE(shared_pri_mutex)

* protect the use of interruptions with 
BOOST_THREAD_PROVIDES_INTERRUPTIONS as in trunk

#if defined BOOST_THREAD_PROVIDES_INTERRUPTIONS
             boost::this_thread::disable_interruption do_not_disturb;
#endif

* boost::mutex::scoped_lock is deprecated
Use
             boost::unique_lock<boost::mutex> lk(state_change);

instead of

              boost::mutex::scoped_lock lk(state_change);

as in trunk.

Maybe it is worth creating a Trac ticket.

I would be able to take a deeper look beginning of next year.

Thanks for working in this proposal.

Best,
Vicente