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

From: Batov, Vladimir (Vladimir.Batov_at_[hidden])
Date: 2004-07-11 17:30:18

What a wonderfully clean suggestion of having just one lock and
(hopefully) just one mutex. IMHO having just one mutex class is even
more important than having one lock as mutex represents something
lockable that seems to always be the same. That is, I create a mutex and
sometimes want to apply a blocking lock on it and sometimes I just want
to try a lock. The functionality that implements various ways of locking
is in locks.

Although I can see the technical merits of having just one lock
constructor, I still feel that having two explicit instead would be less
ambiguous, friendlier towards the user and cleaner. Additionally, it'd
keep the door open for possible optimization. Additionally, I'd expect
the 

        scoped_lock l(m, true)

to be used much more often than

        scoped_lock l(m, false)

Consequently, IMHO the first should get the preferential treatment. That
is, to have an explicit and more efficient representation:

        scoped_lock::scoped_lock(Mutex&)  

Best,
V.

-----Original Message-----
From: boost-bounces_at_[hidden]
[mailto:boost-bounces_at_[hidden]] On Behalf Of Peter Dimov
Sent: Sunday, 11 July 2004 10:47 PM
To: boost_at_[hidden]
Subject: Re: [boost] Re: Boost.Threads: Do we need all those mutexes?

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().
};

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

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?

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:

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. 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).

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 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. It seems wrong (and the implementation seems buggy),
but
as usual, I may be missing something.

_______________________________________________
Unsubscribe & other changes:
http://lists.boost.org/mailman/listinfo.cgi/boost