|
|
Threads-Devel : |
Subject: Re: [Threads-devel] Extend shared_mutex with support for priority policies?
From: Fredrik Orderud (forderud_at_[hidden])
Date: 2012-12-30 15:25:29
On Thu, Dec 20, 2012 at 11:44 PM, Vicente J. Botet Escriba <
vicente.botet_at_[hidden]> wrote:
> Le 20/12/12 17:24, Fredrik Orderud a écrit :
>
> 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.
>
> This is a good starting point. Do you expect that shared_pri_mutex<
> UNSPECIFIED_PRIORITY> behaves as shared_mutex?
>
Yes, that was the idea. Note that my internal state_data struct does differ
a bit from the ones in the win32 & pthread shared_mutex implementations,
since I couldn't fully understand them. In particular, I've extended the
1-bit/bool exclusive_waiting_blocked flag into an exclusive_request_count
counter.
Questions:
* Are you aware of any documentation for the shared_mutex locking
algorithm? In particular, I'm curious about the precise workings of
exclusive_waiting_blocked.
* Why does state_data differ so much between the pthread and win32
implementations? In particular, why are the
shared_waiting/exclusive_waiting counters only present in the win32 impl.?
Does this mean that win32 vs. pthread shared_mutex behaves differently?
* The win32 imp. appears to be "better" in that it's lockless (except for
semaphores), whereas pthread has an internal mutex protecting all methods
(with condition variables). Are there any specific reasons why the pthread
imp. isn't also lockless?
I think it could be beneficial to first take some steps towards
unifying/deduplicating the shared_mutex implementations before adding more
"priority policy" complexity into the mix.
> 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.
>
Agree.
> 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.
>
Agree. I just need some time to dig into the details on the existing
implementations.
> Anyway, for the time been I have some 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.
>
Fully agree with you comments. Consider them "done". :-)
> Maybe it is worth creating a Trac ticket.
>
That might be, but I will first need to spend some time first to implement
a proper proposal that also incorporates the "upgrade" aspect. This will
take some time.
Thanks for all the constructive comments Vicente.
Regards,
Fredrik