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

bool foo(bool threadsafe) { mutex::scoped_lock l(m, !threadsafe); }
I am not sure if the example above is appropriate and indicative of the problem we are trying to solve. Thread-safety should not be a run-time configurable property. Like we do not decide on the fly (at run-time) if our ref.-counting class is to be thread-safe or not. All that is resolved at compile time. More so, that still can be solved with statically defined lock/nolock constructors: bool foo(bool some_condifion) { mutex::scoped_lock l(m, boost::nolock); if (some_condition) l.lock(); } 2. I feel that our assumptions of try_lock (yes, I know it is try_scoped_lock, I am just lazy) and timed_lock being refinements of scoped_lock are incorrect. At best scoped_lock and try_lock are refinements of timed_lock as scoped_lock is timed_lock with time=infinite and try_lock is timed_lock with time=0. Therefore, we you insist on common lock functionality, it has to be encapsulated in one timed_lock (or maybe some basic_lock or just a lock). Then, all locks would refine that common behavior. That approach has the advantage of covering the case when I do not know what I want :-) -- then I'd simply use the basic_lock. 3. However, I personally feel that our attempts of generalizing locks are misguided. Locks are different, serve different purposes and provide different and distinct functionality. I feel that we should make an effort to keep different locks different and lean. Like try_lock actually tries to lock. Adding blocking (lock()) functionality to such a lock looks like a mistake as IMHO it encourages bad programming style as it allows to use try_lock where scoped_lock is needed. I feel that the situation when I do not know what kind of lock I need when I step into a function shows that the function needs to be-restructured/split so that in every new component I do know what I need. V.

Batov, Vladimir wrote:
bool foo(bool threadsafe) { mutex::scoped_lock l(m, !threadsafe); }
I am not sure if the example above is appropriate and indicative of the problem we are trying to solve. Thread-safety should not be a run-time configurable property. Like we do not decide on the fly (at run-time) if our ref.-counting class is to be thread-safe or not. All that is resolved at compile time. More so, that still can be solved with statically defined lock/nolock constructors:
bool foo(bool some_condifion) { mutex::scoped_lock l(m, boost::nolock);
if (some_condition) l.lock(); }
I think that you have that backwards. If the only use case for 'nolock' is the example above, then the bool argument is obviously an improvement. The 'nolock' version needs a rationale, not the bool version.
2. I feel that our assumptions of try_lock (yes, I know it is try_scoped_lock, I am just lazy) and timed_lock being refinements of scoped_lock are incorrect. At best scoped_lock and try_lock are refinements of timed_lock as scoped_lock is timed_lock with time=infinite and try_lock is timed_lock with time=0. Therefore, we you insist on common lock functionality, it has to be encapsulated in one timed_lock (or maybe some basic_lock or just a lock). Then, all locks would refine that common behavior.
No, this is not correct. timed_lock is a refinement on both lock and try_lock, because it is-a lock (with time=infinite) and a try_lock (with time=0). The reverse is not true.

Peter Dimov wrote:
Batov, Vladimir wrote:
bool foo(bool threadsafe) { mutex::scoped_lock l(m, !threadsafe); }
I am not sure if the example above is appropriate and indicative of the problem we are trying to solve. Thread-safety should not be a run-time configurable property. Like we do not decide on the fly (at run-time) if our ref.-counting class is to be thread-safe or not. All that is resolved at compile time. More so, that still can be solved with statically defined lock/nolock constructors:
bool foo(bool some_condifion) { mutex::scoped_lock l(m, boost::nolock);
if (some_condition) l.lock(); }
I think that you have that backwards. If the only use case for 'nolock' is the example above, then the bool argument is obviously an improvement. The 'nolock' version needs a rationale, not the bool version.
The primary rationale for the struct-based nolock is to prevent the possibility of meaningless combinations of parameters that can result if a constructor accepts both an initial lock state parameter and a blocking mode parameter, like this: try_lock(m, false, blocking); Another possible rationale is to prevent redundancy, where "lock(m)" and "lock(m, true)" mean the same thing, for example. I consider this to be pretty unimportant, however. Another possible rationale is that the compile-time choice of a constructor that does only one thing is probably more efficient than the run-time choice within the constructor of what operation to perform (e.g. whether to lock or not), but the difference is probably so small as to be practically meaningless. So, all things considered, I guess I agree that advantages of the struct-based approach don't compensate for the the loss of run-time flexibility given by the bool. Though, in passing, I still prefer an enumeration to a bool, both because its meaning is more explicit, and because it prevents silently changing the meaning of existing code; i.e., if we change "try_lock(m, true)" from its current meaning of a blocking lock to the more obvious meaning of a non-blocking lock, it would also be better to change the bool to an enum, so that you have to type "try_lock(m, unlocked)" or "try_lock(m, locked)", and existing occurrences of "try_lock(m, true)" result in compile errors.
2. I feel that our assumptions of try_lock (yes, I know it is try_scoped_lock, I am just lazy) and timed_lock being refinements of scoped_lock are incorrect. At best scoped_lock and try_lock are refinements of timed_lock as scoped_lock is timed_lock with time=infinite and try_lock is timed_lock with time=0. Therefore, we you insist on common lock functionality, it has to be encapsulated in one timed_lock (or maybe some basic_lock or just a lock). Then, all locks would refine that common behavior.
As Peter points out, this is incorrect because in Boost, "concept" and "refinement" are technical terms with specific meanings: a concept is a set of requirements; and a refinement is another concept that includes the same requirements as the original concept, plus additional requirements. In this technical sense, the TimedLock concept (as currently defined) is a refinement of the Lock concept (as currently defined), not the other way around, and there's no sensible way to define a Lock concept or a TryLock concept that is a refinement of the TimedLock concept.
No, this is not correct. timed_lock is a refinement on both lock and try_lock, because it is-a lock (with time=infinite) and a try_lock (with time=0). The reverse is not true.
Having said that, I see Vladimir's meaning and agree with it: the idea (since I can't use the word concept) of a Lock is a special case of the idea of a TimedLock, with t = infinity; and the idea of a TryLock is a special case of the idea of a TimedLock, with t = 0. That doesn't necessarily translate into the definitions of the Boost.Threads locking concepts or lock class definitions, however, as those definitions also must take other things into account: efficient implementation, existing threading APIs, etc. Mike
participants (3)
-
Batov, Vladimir
-
Michael Glassford
-
Peter Dimov