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