Howard, Thank you for your time to get the spec. together. A must to move forward. A few comments. 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. (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. My 2c. Best, V.
-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Howard Hinnant Sent: Sunday, 18 July 2004 2:32 PM To: boost@lists.boost.org Subject: [boost] Re: Lock unification [move]
I have attempted a specification of moving scoped_lock/read_lock/upgradable_read_lock/write_lock at:
http://home.twcny.rr.com/hinnant/cpp_extensions/threads.html
This is an attempt to look forward to locks+move semantics, taking into account the recent discussions. Because of the length of the specification I put it in an html on my personal website instead of stuffing it into an email. Comments welcome. Any response from me is likely to be delayed by several days due to travel.
-Howard
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Thanks everyone for your comments and patience. There's a new spec up at: 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. 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 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
* 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. 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).
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)); 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).
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.
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 -Howard
Howard Hinnant <hinnant@twcny.rr.com> writes:
<nod> How's:
scoped_lock lk1(m, defer_lock); // not locked ^^^^ ^^^^
Redundant. "deferred" is better. -- Dave Abrahams Boost Consulting http://www.boost-consulting.com
On Jul 21, 2004, at 7:16 PM, David Abrahams wrote:
Howard Hinnant <hinnant@twcny.rr.com> writes:
<nod> How's:
scoped_lock lk1(m, defer_lock); // not locked ^^^^ ^^^^
Redundant. "deferred" is better.
I seriously thought about deferred. The word, whatever it is, if standardized, would be at namespace scope in namespace std. I was worried about it conflicting, or with its use not being easily associated with locks if it was just "deferred". Otoh, it is only a tag. Maybe it is a feature, not a bug for "deferred" to be able to influence (conflict with) things other than locks! Thanks for the second thought. -Howard
On Jul 21, 2004, at 8:30 PM, Howard Hinnant wrote:
On Jul 21, 2004, at 7:16 PM, David Abrahams wrote:
Howard Hinnant <hinnant@twcny.rr.com> writes:
<nod> How's:
scoped_lock lk1(m, defer_lock); // not locked ^^^^ ^^^^
Redundant. "deferred" is better.
I seriously thought about deferred. The word, whatever it is, if standardized, would be at namespace scope in namespace std. I was worried about it conflicting, or with its use not being easily associated with locks if it was just "deferred". Otoh, it is only a tag. Maybe it is a feature, not a bug for "deferred" to be able to influence (conflict with) things other than locks! Thanks for the second thought.
Oh, I just remembered another reason I chose defer_lock over deferred. I was trying to be symmetric with the try-counterpart: scoped_lock lk1(m, defer_lock); scoped_lock lk2(m, try_lock); vs: scoped_lock lk1(m, deferred); scoped_lock lk2(m, tried); <shrug> I disliked "tried" more than I liked "deferred". And I also felt that the benefit of symmetry was important to make the interface easier to learn. -Howard
Howard Hinnant <hinnant@twcny.rr.com> writes:
scoped_lock lk1(m, deferred); scoped_lock lk2(m, tried);
<shrug> I disliked "tried" more than I liked "deferred". And I also felt that the benefit of symmetry was important to make the interface easier to learn.
You only need part-of-speech symmetry. "deferred" is an adjective, so "non_blocking", "optional", "permeable",... would work find for the other one, from a grammatical point of view. I don't think "try" makes a very good adjective, and adding "_lock" to the end of it doesn't help. So I don't see the symmetry in "try_lock"/"deferred_lock" other than the suffix :vP -- Dave Abrahams Boost Consulting http://www.boost-consulting.com
From: Howard Hinnant <hinnant@twcny.rr.com>
Oh, I just remembered another reason I chose defer_lock over deferred. I was trying to be symmetric with the try-counterpart:
scoped_lock lk1(m, defer_lock); scoped_lock lk2(m, try_lock);
vs:
scoped_lock lk1(m, deferred); scoped_lock lk2(m, tried);
<shrug> I disliked "tried" more than I liked "deferred". And I also felt that the benefit of symmetry was important to make the interface easier to learn.
Actually, I think "deferred" and "tried" are better. The reason is that they say that locking of a lock so constructed was "deferred" or "tried." The latter doesn't indicate whether the attempt to lock succeeded, just that there was an attempt. From: Michael Glassford <glassfordm@hotmail.com>
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);
This, seems even better; it's at least easier to explain! scoped_lock lk1(m, unlocked); scoped_lock lk2(m, tried); -- Rob Stewart stewart@sig.com Software Engineer http://www.sig.com Susquehanna International Group, LLP using std::disclaimer;
Howard Hinnant wrote:
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.
template<class Lock> void foo() { bool const deferred = false; Lock lock( m, deferred ); if( lock.try_lock() ) { // locked } } as usual. Clean separation between lock type (scoped/exclusive, shareable, upgradable) and lock operations (unconditional blocking lock, try lock, timed lock, unlock).
On Jul 22, 2004, at 7:54 AM, Peter Dimov wrote:
Howard Hinnant wrote:
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.
template<class Lock> void foo() { bool const deferred = false;
Lock lock( m, deferred );
if( lock.try_lock() ) { // locked } }
as usual. Clean separation between lock type (scoped/exclusive, shareable, upgradable) and lock operations (unconditional blocking lock, try lock, timed lock, unlock).
Ok. By that reasoning the try_lock(Mutex&) helper function is also unnecessary. The ability to construct an unlocked lock is fundamental, and everything else can be built in terms of that, including locked and timed-lock constructors/helpers. But the other constructors / helpers are convenient. Let's review: // no try-ctor, no try-helper func template <class Lock> void foo1() { Lock lk(m, false); if (lk.try_lock()) ... } // use a try_lock helper func template <class Lock> void foo2() { if (Lock lk = try_lock(m)) ... } Notation is a little more compact, but it only works for whatever type Lock that try_lock() returns (presumably scoped_lock). // use a generic try_lock helper func template <class Lock> void foo3() { if (Lock lk = try_lock<Lock>(m)) ... } This is now generalized to work for all Lock types. // use a try-lock ctor template <class Lock> void foo4() { if (Lock lk = Lock(m, try_lock)) ... } This also works for all Lock types. Solution 1 is slightly less efficient than solutions 3 and 4 (solution 2 isn't really a solution). The reason is that foo1 expands to: m_ = m; locked_ = lock_it; if (locked_) m_.lock(); locked_ = m_.try_lock(); if (locked_) ... I.e. the Lock(m, bool) constructor checks the value of the bool to decide whether to lock or not. In contrast, solutions 3 and 4 look more like: m_ = m; locked_ = m_.try_lock(); if (locked_) ... I.e. no attempt is made to call m_.lock(). Actually solution 3 will only be this efficient if the try_lock helper function doesn't construct its Lock via the code shown in foo1, but instead uses something equivalent to the Lock(m, try_lock) constructor. So 3 and 4 look the best to me, both in terms of readability, and in terms of more optimized code (size and speed). And it seems like the best way to efficiently implement 3 is by using the same constructor that 4 uses. If you follow up to this point, then given 4 exists anyway, is 3 worth the trouble? -Howard
Howard Hinnant wrote:
But the other constructors / helpers are convenient. Let's review:
// no try-ctor, no try-helper func template <class Lock> void foo1() { Lock lk(m, false); if (lk.try_lock()) ... }
// use a try_lock helper func template <class Lock> void foo2() { if (Lock lk = try_lock(m)) ... }
Notation is a little more compact, but it only works for whatever type Lock that try_lock() returns (presumably scoped_lock).
// use a generic try_lock helper func template <class Lock> void foo3() { if (Lock lk = try_lock<Lock>(m)) ... }
This is now generalized to work for all Lock types.
// use a try-lock ctor template <class Lock> void foo4() { if (Lock lk = Lock(m, try_lock)) ... }
This also works for all Lock types.
The main reason I'm unconvinced about these "useful helper" constructors is that all of the use cases I see on the list are totally bogus. The only real use case for a try_lock (lock_both) does not need helper constructors. The only other real use case templated on a lock (cv::wait) accepts a lock variable and does not need helper constructors. I may be missing something, of course, but the conservative approach is to resist the temptation to provide the "but someone might find them useful!!~1" parts until we see how well they compare to the "no frills" approach in real code.
On Jul 23, 2004, at 8:10 AM, Peter Dimov wrote:
The main reason I'm unconvinced about these "useful helper" constructors is that all of the use cases I see on the list are totally bogus. The only real use case for a try_lock (lock_both) does not need helper constructors. The only other real use case templated on a lock (cv::wait) accepts a lock variable and does not need helper constructors.
I may be missing something, of course, but the conservative approach is to resist the temptation to provide the "but someone might find them useful!!~1" parts until we see how well they compare to the "no frills" approach in real code.
I sat down to write a realistic example of when a "try_lock" constructor might be sufficiently desirable to warrant its existence: Consider a program with a list a jobs that needs doing, in no particular order (maybe updating various parts of a display - clock, temperature, internet status, whatever). A reasonable strategy would be to have a vector<Job> where each Job has a functor, a mutex, and a bool indicating whether or not it needed running. You could throw several threads at the function which executes each job in the vector<Job>, and each thread could try to get the mutex, and if it failed, just move on to the next job. Such a strategy might make good use of multiple processors. Here's what I came up with: template <class F, class Mutex> struct job { typedef F Func; typedef Mutex Mutex; F f_; Mutex* mut_; bool needs_executing_; }; template <class Job> void do_jobs(std::vector<Job>& v) { typedef typename Job::Mutex Mutex; typedef typename Mutex::scoped_lock Lock; for (typename std::vector<Job>::iterator i = v.begin(), e = v.end(); i != e; ++i) { Lock lock(*i->mut_, try_lock); if (lock && i->needs_executing_) { i->needs_executing_ = false; i->f_(); } } } Then I rewrote do_jobs for the case where I have no try-lock ctor: template <class Job> void do_jobs2(std::vector<Job>& v) { using namespace Metrowerks; typedef typename Job::Mutex Mutex; typedef typename Mutex::scoped_lock Lock; for (typename std::vector<Job>::iterator i = v.begin(), e = v.end(); i != e; ++i) { Lock lock(*i->mut_, defer_lock); if (lock.try_lock() && i->needs_executing_) { i->needs_executing_ = false; i->f_(); } } } You really have to squint to see the difference. ... And I just wrote AND erased several paragraphs proposing that we eliminate the try and timed constructors, but also how I did not feel strongly either way! :-) But I just noticed a difference (apparently I didn't squint enough the first time ;-) ). The loop in do_jobs is no-throw if i->f_() is no-throw. The same loop in do_jobs2 should be no-throw as well, and it is, but the compiler doesn't know it. That is, the statement: lock.try_lock() hides the test: if (locked_) throw lock_error(); This is an unnecessary run time inefficiency. And for a compiler scanning the code to determine whether or not it might throw (a reasonable optimization step), do_jobs may come up no-throw whereas do_jobs2 probably won't (there's no way one can tag try_lock() as a function that won't throw). The code size hit may be the more important consideration (over the run time hit). -Howard Flipping like a fish on the dock
Howard Hinnant wrote:
I sat down to write a realistic example of when a "try_lock" constructor might be sufficiently desirable to warrant its existence:
[...]
template <class Job> void do_jobs(std::vector<Job>& v) { typedef typename Job::Mutex Mutex; typedef typename Mutex::scoped_lock Lock; for (typename std::vector<Job>::iterator i = v.begin(), e = v.end(); i != e; ++i) { Lock lock(*i->mut_, try_lock); if (lock && i->needs_executing_) { i->needs_executing_ = false; i->f_(); } } }
Then I rewrote do_jobs for the case where I have no try-lock ctor:
template <class Job> void do_jobs2(std::vector<Job>& v) { using namespace Metrowerks; typedef typename Job::Mutex Mutex; typedef typename Mutex::scoped_lock Lock; for (typename std::vector<Job>::iterator i = v.begin(), e = v.end(); i != e; ++i) { Lock lock(*i->mut_, defer_lock); if (lock.try_lock() && i->needs_executing_) { i->needs_executing_ = false; i->f_(); } } }
You really have to squint to see the difference. ... And I just wrote AND erased several paragraphs proposing that we eliminate the try and timed constructors, but also how I did not feel strongly either way! :-)
But I just noticed a difference (apparently I didn't squint enough the first time ;-) ).
The loop in do_jobs is no-throw if i->f_() is no-throw. The same loop in do_jobs2 should be no-throw as well, and it is, but the compiler doesn't know it. That is, the statement:
lock.try_lock()
hides the test:
if (locked_) throw lock_error();
This is an unnecessary run time inefficiency.
Interesting point. I really hoped that compilers are good enough to inline the constructor and try_lock and eliminate the dead code. But it doesn't seem so. The question is, is this an argument in favor of the try_lock constructor, or an argument against the if( locked_ ) overhead in the try_lock member function? As for the constructor vs member function: a counterpoint: consider what happens when the programmer omits the if(lock) test by mistake. I'm not saying that this is a particularly compelling argument, by the way, but you may feel differently.
On Jul 29, 2004, at 9:52 AM, Peter Dimov wrote:
The loop in do_jobs is no-throw if i->f_() is no-throw. The same loop in do_jobs2 should be no-throw as well, and it is, but the compiler doesn't know it. That is, the statement:
lock.try_lock()
hides the test:
if (locked_) throw lock_error();
This is an unnecessary run time inefficiency.
Interesting point. I really hoped that compilers are good enough to inline the constructor and try_lock and eliminate the dead code. But it doesn't seem so.
<nod>
The question is, is this an argument in favor of the try_lock constructor, or an argument against the if( locked_ ) overhead in the try_lock member function?
If we were to eliminate the if (locked_) overhead from the try_lock member, it seems reasonable that we also do so for the timed_lock member, the lock member, and the if (!locked) overhead from the unlock member. This does not seem unreasonable to me. After all, locking an already locked mutex, through the same lock, seems like a programming error, not an unforeseen run time event (like memory running out, disk full or missing, connection dropped, etc.). Perhaps relegating this to undefined behavior (i.e. assert) is more appropriate. I'm not positive of that line of reasoning. Just exploring it. I'm trying to come up with a scenario where trying to lock an already locked lock is an unforeseen run time event as opposed to a logic error...
As for the constructor vs member function: a counterpoint: consider what happens when the programmer omits the if(lock) test by mistake. I'm not saying that this is a particularly compelling argument, by the way, but you may feel differently.
If the efficiency argument goes away, then I'm back to square 1 on this (i.e. starting over in my thought process). A constructor has a job: To put the object into a consistent state with all invariants valid. After construction a lock may either be locked or not: scoped_lock<Mutex> lock1(m); // locked scoped_lock<Mutex> lock2(m, defer_lock); // not locked scoped_lock<Mutex> lock3(m, try_lock); // Heisenberg ;-) This 3rd constructor doesn't result in invalid invariants despite the cute comment. You just don't know its state without further testing. scoped_lock<Mutex> lock4(m, elasped_time(1)); Same with this constructor: you don't know its state. But you do know that all of its internal invariants are held. Forgetting the if (lock) test after such a constructor doesn't concern me too much. There may be situations where you don't want to test immediately. There may be others where the test is right at the constructor: if (scoped_lock<Mutex> lock = scoped_lock<Mutex>(m, try_lock)) ... <shrug> If the efficiency argument goes away, I can't get too worked up about the try and timed constructors one way or the other. I feel strongly that the defer_lock constructor only result in an unlocked lock, and not test a bool or an enum (as long as locks are movable). I feel strongly that the existence of the try and timed constructors be considered together: either we have both or neither. ... So what color would you like this bicycle shed anyway? :-) -Howard
Howard Hinnant wrote:
Thanks everyone for your comments and patience. There's a new spec up at:
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.
Something that I missed the first time: I'd prefer more fine-grained exceptions. lock_error should be a base class only.
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
On Jul 22, 2004, at 9:41 AM, Michael Glassford wrote:
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.
Sorry, my acknowledgment list has gotten so confused I didn't even attempt one in the latest spec. The thread has gotten so long I'm forgetting who's proposed what, and have not taken the time to read back and check.
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?
My comments were based on the fact that currently the mutex interface is private except for construction and destruction. This is meant to encourage the programmer to use the mutex only in an orderly (RAII) fashion. <soapbox> However we do not currently prohibit code such as the following: typedef mutex::scoped_lock MyMutex; mutex m; MyMutex my_mut(m, false); void foo() { my_mut.lock(); } void bar() { my_mut.unlock(); } int main() { foo(); bar(); } I.e. It is not difficult to make a lock look exactly like a mutex with public member functions, and then use it in whatever way (orderly or not) you want to. Given that, perhaps it is better to just make the mutex interface public in the first place, and expose the mutex of a lock. Afterall, there are legitimate uses for locking and unlocking a mutex in a non-RAII pattern, and there are legitimate uses for needing access to a lock's mutex. The tools should not get in the way of the coder in the name of trying to save the coder from his own stupidity. Otoh, a good tool will make it easy to avoid stupid mistakes and write correct, efficient and elegant code. It is always a delicate line to walk when designing an interface. </soapbox>
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);
scoped_lock l(m, tried); // ? scoped_lock l(m, Heisenberg); // ? :-) Imho we need to consider both constructors (defer_lock / try_lock) together for consistency's sake (unless, as Peter suggests, we do not want the try-ctor at all). -Howard
Howard Hinnant wrote:
On Jul 22, 2004, at 9:41 AM, Michael Glassford wrote:
What can you do to a mutex except lock it with a lock object?
My comments were based on the fact that currently the mutex interface is private except for construction and destruction. This is meant to encourage the programmer to use the mutex only in an orderly (RAII) fashion.
<soapbox> However we do not currently prohibit code such as the following:
typedef mutex::scoped_lock MyMutex; mutex m; MyMutex my_mut(m, false);
void foo() { my_mut.lock(); }
void bar() { my_mut.unlock(); }
int main() { foo(); bar(); }
I.e. It is not difficult to make a lock look exactly like a mutex with public member functions, and then use it in whatever way (orderly or not) you want to. Given that, perhaps it is better to just make the mutex interface public in the first place, and expose the mutex of a lock.
Please no. The scoped_lock keeps track of whether it has locked the mutext or not, so it kows whether or not to unlock the mutex in its destructor. If you make the mutex interface public AND expose the mutex of the lock, you break encapsulation: mutext m; { scoped_lock l( m ); // locked l.mutext()->unlock(); // unlocked } // unlocked again by l's destructor! It is for precisely this reason that I think you shouldn't be able to get a pointer to the mutex from the scoped_lock. You should get back a token or a void* that is useful only for comparison purposes. -- Eric Niebler Boost Consulting www.boost-consulting.com
On Jul 22, 2004, at 1:16 PM, Eric Niebler wrote:
Please no. The scoped_lock keeps track of whether it has locked the mutext or not, so it kows whether or not to unlock the mutex in its destructor. If you make the mutex interface public AND expose the mutex of the lock, you break encapsulation:
mutext m;
{ scoped_lock l( m ); // locked l.mutext()->unlock(); // unlocked } // unlocked again by l's destructor!
I just knew I was getting too much in the mood of: Here's the rope, don't hang yourself! :-)
It is for precisely this reason that I think you shouldn't be able to get a pointer to the mutex from the scoped_lock. You should get back a token or a void* that is useful only for comparison purposes.
I'll let you and Peter hammer that one out. I could be happy with whatever you guys decide. -Howard
Howard Hinnant wrote:
On Jul 22, 2004, at 1:16 PM, Eric Niebler wrote:
Please no. The scoped_lock keeps track of whether it has locked the mutext or not, so it kows whether or not to unlock the mutex in its destructor. If you make the mutex interface public AND expose the mutex of the lock, you break encapsulation:
mutext m;
{ scoped_lock l( m ); // locked l.mutext()->unlock(); // unlocked } // unlocked again by l's destructor!
I just knew I was getting too much in the mood of: Here's the rope, don't hang yourself! :-)
It is for precisely this reason that I think you shouldn't be able to get a pointer to the mutex from the scoped_lock. You should get back a token or a void* that is useful only for comparison purposes.
I'll let you and Peter hammer that one out. I could be happy with whatever you guys decide.
The point is that the scoped_lock<> template does not make any decisions or assumptions about the accessibility of the mutex interface. That is, even if scoped_lock<> (the usual suspect - parameter defaults to 'mutex') gives you a mutex*, you still can do nothing with it (unless you a a frind of said 'mutex' class, i.e. you are a condition variable ;-) ). However, when scoped_lock<MyMutexType> returns you a MyMutexType*, you may be able to do something with it. IOW: the decision as to whether to expose the mutex interface to the general public is made by the designer of the mutex class. The lock class is a fully-specified, general purpose helper. Its interface is not influenced by the standard mutex types.
On Jul 23, 2004, at 2:10 PM, Howard Hinnant wrote:
On the constructor side, this could look like:
upgradable_lock ul(mut); scoped_lock sl(ul); // block, we have this in the current spec scoped_lock sl(ul, try_lock); // proposed to replace sl(try_move(ul))
Oops, forgot the cast-to-rvalue on ul: upgradable_lock ul(mut); scoped_lock sl(move(ul)); // block, we have this in the current spec scoped_lock sl(move(ul), try_lock); // proposed to replace sl(try_move(ul)) -Howard
Eric Niebler wrote:
get a pointer to the mutex from the scoped_lock. You should get back a token or a void* that is useful only for comparison purposes.
It just came to my mind that if we decide to add common base class, such void* could be returned there. Thus it would look something like: class lock { bool* active_; void* mutex_; protected: ~lock() {} // no-op lock(bool* active, void* mutex) : active_(active), mutex_(mutex) {assert(active_ != NULL); assert(mutex_ != NULL);} lock(const lock& src) : active_(src.active_) {assert(src.mutex_ == mutex_);} public: bool active() const {return *active_;} void* mutex() const {return mutex_;} // or just polymorphic functions: // virtual bool active() const = 0; // virtual void* mutex() const = 0; operator safe_bool() const {return active() ? safe_true : safe_false;} }; Actually "mutex" can be renamed here to "handle" or "primitive" ... B.
Howard Hinnant wrote:
On Jul 22, 2004, at 9:41 AM, Michael Glassford wrote:
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.
Sorry, my acknowledgment list has gotten so confused I didn't even attempt one in the latest spec. The thread has gotten so long I'm forgetting who's proposed what, and have not taken the time to read back and check.
I'm not worried about that; I've forgotten who said what myself. I just wondered if you remembered that it had come up before. For what it's worth, I believe my final proposal looked as follows (the items marked "-" are possible constructors that I intentionally rejected): ScopedLock ---------- lock(m) lock(m, unlocked) - lock(m, locked) - lock(m, locked, blocking) - lock(m, blocking) ScopedTryLock ------------- - try_lock(m) //locked, blocking try_lock(m, unlocked) try_lock(m, blocking) try_lock(m, non_blocking) - try_lock(m, locked, blocking) - try_lock(m, locked, non_blocking) ScopedTimedLock --------------- - timed_lock(m) //locked, blocking timed_lock(m, unlocked) timed_lock(m, blocking) timed_lock(m, non_blocking) timed_lock(m, t) - timed_lock(m, locked, blocking) - timed_lock(m, locked, non_blocking) - timed_lock(m, t, locked) - timed_lock(m, t, locked, blocking) - timed_lock(m, t, blocking)
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?
My comments were based on the fact that currently the mutex interface is private except for construction and destruction. This is meant to encourage the programmer to use the mutex only in an orderly (RAII) fashion.
<soapbox> However we do not currently prohibit code such as the following:
typedef mutex::scoped_lock MyMutex; mutex m; MyMutex my_mut(m, false);
void foo() { my_mut.lock(); }
void bar() { my_mut.unlock(); }
int main() { foo(); bar(); }
No, but the lock still unlocks when it goes out of scope and the mutex doesn't, which is some difference. Do you know of a way to prohibit such code?
I.e. It is not difficult to make a lock look exactly like a mutex with public member functions, and then use it in whatever way (orderly or not) you want to. Given that, perhaps it is better to just make the mutex interface public in the first place,
I don't agree.
and expose the mutex of a lock. Afterall, there are legitimate uses for locking and unlocking a mutex in a non-RAII pattern, and there are legitimate uses for needing access to a lock's mutex. The tools should not get in the way of the coder in the name of trying to save the coder from his own stupidity. Otoh, a good tool will make it easy to avoid stupid mistakes and write correct, efficient and elegant code. It is always a delicate line to walk when designing an interface. </soapbox>
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);
scoped_lock l(m, tried); // ? scoped_lock l(m, Heisenberg); // ? :-)
scoped_lock l(m, non_blocking); Mike
Howard Hinnant wrote:
<soapbox> However we do not currently prohibit code such as the following:
typedef mutex::scoped_lock MyMutex; mutex m; MyMutex my_mut(m, false);
void foo() { my_mut.lock(); }
void bar() { my_mut.unlock(); }
int main() { foo(); bar(); }
No, a lock is not a mutex, because a mutex is inter-thread, and a lock is thread-local (don't worry, I've been that same road myself, but Bill Kempf enlightened me).
Howard Hinnant wrote:
Thanks everyone for your comments and patience. There's a new spec up at:
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!
<snip> The scoped_lock assignment operators make me a bit nervous. For instance, you don't specify what this should do: Mutex mut; scoped_lock< Mutex > l1( mut ); // lock once scoped_lock< Mutex > l2( mut ); // lock twice (recursive) l1 = move(l2); // ??? If I had to guess, I suppose this would have the effect of simply unlocking l2. Or should it throw? I would prefer the former if I had to choose, but the specification should be clear on this point. Oh, and thanks for your work on the spec Howard. :-) -- Eric Niebler Boost Consulting www.boost-consulting.com
On Jul 22, 2004, at 2:37 PM, Eric Niebler wrote:
The scoped_lock assignment operators make me a bit nervous. For instance, you don't specify what this should do:
Mutex mut; scoped_lock< Mutex > l1( mut ); // lock once scoped_lock< Mutex > l2( mut ); // lock twice (recursive) l1 = move(l2); // ???
If I had to guess, I suppose this would have the effect of simply unlocking l2. Or should it throw? I would prefer the former if I had to choose, but the specification should be clear on this point.
Excellent point. I did not consider the possibility of a recursive mutex when I wrote the spec for this, nor when I prototyped it. You've found a bug in both the spec and my prototype. The (proposed) move assignment philosophy is: l1 = move(l2); behaves as if: atomic { if (l1.locked()) l1.unlock(); if (l2.locked()) { l2.unlock(); l1.lock(); } } So in your example l2 would be unlocked, l1 would remain locked, and the underlying mutex lock count would be decremented by 1. Proposed spec: Effects: Throws a lock_error() if mutex() != sl.p_->mutex(). If locked() before the call, then unlock() is called on the mutex. Postconditions: locked() == the value of sl.p_->locked() before the assignment. sl.p_->locked() == false after the assignment. Notes: This scoped_lock relinquishes any mutex ownership it has. Then if the sl scoped_lock owns the mutex, ownerhisp is transferred to this scoped_lock with no blocking. If the sl scoped_lock does not own the mutex, then neither will this scoped_lock after the assignment. Only rvalue scoped_lock's will match this signature. lvalue scoped_lock's can not transfer ownership unless "cast" to an rvalue. An lvalue scoped_lock can be "cast" to an rvalue with the expression: move(lock); Thanks for the catch. -Howard
Howard Hinnant <hinnant@twcny.rr.com> writes:
The (proposed) move assignment philosophy is:
l1 = move(l2);
behaves as if:
atomic { if (l1.locked()) l1.unlock(); if (l2.locked()) { l2.unlock(); l1.lock(); } }
So in your example l2 would be unlocked, l1 would remain locked, and the underlying mutex lock count would be decremented by 1.
weird; the mutex association is not transferred along with "locked-ness"? Uh-oh: I think I just invoked the locked-ness monster. -- Dave Abrahams Boost Consulting http://www.boost-consulting.com
David Abrahams wrote:
Howard Hinnant <hinnant@twcny.rr.com> writes:
So in your example l2 would be unlocked, l1 would remain locked, and the underlying mutex lock count would be decremented by 1.
weird; the mutex association is not transferred along with "locked-ness"?
It throws if the two locks don't refer to the same mutex.
Uh-oh: I think I just invoked the locked-ness monster.
<groan> ;-) -- Eric Niebler Boost Consulting www.boost-consulting.com
Howard Hinnant wrote:
* 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.
I'm little nervous seeing "try_move". If move semantics (the one you proposed in N1377) gets into language, "move" can be replaced with simple move-constructor, while "try_move" cannot. Moreover, there is already constructor of scoped_lock) taking two parameters, second of them being enumeration (or bool - I'm not in favour of either) meaning "this operation may block". Thus, under my proposal we would have: upgradable_lock ul(m1); // blocking scoped_lock s2(m2); // blocking scoped_lock sl(move(ul)); // blocking scoped_lock s3(m3, try_lock); // non-blocking (A) upgradable_lock u4(m4); // blocking scoped_lock s4(move(u4), try_lock); // non-blocking (B) I think that you will agree that A and B are similar and consistent?
I went with Peter's non-const pointer.
I think that additional const member functions: template <typename Mutex> bool same_mutex(const scoped_lock<Mutex>&) const; template <typename Mutex> bool same_mutex(const upgradable_lock<Mutex>&) const; template <typename Mutex> bool same_mutex(const read_lock<Mutex>&) const; are not useful any more?
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.
users of all locks could use the same templated functions: template <template <typename _Mutex> class Lock, typename Mutex> Lock<Mutex> lock(Mutex&); template .... Lock<Mutex> try_lock(Mutex&); template .... Lock<Mutex> try_lock(Mutex&, const elapsed_time&);
* 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().
destructor would be protected (as constructor, cctor and assignment) and do nothing. There's no special need for any virtual functions, including destructor. This base class would be useful to accept and use reference to lock value returned by helper functions: // we do not know type of some_synchronization_primitive here lock_base& l = try_lock<scoped_lock>(some_synchronization_primitive); if (l) { // ... } It would provide very limited set of non-virtual member functions: bool locked() const; operator safe_bool() const; and maybe some way to test identity of mutex with other lock. We could use it when using generic helper functions to create lock from unknown (at the time of writing code) type of mutex, as demonstrated above.
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 whole thing with helper functions is that we minimize number of constructors, instead providing functions. Take a look at following code: { exclusive_lock some_lock(some_mutex); // some_mutex is not locked yet; some_lock merely points to it some_lock.lock(); // some_mutex is locked now } // after destruction of some_lock, some_mutex is unlocked again Here exclusive_lock would have only one, nonblocking constructor. All blocking operations would be performed "by hand" using member functions, or in template helper functions (like try_lock above). I know this is very different design. Just one nonblocking constructor, number of simple functions (mostly forwarding requests to synchronization primitive and remembering returned state of it when necessary) and destructor required to free lock. Move-constructors and move-assignment (as well as regular cctor and assignment in case of shared lock) are great ideas, also upgradable_lock has its place in such design. Best regards B.
On Jul 22, 2004, at 4:52 PM, Bronek Kozicki wrote:
Howard Hinnant wrote:
* 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.
I'm little nervous seeing "try_move". If move semantics (the one you proposed in N1377) gets into language, "move" can be replaced with simple move-constructor, while "try_move" cannot.
Actually N1377 suggests a move helper function: template <class T> inline T&& move(T&& x) { return static_cast<T&&>(x); } And N1377 got it slightly wrong. It should really look like: template <class T> inline typename remove_reference<T>::type&& move(T&& x) { return x; } If we introduce "try_move" it could be a similarly simple helper which just returns an auto_ptr-ref-like object tagged with the "try" information. Not that I'm 100% dead set on try_move, I'm not.
Moreover, there is already constructor of scoped_lock) taking two parameters, second of them being enumeration (or bool - I'm not in favour of either) meaning "this operation may block". Thus, under my proposal we would have:
upgradable_lock ul(m1); // blocking scoped_lock s2(m2); // blocking scoped_lock sl(move(ul)); // blocking scoped_lock s3(m3, try_lock); // non-blocking (A)
upgradable_lock u4(m4); // blocking scoped_lock s4(move(u4), try_lock); // non-blocking (B)
I think that you will agree that A and B are similar and consistent?
Yes, but my concern is what does the syntax look like when s4 already exists and you try to upgrade to it by assigning u4 to it: s4 = u4; // ??? Try to upgrade (non-blocking) u4 to s4 This isn't a hypothetical scenario, I have a use case for it. I've suggested s4 = try_move(u4); and am open to other syntax suggestions.
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.
users of all locks could use the same templated functions:
template <template <typename _Mutex> class Lock, typename Mutex> Lock<Mutex> lock(Mutex&);
template .... Lock<Mutex> try_lock(Mutex&);
template .... Lock<Mutex> try_lock(Mutex&, const elapsed_time&);
I believe this still requires Lock to be explicitly supplied in each case. That's doable. But is it sufficiently desirable?
* 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().
destructor would be protected (as constructor, cctor and assignment) and do nothing. There's no special need for any virtual functions, including destructor. This base class would be useful to accept and use reference to lock value returned by helper functions:
// we do not know type of some_synchronization_primitive here lock_base& l = try_lock<scoped_lock>(some_synchronization_primitive); if (l) { // ... }
It would provide very limited set of non-virtual member functions: bool locked() const; operator safe_bool() const;
and maybe some way to test identity of mutex with other lock. We could use it when using generic helper functions to create lock from unknown (at the time of writing code) type of mutex, as demonstrated above.
Could you flesh this out with a little more complete code. Sorry, I'm just not following. For instance the first line appears to bind an rvalue to a non-const reference. Are we just missing a const on the lock_base& or is there something else going on?
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 whole thing with helper functions is that we minimize number of constructors, instead providing functions. Take a look at following code:
{ exclusive_lock some_lock(some_mutex); // some_mutex is not locked yet; some_lock merely points to it some_lock.lock(); // some_mutex is locked now } // after destruction of some_lock, some_mutex is unlocked again
Here exclusive_lock would have only one, nonblocking constructor. All blocking operations would be performed "by hand" using member functions, or in template helper functions (like try_lock above).
I know this is very different design. Just one nonblocking constructor, number of simple functions (mostly forwarding requests to synchronization primitive and remembering returned state of it when necessary) and destructor required to free lock. Move-constructors and move-assignment (as well as regular cctor and assignment in case of shared lock) are great ideas, also upgradable_lock has its place in such design.
I think this will have an uphill battle. This code: { exclusive_lock some_lock(some_mutex); // assume some_mutex is locked here } is just too easy to write (and be wrong). -Howard
On Jul 22, 2004, at 8:54 PM, Howard Hinnant wrote:
On Jul 22, 2004, at 4:52 PM, Bronek Kozicki wrote:
Howard Hinnant wrote:
* 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.
I'm little nervous seeing "try_move". If move semantics (the one you proposed in N1377) gets into language, "move" can be replaced with simple move-constructor, while "try_move" cannot.
Actually N1377 suggests a move helper function:
template <class T> inline T&& move(T&& x) { return static_cast<T&&>(x); }
And N1377 got it slightly wrong. It should really look like:
template <class T> inline typename remove_reference<T>::type&& move(T&& x) { return x; }
If we introduce "try_move" it could be a similarly simple helper which just returns an auto_ptr-ref-like object tagged with the "try" information. Not that I'm 100% dead set on try_move, I'm not.
I've been thinking more about Bronek's nervousness with "try_move". I've just thought of alternative syntax. I'm not sure yet whether I like it or not. But I'm putting it out there so that it gets more eyes than just my own (which are badly overworked at the moment)... Below, allow for the pseudo code notation: scoped_lock = move(scoped_lock); To mean that given: scoped_lock<Mutex> lock1(mut, defer_lock); scoped_lock<Mutex> lock2(mut); you can do: lock1 = move(lock1); We currently have (just discussing assignment forms below): scoped_lock = move(scoped_lock); scoped_lock = move(upgradable_lock); // 1 scoped_lock = try_move(upgradable_lock); // 2 sharable_lock = sharable_lock; sharable_lock = upgradable_lock; sharable_lock = move(scoped_lock); upgradable_lock = sharable_lock; upgradable_lock = move(upgradable_lock); upgradable_lock = move(scoped_lock); Those forms not using try_move or move take a const& on the rhs, and never block. Those forms using move on the rhs require the rhs to be an rvalue (or cast with move), and never block except for 1, which can block. The form taking try_move is a try-lock form of 1 (and thus can not block). Possible alternative syntax: scoped_lock = move(scoped_lock); scoped_lock |= move(upgradable_lock); // 1 scoped_lock <<= move(upgradable_lock); // 2 sharable_lock = sharable_lock; sharable_lock = upgradable_lock; sharable_lock = move(scoped_lock); upgradable_lock = sharable_lock; upgradable_lock = move(upgradable_lock); upgradable_lock = move(scoped_lock); try_move now does not exist. All operator= indicate a non-blocking operation. The one operator|= indicates a blocking operation. And operator<<= indicates a try-lock operation. The move on the rhs indicates whether or not an rvalue is required there. Specific upgradable_lock -> scoped_lock example: upgradable_lock ul(mut); scoped_lock sl(mut, defer_lock); ... sl = move(ul); // compile-time error, invalid operation sl |= move(ul); // Upgrades, blocking if necessary sl <<= move(ul); // Tries to upgrade without blocking On the constructor side, this could look like: upgradable_lock ul(mut); scoped_lock sl(ul); // block, we have this in the current spec scoped_lock sl(ul, try_lock); // proposed to replace sl(try_move(ul)) Thoughts? -Howard
From: Howard Hinnant <hinnant@twcny.rr.com>
I've just thought of alternative syntax. I'm not sure yet whether I like it or not. But I'm putting it out there so that it gets more eyes than just my own (which are badly overworked at the moment)...
Below, allow for the pseudo code notation:
scoped_lock = move(scoped_lock);
To mean that given:
scoped_lock<Mutex> lock1(mut, defer_lock); scoped_lock<Mutex> lock2(mut);
you can do:
lock1 = move(lock1);
lock2 = move(lock1) // ???
We currently have (just discussing assignment forms below):
scoped_lock = move(scoped_lock); scoped_lock = move(upgradable_lock); // 1 scoped_lock = try_move(upgradable_lock); // 2
sharable_lock = sharable_lock; sharable_lock = upgradable_lock; sharable_lock = move(scoped_lock);
upgradable_lock = sharable_lock; upgradable_lock = move(upgradable_lock); upgradable_lock = move(scoped_lock);
Those forms not using try_move or move take a const& on the rhs, and never block.
Those forms using move on the rhs require the rhs to be an rvalue (or cast with move), and never block except for 1, which can block.
The form taking try_move is a try-lock form of 1 (and thus can not block).
Possible alternative syntax:
scoped_lock = move(scoped_lock); scoped_lock |= move(upgradable_lock); // 1
I like the pipe's suggestion of a wall and, hence, blocking. The x= operators are good, in general, as they still suggest assignment (<<= is better than << for that reason).
scoped_lock <<= move(upgradable_lock); // 2
This isn't as suggestive as the |=. How about these: scoped_lock ~= move(upgradable_lock); scoped_lock %= move(upgradable_lock); The ~ in ~= sort of suggests try, as in try-lock. Unfortunately, the tilde is small so it doesn't stand out really well. %= is almost never overloaded, so that makes it a good candidate for coopting for this purpose, and it sort of suggests the wishy-washy nature of try-lock. -- Rob Stewart stewart@sig.com Software Engineer http://www.sig.com Susquehanna International Group, LLP using std::disclaimer;
Rob Stewart <stewart@sig.com> writes:
scoped_lock = move(scoped_lock); scoped_lock |= move(upgradable_lock); // 1
I like the pipe's suggestion of a wall and, hence, blocking. The x= operators are good, in general, as they still suggest assignment (<<= is better than << for that reason).
Problem is that the syntax is illegal. scoped_lock is a type. You can do: scoped_lock x(move(my_upgradable_lock),blocking); x |= move(my_upgradable_lock); but that's it. No constructing new scoped_locks with |= -- Dave Abrahams Boost Consulting http://www.boost-consulting.com
From: David Abrahams <dave@boost-consulting.com>
Rob Stewart <stewart@sig.com> writes:
scoped_lock = move(scoped_lock); scoped_lock |= move(upgradable_lock); // 1
I like the pipe's suggestion of a wall and, hence, blocking. The x= operators are good, in general, as they still suggest assignment (<<= is better than << for that reason).
Problem is that the syntax is illegal. scoped_lock is a type. You can do:
scoped_lock x(move(my_upgradable_lock),blocking); x |= move(my_upgradable_lock);
but that's it. No constructing new scoped_locks with |=
Actually, that's what Howard's pseudo-code syntax meant. -- Rob Stewart stewart@sig.com Software Engineer http://www.sig.com Susquehanna International Group, LLP using std::disclaimer;
Howard Hinnant wrote:
If we introduce "try_move" it could be a similarly simple helper which just returns an auto_ptr-ref-like object tagged with the "try" information. Not that I'm 100% dead set on try_move, I'm not.
the point is that "move" funcion in some situations (construction of temporary object) is not necessary. And when it's necessary (construction from lvalue, taking over it's property) it has well known meaning. Proposed function "try_move" does not have such well-defined meaning, and there is no way you could avoid it (just like we can avoid "move" when creating object from temporary) if we keep this constructor: scoped_lock& operator=(try_rvalue_ref<upgradable_lock<mutex_type> > r);
Yes, but my concern is what does the syntax look like when s4 already exists and you try to upgrade to it by assigning u4 to it:
s4 = u4; // ??? Try to upgrade (non-blocking) u4 to s4
This isn't a hypothetical scenario, I have a use case for it. I've suggested
s4 = try_move(u4);
and am open to other syntax suggestions.
Good point. Maybe upgradable_lock should have member function try_upgrade(), that would try to give away ownership of mutex object to temporary scoped_lock? Then you would have: s4 = u4.try_upgrade(); // non-blocking upgrade You could have also: s4 = u4.upgrade(); // blocking upgrade which would have result identical to: s4 = move(u4); // blocking upgrade Moreover, we could also have: // blocking upgrade with timeout s4 = u4.try_upgrade(some_timeout_value); which is (I think) something new and quite useful?
I believe this still requires Lock to be explicitly supplied in each case. That's doable. But is it sufficiently desirable?
I think so. You need to supply type of constructed lock, but you do not need to know (or supply) type of Mutex class; it's deduced from function parameter. When writing code like: const lock_base& l = lock<scoped_lock>(m); you do not need to know type of "m". You just need to know what template class should be used to construct lock, without taking care of template parameter. Moreover, with some simple SFINAE trickery you could even use above code when "m" is not mutax at all, but upgradable_lock instead (of course this would compile different overload of "lock"). As result, we would have familty of functions that could be used to create lock from mutex and upgrade upgradable_lock. In last case template template parameter <scoped_lock> is somehow redudand, thus it's not really great benefit. On the other hand I think that genericity of such functions is appealing.
Could you flesh this out with a little more complete code. Sorry, I'm just not following. For instance the first line appears to bind an rvalue to a non-const reference. Are we just missing a const on the lock_base& or is there something else going on?
lack of "const" is just my mistake :> See attached file for more complete example.
I think this will have an uphill battle. This code:
{ exclusive_lock some_lock(some_mutex); // assume some_mutex is locked here }
is just too easy to write (and be wrong).
You are definitely right. What about supplying just two constructors: explicit scoped_lock(Mutex&); explicit scoped_lock(Mutex&, deferred_enum); Best regards B. #include <cassert> class lock_base { bool* locked_; void* mutex_; protected: lock_base(bool * locked, void* mutex) : locked_(locked), mutex_(mutex) {} lock_base(const lock_base& src) : locked_(src.locked_), mutex_(src.mutex_) {} ~lock_base() {} public: bool locked() const {return *locked_;} // bool for brevity; should be safe_bool instead operator bool() const {return locked();} bool same(const lock_base& oth) const { return oth.mutex_ == mutex_; } }; bool operator== (const lock_base& lh, const lock_base& rh) { return lh.same(rh); } bool operator!= (const lock_base& lh, const lock_base& rh) { return !(lh == rh); } template <typename Mutex> class scoped_lock : public lock_base { bool locked_; Mutex* mutex_; public: scoped_lock(Mutex& m) : lock_base(&locked_, mutex_), locked_(false), mutex_(&m) { lock(); } // "bool deferred" for simplicity scoped_lock(Mutex& m, bool deferred) : lock_base(&locked_, mutex_), locked_(false), mutex_(&m) { if (!deferred) lock(); } // copy constructor and assignment operator skipped for brevity ~scoped_lock() { if (locked_) unlock(); } void lock() { mutex_->lock(); locked_ = true; } void try_lock() { locked_ = mutex_->try_lock(); } void try_lock(unsigned long timeout_ms) { locked_ = mutex_->try_lock(timeout_ms); } void unlock() { mutex_->unlock(); locked_ = false; } }; class fast_mutex { void lock() {} bool try_lock() {return true;} void unlock() {} friend class scoped_lock<fast_mutex>; }; class waitable_mutex { void lock() {} bool try_lock() {return true;} // "unsigned long timeout_ms" for brevity bool try_lock(unsigned long timeout_ms) {return false;} void unlock() {} friend class scoped_lock<waitable_mutex>; }; template <template <typename _Mutex> class Lock, typename Mutex> Lock<Mutex> lock(Mutex& mutex) { Lock<Mutex> tmp(mutex); return tmp; } template <template <typename _Mutex> class Lock, typename Mutex> Lock<Mutex> try_lock(Mutex& mutex) { Lock<Mutex> tmp(mutex, true); tmp.try_lock(); return tmp; } template <template <typename _Mutex> class Lock, typename Mutex> Lock<Mutex> try_lock(Mutex& mutex, unsigned long timeout_ms) { Lock<Mutex> tmp(mutex, true); tmp.try_lock(timeout_ms); return tmp; } fast_mutex m1; waitable_mutex m2; int main() { scoped_lock<fast_mutex> l1(m1); // I do not need to know type of "m1" here in order to create lock for it const lock_base& l2 = lock<scoped_lock>(m1); { // I do not need to know type of "m1" and even if it supports "try_lock" const lock_base& l3 = try_lock<scoped_lock>(m1); if (l3) { ; // got m1 } } { // I do not need to know if m2 is waitable; if it isn't // there would be just compilation error const lock_base& l4 = try_lock<scoped_lock>(m2, 200); if (l4) { ; // got m2 } if (l4 == l1) ; // well, this time mutexes are not equal } }
Bronek Kozicki wrote:
explicit scoped_lock(Mutex&, deferred_enum);
It's really disturbing - I'm trying to write something useful, it takes me quite some time, and then I find such stupid mistake like "explicit" in front of constructor taking two parameters. Sorry for that B.
participants (8)
-
Batov, Vladimir -
Bronek Kozicki -
David Abrahams -
Eric Niebler -
Howard Hinnant -
Michael Glassford -
Peter Dimov -
Rob Stewart