[boost] Re: Lock unification [move]

From: Michael Glassford (glassfordm_at_[hidden])
Date: 2004-07-22 08:41:50

Apologies that some of this has fallen behind the current discussion: I 
tried to post it last night, but got errors, and am posting it again 
today unchanged.

Mike

Howard Hinnant wrote:
> Thanks everyone for your comments and patience.  There's a new spec up at:

Sorry I haven't had time to read and comment as well due to the upcoming 
release; my comments here are pretty brief as well. I hope to be able to 
do better soon.

> http://home.twcny.rr.com/hinnant/cpp_extensions/threads.html
> 
> I've tried to incorporate those comments I've agreed with, and explain 
> my disagreements below.  Further discussion would be much appreciated.  
> I feel there has been dramatic improvements in this first iteration of 
> the spec.  Thanks again!
> 
> On Jul 18, 2004, at 6:41 PM, Batov, Vladimir wrote:
> 
>> 1.    explicit scoped_lock(mutex_type& m);
>> 2.    scoped_lock(mutex_type& m, locking_enum lock_it);
>> 3.    scoped_lock(mutex_type& m, blocking_enum block_it);
>> 4.    scoped_lock(mutex_type& m, const elapsed_time& elps_time);
>>
>> Do we need (3)? Isn't is covered by (4) with elps_time = INFINITE
>> (blocking) and - 0 (non blocking)? If it is intended as a "convenience"
>> interface, my expectation would be that average users would use thin and
>> more explicit wrappers around the general lock. Like try_lock and
>> timed_lock. Then, those thin wrappers would address the "convenience"
>> issue better.
> 
> 
> I agree that a timed lock collapses to non-locking and non-blocking on 
> the extremes, but I don't think it is a good idea to collapse the 
> implementations or the signatures.  Separate constructors allows a clean 
> separation of ideas, and also allows for the fact that the underlying 
> mutex may well support try_lock (for example), but not timed_lock.

I agree with your disagreement.

> However, I've modified the signatures of the enum-based constructors 
> (read on).
> 
>> (1) and (2) overlap as (1) is (2) with lock_it = true. Does not it mean
>> that some functionality will have to be duplicated in (1) and (2)? How
>> about avoiding the overlap with
>>
>> 1.    explicit scoped_lock(mutex_type& m);
>> 2.    scoped_lock(mutex_type& m, deferred_t);
>>
>> Then, every constructor would do just one thing.
> 
> 
> I agree 100%, though I've spelled deferred_t as defer_lock_type and 
> added try_lock_type.  Thanks.  You can now:
> 
> scoped_lock lk1(m, defer_lock);  // not locked
> scoped_lock lk2(m, try_lock);    // tries to lock

If you remember, I proposed something along these lines (at the 
suggestion of Vladimir, IIRC) and liked it a lot, but changed my mind 
when it was pointed out that it prevents making the choice at run time 
whether the lock should be locked or not. With movable locks, as you 
point out below, this is changed somewhat.

> On Jul 19, 2004, at 6:55 AM, Bronek Kozicki wrote:
> 
>> * do we need distinction between "write_lock" and "scoped_lock"? After
>> all, both lock are:
>>   1. exclusive (in regard to ownership of "mutex" object)
>>   2. moveable (ie. have move semantics instead of copy semantics)
>>
>> write_lock delivers some superset of functionality that scoped_lock does
>> not have, but do we need separate class for this functionality? After
>> all "if the client of scoped_lock<Mutex> does not use funtionality which
>> the Mutex does not supply, no harm is done". I understand that under
>> current proposal mutex class may deliver three separate families of lock
>> functions : lock, read_lock and write_lock, but I want you to consider
>> just two: lock (== exclusive == write) and shared_lock (== read). I
>> suggest that scoped_lock (another possible name "exclusive_lock") would
>> be move-constructible also from upgradable_shared_lock. If Mutex class
>> does not deliver member function
>> unlock_upgradable_shared_lock_exclusive , we would have compilation
>> error. Otherwise, no harm is done. The same applies to move-assignment
>> from upgradable_shared_lock (hm, some shorter name please?)
> 
> 
> Good comments.  Here's what I've done in response:
> 
> scoped_lock  |----------------> scoped_lock
> write_lock   |
> read_lock  -------------------> sharable_lock
> upgradable_read_lock ---------> upgradable_lock

At first glance, this looks good to me.

>> * I do not quite like
>> "write_lock(try_rvalue_ref<upgradable_read_lock<mutex_type> > r);"
>> (neither
>> "exclusive_lock(try_rvalue_ref<upgradable_shared_lock<mutex_type> >
>> r)" ). It seems inconsistent with rest of interface. What about
>> "write_lock (rvalue_ref<upgradable_read_lock<mutex_type> > r,
>> blocking_enum block_it);" ?
> 
> 
> My problem is that this syntax does not generalize to the 
> try-move-from-upgradable_lock-assignment:
> 
> upgradable_lock ul(m);
> 
> scoped_lock sl  (try_move(ul));
>             sl = try_move(ul);
> 
> I really want the try-to-upgrade syntax to be consistent between copy 
> ctor and assignment forms.  The consistency makes the interface easier 
> to learn.
> 
>> * mutex() memeber function may return void* instead of "const
>> mutex_type*". This will make it suitable just to check for equality of
>> owned mutex, while preventing any kind of mutex manipulation. Just an
>> idea, because "const" (which is already there) in most cases should
>> suffice.
> 
> 
> I went with Peter's non-const pointer.
> 
>> * I'd still like to have number of free functions:
>>
>> template <typename Mutex>
>> scoped_lock<Mutex> lock_mutex(Mutex); // or "acquire"
>>
>> template <typename Mutex>
>> scoped_lock<Mutex> try_lock_mutex(Mutex); // nonblocking; or
>> "try_acquire"
>>
>> template <typename Mutex>
>> scoped_lock<Mutex> try_lock_mutex(Mutex, const elapsed_time&); // or
>> "try_acquire"
>>
>> These functions might look little different (eg. use enumerations
>> locking_enum  and blocking_enum). The idea is that function will create
>> lock appropriate for given Mutex type - assuming that there migh be
>> Mutex-es delivered with their own lock class, or that.
> 
> 
> I'm not completely against the free functions.  But I do feel that they 
> are syntax sugar for the constructors that must be there anyway 
> (discussed more below).  And if you have them for one lock, you need 
> them for all 3 locks for consistency.
> 
>> * I'd like to have common base class over all lock classes. I do not
>> want polymorphic behaviour (all member functions might be protected,
>> including destructor, ctor and cctor), but to allow following very
>> simple and I believe typical usage scenarios:
> 
> 
> The common base class stuff makes me nervous.  I don't want polymorphic 
> behavior either.  But if it isn't polymorphic behavior what would 
> ~lock() do?  Choices are m_.unlock(), m_.unlock_sharable() and 
> m_.unlock_upgradable().
> 
> On Jul 19, 2004, at 8:47 AM, Peter Dimov wrote:
> 
>> I mostly agree with Bronek Kozicki. Given a movable lock, Eric Niebler's
>> proposal:
>>
>>     scoped_lock try_lock( Mutex & m );
>>     scoped_lock timed_lock( Mutex & m );
>>
>> is a better try/timed interface. Heisenberg constructors must die.
> 
> 
> Sorry, I don't know what a Heisenberg constructor is.

One that leaves you uncertain what state it's in after it runs (name 
taken from the "Heisenberg Uncertainty Principle" of physics).

> I don't dislike these helper functions.  But I do dislike the idea that 
> they should be the only way to construct-and-try or 
> construct-with-time.  The problem is that we have 3 different types of 
> locks, and I want to be able to construct-and-try in a generic-lock 
> function:
> 
> template <class Lock>
> void
> foo()
> {
>     Lock lk(m, try_lock);
>     ...
> }
> 
> The above code should work for scoped_lock, sharable_lock and 
> upgradable_lock.  I don't see how to make that work with the helper 
> function syntax, at least not without getting a lot uglier (like using 
> explicit template arguments).

I like the free-function syntax, but this seems like a good point to me.

>> Also, I still think that the bool parameter is better than a "locking" 
>> enum,
>> as evidenced by the use case shown in the specification:
>>
>>     scoped_lock lock( m, want_to_lock? locking: non_locking );
>>
>> I find this an unnecessarily verbose and contrived way to express
>>
>>     scoped_lock lock( m, want_to_lock );
>>
>> which is the original intent.
> 
> 
> So how do you feel about:
> 
> scoped_lock lk1(m, defer_lock);  // not locked
> scoped_lock lk2(m, try_lock);    // tries to lock
> 
> As others have shown, you can still decide at runtime with something like:
> 
> scoped_lock lk(want_to_lock ? scoped_lock(m) : scoped_lock(defer_lock));

I see. Not being able to decide at runtime was what shot down the 
type-based constructors for me (though I liked them lot) before; movable 
locks make it possible. However, if scoped_lock lock( m, want_to_lock? 
locking: non_locking ) is too verbose, this is even worse. To me the 
above syntax is ugly for something so common, and slightly inefficient. 
The free functions would be just as inefficient, I guess, but prettier.

> or whatever.  As Vladimir pointed out, these new constructors are very 
> cheap and fast since they don't have to test anything to find out what 
> they should do.  The defer_lock constructor is especially fast.  It just 
> sets the internal mutex reference to m, and the locked_ to false (two, 
> maybe 3 assembly instructions).

This is one of the things I really liked about the type-based 
constructor selection.

>> I disagree that
>>
>>     void * mutex() const;
>>
>> is a better mutex() accessor. On the contrary, I'd argue that mutex() 
>> should
>> return a non-const mutex_type. A const return doesn't make a lot of sense
>> given that our mutexes have no const member functions; the private 
>> interface
>> is a better protection than the const. On the other hand, a non-const 
>> return
>> allows the client to use scoped_lock<> on user-defined mutex types (in
>> implementing the user-defined condition::wait, for example).
> 
> 
> I've gone with:
> 
> mutex_type* mutex() const;
> 
> I don't have very strong feelings on this one, except that the current 
> boost design does not hide the mutex interface as we thought it should. 
>  You just get to the mutex interface via the lock instead of directly.  
> So I'm not that anxious to make it difficult to access the mutex and 
> operate on it directly.  Sometimes there's good reason to do that.

What can you do to a mutex except lock it with a lock object?

>> I also like the proposed scoped_lock == write_lock unification.
> 
> 
> <nod>
> 
> On Jul 19, 2004, at 2:05 PM, Eric Niebler wrote:
> 
>> Movable locks open up all sorts of interesting interface possibilities.
> 
> 
> <nods so agreeably that head almost falls off!>  :-)
> 
> On Jul 19, 2004, at 6:28 PM, David Abrahams wrote:
> 
>> "non_locking" is a terribly obscure name to associate with a lock.  I
>> really think "deferred" works well.
> 
> 
> <nod>  How's:
> 
> scoped_lock lk1(m, defer_lock);  // not locked

Another possibility is to name the second parameter as an adjective 
describing the initial state of the lock instead of a verb:

scoped_lock l(m, unlocked);

Mike