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

From: Michael Glassford (glassfordm_at_[hidden])
Date: 2004-07-07 23:23:07

Peter Dimov wrote:
> Michael Glassford wrote:
> 
>>Peter Dimov wrote:
> 
> 
> [...]
> 
> 
>>>This is certainly possible, but I don't see what the additional
>>>complexity buys us.
>>>
>>>    TryLock l( m, false );
>>>
>>>    if( l.try_lock() )
>>>    {
>>>    }
>>>
>>>looks acceptable to me.
>>>
>>>    TryLock l( m, non_blocking );
>>>
>>>    if( l.locked() )
>>>    {
>>>    }
>>>
>>>doesn't seem much of an improvement.
>>
>>The two aren't equivalent (I'm not sure if you meant them to be): in
>>your first example the constructor doesn't attempt to lock, and your
>>second example it attempts a non-blocking lock.
> 
> 
> I'm not sure where you think the two examples differ. Both attempt a
> non-blocking lock.

It doesn't really matter, but if you're talking about the way
Boost.Threads works currently (in the 1.31.0 release), according to both
the code and the documentation, TryLock(m, false) makes no attempt to
lock. If you're talking about a new design, then of course it means
whatever you want :)

That's one reason I prefer the enum: it's clear what the parameter
actually means.

>>2) What it buys over your suggestion (at the cost of complexity):
>>
>>* The ability to specify all applicable lock types (blocking,
>>non-blocking, timed-blocking) in a constructor.
> 
> 
> Why is that an improvement?

It seems reasonable that, if you're going to allow constructors to lock,
you should allow them to lock in whatever ways are possible. I guess I
don't feel that strongly about the issue, though; if a single
constructor is strongly preferred by others, I guess that's OK with me.
However, the constructors I suggested, especially as simplified by
Howard, don't seem all that complicated to me. Also, see my comment below.

>>3) I'm proposing the use of enums rather than bools to indicate the
>>initial state of the lock for these reasons: [...]
> 
> 
> OK, let's leave the enum vs bool distinction aside for a while. The crucial
> difference is explicit try_lock() vs implicit try_lock (a constructor with
> the appropriate combination of arguments).

OK. I do generally like the idea of simplifying, and a single
constructor is certainly simpler.

The constructor locking is kind of nice for this syntax, though:

    if (try_lock l(...)) //or timed_lock
    {
       //do something
    }

>>4) Which leads to a problem with both your proposal and mine: they
>>both silently break existing code (by changing try_lock(M) from
>>non-blocking
>>to blocking); at least, I presume you meant that. Would eliminating
>>the lock_type(M) constructor (perhaps only for a release or two) be a
>>good
>>idea, or is a prominent warning in the documentation enough?
> 
> 
> My proposal (I'm not sure whether I should be credited for it; Howard posted
> locks that had these properties and I merely commented) can be made to break
> such code at compile time, simply by removing the nested
> TryMutex::scoped_try_lock typedef (as there is no longer a need for it,
> because ::scoped_lock is a TryLock or TimedLock, as appropriate).

OK. I have to admit I didn't notice this part of the design. I'll have
to go back and look again.

Mike