Re: [boost] Boost.Threads draft library submission

From: Dan Nuffer (dnuffer_at_[hidden])
Date: 2001-03-14 22:29:13

William Kempf wrote:
> Despite the delay I've uploaded a _DRAFT_ library submission for
> Boost.Threads.  It's not 100% ready for prime time, but it's ready to
> share.  Below is a list of things I'd appreciate others on this list
> helping out with:
>
> 1) Evaluate the code and documentation much as you would for a formal
> submission.  This particular library is going to need a bit more
> scrutiny than normal, so I'd like to do some of this prior to
> submission.
>

I have a vested interest in Boost.Threads, as almost every project I've
worked on for the past 3 years has had it's own implementation of C++
threads, and I think it sure would be nice to have something to standardize
on.
Anyway, I looked at all the code, and documentation, and here are some of my
thoughts.

One of the most commonly used syncronization primitives in the projects I
have worked on has been a read-write lock.  I was surprised that it wasn't
implemented, and I think it should be part of the library.  They are trivial
to implement for pthreads, and if you want to see an example of how to do
them on windows here is a link to some code of CDSA that does it:
http://cvs.sourceforge.net/cgi-bin/cvsweb.cgi/cdsa_dev/cdsa/src/fwk/port/por
tswmr.c?rev=1.2&content-type=text/x-cvsweb-markup&cvsroot=cdsa

Another issue is that of how the code is organized.  This appears in most of
the source files:
#if defined(BOOST_HAS_WINTHREADS)
all the windows code
#elif BOOST_HAS_PTHREADS
all the pthread code
#endif

I think it greatly helps readability of the source to separate the windows
code into one file and the pthread code into another file.  Then when
compiling the library, the appropriate file is compiled.  You can even help
out organization by creating separate directories such as src/windows and
src/pthread and putting the respective files under each directory.
You also have a lot of places where you either include windows.h or
pthread.h.  The code would be cleaner if you created a separate file maybe
called platforminc.hpp which has the #ifdef to include either windows.h or
pthread.h and then in all your code you just include platforminc.hpp.  The
goal is to clarify the code by minimizing the number of spots of conditional
compilation.

In the semaphore docs you mention that the semaphore is the simplest
primitive.  It seems to me that a mutex is simpler than a semaphore,
although I guess in reality it's the same thing, a mutex is just a semaphore
with limits of 0 and 1.
You also mention that the semaphore is dangerous, but don't explain why.
The docs don't explain the arguments to the semaphore constructor.  I
understood max, but count wasn't too obvious, is it the initial value of the
semaphore?
I don't like the names up and down for the semaphore member functions.  I
haven't ever seen this terminology used.  In college, I learned about wait
and signal operations on semaphores, and I believe that is the most
prevalent usage (other than p and v)  Even the windows API uses the terms
wait and signal.
The example you provided for the semaphore is more appropriate for a mutex.
I think that a bounded buffer would show the semaphore concept better.

In mutex_concept.html under the Locking Strategies heading, the first
sentence of each strategy is hard to understand.  Maybe there is a better
wording?

You mention that the user shouldn't include <boost/xlock.hpp> to further
enforce the idea, I think you should move it into boost/detail

I think you should move a lot of the simple 1-3 line functions from the .cpp
files into the .hpp files such as:

    semaphore::semaphore(unsigned count, unsigned max)
        : pimpl(new impl(count, max))
    {
    }

    semaphore::~semaphore()
    {
        delete pimpl;
    }

    bool semaphore::up(unsigned count, unsigned* prev)
    {
        return pimpl->up(count, prev);
    }

    bool semaphore::down(unsigned milliseconds)
    {
        return pimpl->down(milliseconds);
    }

Also, I wonder if the advantages of the pimpl idiom are greater than the
disadvantages?  I definitely think it will cause a performance hit.
Everytime you lock or unlock a mutex, you've got to dereference the pimpl
pointer.  And for every primitive you create you've got to call new and then
delete when it goes away.  Is there any advantage other than a slightly
faster compile?

>
> 3) I need serious help in creating some sort of build system.  I
> realize this is a touchy subject since Boost doesn't yet have a
> standard way of handling this, but I'll settle for multiple platform
> specific makefiles much as Regex does.  I'm just not qualified to
> handle this part.
>

I can probably help you out on that.  Let me see what I can come up with.

--Dan Nuffer