[boost] Re: Boost.Threads: Do we need all those mutexes?

From: Michael Glassford (glassfordm_at_[hidden])
Date: 2004-07-12 08:31:44

Peter Dimov wrote:

> Michael Glassford wrote:
> 
>>Peter Dimov wrote:
>>
>>>After giving it some thought:
>>>
>>>- one lock class: tie;
>>
>>This seems both popular and reasonable. I presume it would be
>>templated on the mutex type?
> 
> 
> Seems entirely reasonable. Perhaps it's time to move to formal wording.
> 
> template<class M> class scoped_lock
> {
> public:
> 
>     explicit scoped_lock( M & m, bool l = true );
>     // effects: if( l ) m.lock();
>     // post: locked() == l && mutex() == &m;
> 
>     ~scoped_lock();
>     // effects: if( locked() ) mutex()->unlock();
> 
>     void lock();
>     // throws: lock_aready_locked if locked();
>     // effects: mutex()->lock();
>     // post: locked();
> 
>     bool try_lock();
>     // throws: lock_aready_locked if locked();
>     // returns: mutex()->try_lock();
>     // post: locked() == (return value);
> 
>     bool timed_lock( xtime xt );
>     // throws: lock_aready_locked if locked();
>     // returns: mutex()->timed_lock( xt );
>     // post: locked() == (return value);
> 
>     void unlock();
>     // throws: lock_not_locked when !locked();
>     // effects: mutex()->unlock();
>     // post: !locked();
> 
>     bool locked() const;
> 
>     Mutex * mutex() const;
>     // returns: the associated mutex;
> 
>     operator unspecified-bool-type() const;
>     // returns: locked().
> };

Yes, this is much what I had in mind. The mutex type(s) would continue 
to supply a typedef, I presume (but only one):

     class some_mutex_type
     {
     public:

         typedef lock<some_mutex_type> scoped_lock;

         //...
     };

Also, although there may only be one lock class, there are still three 
lock concepts specifying which lock operations are supported by each of 
the three mutex concepts, right? (Unless we actually do combine all of 
the mutex types into one as well.)

> I've added the mutex() accessor to support Andrei's lock idiom:

OK.

> void f()
> {
>     scoped_lock lock( my_mutex );
>     f( lock );
> }
> 
> void f( scoped_lock & lock )
> {
>     // check for lock validity
>     assert( lock.locked() && lock.mutex() == &my_mutex );
> 
>     // proceed with operation
> }
> 
> Now that we got rid of the excess locks, how about doing the same with the
> mutexes?

I had considered this, but there does seem to be some benefit in having 
separate mutex types, which is what I assume led to there being three 
mutex types and three lock types in the original design. You've noted 
these reasons below, but I'll reiterate:

* On pthreads, the timed mutex requires an additional data member, 
(condition variable) to handle cases when pthreads_timedlock isn't 
supported.

* On WinNT, the timed mutex operations require a win32 mutex object, 
while the mutex and try mutex can use a win32 critical section.

* On Win9x, the timed mutex and try mutex operations require a win32 
mutex object, while the mutex can use a win32 critical section.

In other words, on the most widely-used platforms, collapsing the mutex 
types into one imposes some penalty (larger mutex object or more limited 
implementation options) on users. Also (I ask, not as a hypothetical 
question, but as a request for information): is there another platform 
where combining the mutex types incurs a similar or worse penalty?

> class mutex // DefaultConstructible
> {
> private:
> 
>     void lock();
>     // pre: *this is not locked by the current thread
>     // effects: blocks until *this is unlocked
>     // post: *this is locked by the current thread
> 
>     bool try_lock();
>     // returns: true iff *this was unlocked
>     // post: if *this was unlocked, *this is now locked by the current
> thread
> 
>     bool timed_lock( xtime xt );
>     // returns: true if *this is now locked by the current thread
>     // effects: if *this is unlocked, returns immediately, otherwise blocks
>     //             until either *this is unlocked or xt has been reached
>     // post: if *this was or became unlocked, *this is now locked by the
>     //         current thread
> 
>     void unlock();
> 
>     // pre: *this is locked by the current thread
>     // post: *this is unlocked
> };
> 
> The main potential issue here is mutex::timed_lock; try_lock doesn't seem
> controversial. It is true that Windows 95 does not have
> TryEnterCriticalSection, but an implementation can use Alexander Terekhov's
> alternative:

The Boost.Threads try_mutex in CVS currently checks if 
TryEnterCriticalSection is available and uses it if it is. To me this 
seems a reasonable implementation for now, and if it proves not to be 
good enough, a TryEnterCriticalSection facsimile can be implemented for 
Win9x platforms, as you say.

> http://lists.boost.org/MailArchives/boost/msg64648.php
> 
> The mutex::timed_lock situation is a bit more complicated. POSIX labels
> pthread_mutex_timedlock as part of the Timeouts option, and I don't know
> whether this is because many implementations deliberatley do not provide
> timed locks, or because this is a relatively new addition to pthreads. Does
> someone know?
> 
> My not-so-informed opinion at the moment is that we should provide
> timed_lock. I see that the current Boost.Threads implementation uses a mutex
> and a condition variable to implement a timed_lock-capable mutex, so we have
> a proof of concept that it can always be done, however there may be
> efficiency concerns. 

OK.

 > My line of thought is that since a mutex (usually) has
> a fast path user space portion and a slow path kernel space portion, with
> the timed_lock baggage not affecting the fast path, it seems reasonable to
> always require timed_lock (remember we're in "next standard mode" now,
> thinking severals years ahead).

I think I'm being dense, but I'm not sure I understand what you mean by 
"require timed_lock" (require platform support for it?) or what this 
part of the paragraph is getting at.

> Thoughts?
> 
> Some more random remarks regarding the state of Boost.Threads. Several files
> still seem to have incorrect line endings. This usually happens when
> checking in Windows line endings with a Unix CVS client. Please don't do
> that.

I don't. I typically use the VC++ 7.1 editor and TortoiseCVS. I seem to 
remember a problem like this being discussed in the Spirit mailing list 
with an earlier TortoiseCVS, but I'm using the latest version that was 
supposed to have fixed that problem.

> I was somewhat surprised by the mutex/cv implementation. I expected a thin
> wrapper over pthreads, but this is not the case. At first sight it seems
> that the complexity is caused by the fact that boost::condition supports
> recursive mutexes that are locked more than once, whereas POSIX does not. I
> do not recall any discussions about this issue, and I'm not sure why this
> decision was made. 

I can't comment on this since I haven't yet really looked at the 
condition variable implementation and had nothing to do with the 
original decisions.

 > It seems wrong (and the implementation seems buggy), but
> as usual, I may be missing something.

Alexander Terekhov has mentioned one problem with the Win32 condition 
variable implementation that I recall (I have the link somewhere but 
don't see it at the moment); do you have others in mind?

Mike