Re: [boost] Comments about N2320

From: Howard Hinnant (hinnant_at_[hidden])
Date: 2007-10-30 14:23:35

On Oct 28, 2007, at 11:53 AM, Yuval Ronen wrote:

> Hi,
> After reading most of N2320, I have a few comments:

Thanks for taking the time to do this.

As was mentioned by Jeff, the current public draft is:

http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2411.html

and this draft is known to be somewhat incomplete and incorrect (it  
was created at the last meeting site under severe time constraints).   
A revision to it is underway...

> * thread::id has operator== and operator!= declared twice. Once in
> namespace scope, and a second time inside class thread. Second time is
> with operator< and his friends.

<nod>

> * Thread cancellation is new, and
> disable_cancellation/restore_cancellation are even newer. They are new
> for C++ programmers, and maybe new for *all* programmers (I never  
> heard
> of a language with them). I'm not sure if it's a good idea to
> standardize them before we get some real-life experience with thread
> cancellation.

Actually thread cancellation is old.  Many (not all) thread API's  
have some form or another of cancellation.  That being said, the  
compromise reached at the Kona meeting 4 weeks ago was to remove  
cancellation (or interruption) from the proposal.  This removal is  
reflected in N2411.

> * I see that you chose not go accept a mutex in condition's  
> constructor.
> If that is so, why make the condition class templated, instead of only
> the wait() functions, as is in Boost.Thread?

There was an interim paper:

http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2406.html

which changed the way cv's were handled from N2320.  In Kona this was  
folded into N2411, and also renamed along the way.  The condition's  
constructor still does not take a mutex.  There are now two condition  
variable classes, and neither is templated.  The rationale for this  
design is in N2406.

> * If C++0x is going to have lambda support (is it?), then maybe the
> not-accepting-predicate overloads of condition::wait are no longer
> needed, and can be removed? I think the major reason why people use  
> that
> overload is because they are too lazy to write a functor. With core
> support for lambda, it's a breeze. This will solve the other problem
> (mentioned in some other thread in this ML) about absolute vs.  
> relative
> times.

I would be very hesitant to force one programming style over  
another.  Sometimes, for whatever reasons, one might really need an  
explicit loop (say to handle more complex flow control than just  
while (!pred) cv.wait(lk)).

> * Back then we had a discussion about unique_lock, which was never
> finished. I claim that there is no need for a state where mutex() != 0
> && owns() == false, which means that defer_lock is not needed.  
> There is
> one example in the document using defer_lock, which I think can be  
> done
> in another way. That example goes like:
>
> Record&
> Record::operator=(const Record& r)
> {
>      if (this != &r)
>      {
>          std::unique_lock<std::mutex> this_lock(mut, std::defer_lock);
>          std::unique_lock<std::mutex> that_lock(r.mut,  
> std::defer_lock);
>          std::lock(this_lock, that_lock);
>          // Both source and destination are locked
>          // Safe to assign
>          // ...
>      }
>      return *this;
> }
>
> If we change the first 3 lines of the 'if' to:
>
> std::lock(mut, r.mut);
> std::unique_lock<std::mutex> this_lock(mut, std::accept_ownership);
> std::unique_lock<std::mutex> that_lock(r.mut, std::accept_ownership);
>
> then it works without defer_lock.

<nod> However I'm not comfortable with claiming that defer_lock has  
no use beyond this use case, especially when defer_lock-support costs  
nothing (see below).  This is especially true given that  
boost::thread has supported the defer_lock functionality (under a  
different spelling) for six or seven years and this is the first  
report I've seen that the functionality is sufficiently complicating  
that it should be removed.

> This allows us to:
> 1. Make unique_lock's interface simpler.
> 2. Make sizeof(unique_lock) smaller - no need for bool owns.

Even if we remove defer_lock, unique_lock will still need the  
internal bool to support try and timed locks.  This functionality  
still retains a reference to the mutex even if the try/timed lock  
fails (so that further action with the mutex can be easily taken).

> 3. Make unique_lock more similar to unique_ptr, which makes it more
> intuitive.
> 4. Make std::lock simpler in the way that it doesn't need to accept
> locks, only mutexes.

std::lock isn't further complicated by accepting both mutexes and  
locks.  It accepts anything that supports lock(), try_lock() and  
unlock().  To disallow locks, we would have to make std::lock more  
complicated by detecting lock template arguments and then actively  
disabling them.  I see no motivation to add this complication.

> In general it seems you paid a lot of attention to being able to  
> pass a
> lock instead of a mutex wherever possible. I think it's absolutely not
> necessary. It's an undue complication.

Actually I tried to pay attention to what the minimal requirements  
were on template parameters.  It just turned out that some generic  
code would accept either locks or mutexes.

> * Other than that - very nice :)

Thanks.  Sorry the paper keeps changing out from under you.  There  
are many people actively working a single document now and such  
changes are inevitable.