Re: Review: Boost.Threads

From: williamkempf_at_[hidden]
Date: 2001-09-01 13:29:47

--- In boost_at_y..., Beman Dawes <bdawes_at_a...> wrote:
> I believe William Kempf's Boost.Threads submission should be 
accepted by 
> Boost.
> 
> Bill deserves a great deal of credit for patiently working through 
a large 
> number of issues, for discussing the issues at length and over an 
extended 
> period of time, and for making changes to the library when 
indicated.

Thanks.  I think I've honed several skills I was lacking in when this 
all started.  Got a ways to go yet, but this process has been quite 
invigorating.

> A really large number of people helped him.  Be sure to look at the 
> Acknowledgements page in the docs.

A large number isn't the word for it!  None of this could have been 
done with out everyone's input.  I well know that not everyone is 
going to be satisfied with the result (this was one of the reasons 
I've heard given for not trying to include a thread library in the 
original standard), but because of the strong discussions we had on 
here it's hopefully at least a good starting point.  Whether or not 
the library is accepted I'm grateful for the opportunity to work with 
everyone on this.  Thanks aren't enough, and I truly appreciate 
everyeone's input.
 
> Comments
> --------
> 
> * The design goals seem to have been met, including the safety 
goals that 
> are particularly important to me.
> 
> * While a number of alternate designs for portions of the library 
have been 
> discussed, none seem enough better to warrant delay.  The 
alternatives have 
> been beaten to death and it is time to move forward.

Moving forward should include implementations of alternate designs on 
top of the existing design.  This will help to insure that there are 
no holes in the existing design and allow people to evaluate 
alternatives.
 
> * In general, the design seems of reasonable size and functionality 
to 
> me.  While there may be legitimate questions about the inclusion of 
some of 
> the classes (see next comment), the design has not become bloated 
with 
> marginally useful features and functions as happens all too often.
> 
> * I share Peter Dimov's concerns about semaphore.  A long time ago 
I asked 
> on comp.programming.threads if Boost should avoid any features as 
too error 
> prone.  The one suggestion I got was to avoid semaphores.  While 
semaphore 
> is documented in Boost.Threads as dangerous, perhaps we should 
remove it 
> entirely or move it to an implementation namespace.

I've found semaphores to be indispensible for building some higher 
level constructs.  However, one can argue that the higher level 
constructs should be provided by the library to begin with and 
including this type as an implementation detail will allow this to be 
done.  So I guess I don't have overly strong feelings one way or 
another about this.
 
> * There are many functions where a reasonable implementation may 
wish to 
> allocate memory.  These should be documented as potentially 
throwing 
> std::bad_alloc.  Like the standard library, it seems to me this can 
best be 
> done with a blanket "any function may throw std::bad_alloc" 
statement 
> rather than try to guess which functions an implementation may wish 
to do 
> dynamic memory allocation.

I'd agree.  Any suggestions about where in the documentation to put 
this blanket statement?
 
> * It seems to me that some implementations may run out of resources 
(other 
> than memory) in the constructors for almost any of the Boost.Thread 
> classes. Thus I think there should be a blanket statement that all 
> constructors can potentially throw thread_resource_error.  If a 
blanket 
> statement isn't included, at least semaphore and the mutex 
constructors 
> seem to me to need to allow throwing thread_resource_error.

Unfortunately I didn't document this one properly.  Add this to the 
list of things to correct.
 
> * class thread_resource_error needs documentation.
> 
> * class xtime needs documentation.

Both were supposed to be documented, and I failed on this one.  
Things came up at work after we'd set the deadline and I guess I just 
juggled things poorly.  I'll try to fix this stuff this weekend so 
others can comment on the changes before the review is over.
 
> * A tutorial would be helpful, although certainly not necessary 
right away.

Agreed.
 
> * Jens Maurer's bounded queue should be included at least as sample 
code if 
> not as part of the library itself.

The "monitor" example is already an example of a bounded queue.  If 
we want to leave the bounded queue solely as an example then I'll 
just include Jens's refinements in this example.  I'd suggest leaving 
it as an example, because a queue isn't really a threading concept.
 
> * Once Boost.Threads is accepted, and thus the interfaces stable, 
let's 
> quickly try to find implementors for platforms other than Win32 and 
Linux, 
> and get them to contribute their implementations to Boost.

Yes!

> * I've used (an earlier version of) the Boost.Threads code to 
produce a 
> multi-threaded version of an existing industrial program.  It ran 
correctly 
> right away, and was 40+ percent faster on a dual processor machine 
than the 
> single threaded version.  I'd never done any multi-threading 
previously.

This is exciting.  I'd love to hear anyone's stories about usage of 
this library (good and bad experiences).  You can share with the list 
or send them directly to me. :)
 
> * I haven't reviewed the Boost.Threads code in detail, but what I 
have 
> looked at seems reasonable and meets our agreed upon guidelines.  
Hopefully 
> some other reviewer will go over the code with a fine-toothed comb.

Especially those with MT experience!

Thanks,
Bill Kempf