Re: [boost] [thread] On shared_mutex

Subject: Re: [boost] [thread] On shared_mutex
From: Anthony Williams (anthony.ajw_at_[hidden])
Date: 2010-11-29 05:05:22

Howard Hinnant <howard.hinnant_at_[hidden]> writes:

> Three years ago I wrote N2406 "Mutex, Lock, Condition Variable
> Rationale"
> (http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2406.html)
> for the C++ committee in an attempt to explain the combined proposed
> std::mutex/std::unique_lock package and how it fit together with the
> tr2-targeted shared_mutex/shared_lock package.  This paper also
> proposed an upgrade_mutex and upgrade_lock.
>
> Since that time, the std-proposed stuff has been accepted with some
> name changes, and a reworking of the timed-locking interface.
> Additionally Anthony Williams has implemented much of the
> shared-locking functionality in boost (and done a very nice job with
> it).

Thanks Howard.

> That being said, I disagree with some fairly major design changes
> between N2406 and what is now in the boost library.  

> 4.  boost allows implicit conversions between the lock types.  N2406
> made these conversions explicit.  Rationale: changing the ownership
> mode of a mutex is something that should be well documented in the
> code.

If it does, this is a bug --- the conversions are supposed to require
explicit move operations.

> I have an updated implementation of <shared_mutex> (under the boost
> license) here:
>
> http://home.roadrunner.com/~hinnant/mutexes/shared_mutex
> http://home.roadrunner.com/~hinnant/mutexes/shared_mutex.cpp
>
> There is a tutorial-style description of this library here:
>
> http://home.roadrunner.com/~hinnant/mutexes/locking.html

Firstly, thank you for providing an updated implementation and rationale
under the BSL.

Secondly, though I have some comments on the design choices (which I
will outline below), I am less convinced of the utility of the
shared_mutex functionality in its entirety, and quite glad we didn't
standardize it. Before commenting on the design choices, I will attempt
to outline why I am less convinced of its utility overall.

The cost of locking a shared_mutex is higher than that of locking a
plain std::mutex, even for the reader threads. This is a necessary part
of the functionality --- there are more possible states of a
shared_mutex than a mutex, and the code must handle them correctly. This
cost comes in both the size of the object (which in both your
implementation and my POSIX implementation includes both a plain mutex
and a condition variable), and in the performance of the lock and unlock
operations.

Also, the shared_mutex is a point of contention, and thus not
scalable. Locking a shared_mutex necessarily modifies the state of the
mutex, even for a read lock. Consequently, the cache line holding the
shared_mutex state must be transferred to whichever processor is
performing a lock or unlock operation.

If you have a lot of threads performing frequent, short read operations,
then on a multiprocessor system this can lead to a lot of cache
ping-pong, which will considerably impact the performance of the
system. In this case, you may as well adopt the simpler design of just
using a plain mutex, as the readers are essentially serialized anyway.

If the reads are not frequent, then there is no contention, so you don't
need to worry about concurrent readers, and a plain mutex will suffice
for that scenario anyway.

If the read operations are time consuming, then the consequence of this
contention is less visible, since it is dwarfed by the time spent whilst
holding the read lock. However, performing time consuming operations
whilst holding a lock is a design smell.

In the vast majority of cases, I think that there are better
alternatives to a shared_mutex. These may be a plain mutex, the atomic
support of shared_ptr, the use of a carefully constructed concurrent
container, or something else, depending on context.

Now for the design comments.

I can see why you chose to separate shared_mutex and upgrade_mutex both
in the original design and this one, but I felt that the benefits were
not worth the proliferation of mutex types. I'm not so strongly attached
to that feeling as I was, so you may yet convince me ;-)

However, the omission of shared_lock -> upgrade_lock conversion was
deliberate. Just as you clearly feel that shared_mutex and upgrade_mutex
deserve to be distinct types, I feel that a shared_lock should always
remain a shared_lock. This is why upgrade_lock is a distinct type. If
you allow a shared_lock -> upgrade_lock transition then you might as
well allow shared_lock -> unique_lock and omit upgrade_lock
entirely. I see from the diagram that you actually consider the lack of
a conversion from shared_lock -> unique_lock a problem; I consider it a
feature.

There is essentially no difference between a blocking lock and a
try_lock_for(std::chrono::seconds(1000000000)), or repeated calls to
try_lock in a loop. I think such operations should therefore be omitted
in the cases that a blocking lock is not provided.

On the other hand. the omission of the timed operations where the
blocking operation is provided was merely that: an omission. Some of the
operations are present in boost on either POSIX or Windows but not both
--- they should probably all be provided on both platforms.

Anthony
-- 
Author of C++ Concurrency in Action     http://www.stdthread.co.uk/book/
just::thread C++0x thread library             http://www.stdthread.co.uk
Just Software Solutions Ltd       http://www.justsoftwaresolutions.co.uk
15 Carrallack Mews, St Just, Cornwall, TR19 7UL, UK. Company No. 5478976