Boost logo

Boost :

From: williamkempf_at_[hidden]
Date: 2001-03-15 10:02:19

--- In boost_at_y..., "Dan Nuffer" <dnuffer_at_c...> wrote:
> 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
> > helping out with:
> >
> > 1) Evaluate the code and documentation much as you would for a
> > 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
> worked on for the past 3 years has had it's own implementation of
> threads, and I think it sure would be nice to have something to
> on.
> Anyway, I looked at all the code, and documentation, and here are
some of my
> thoughts.

Thanks very much for your interest and comments.

> 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
> 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:
> tswmr.c?rev=1.2&content-type=text/x-cvsweb-markup&cvsroot=cdsa

Read/write locks are trivial to implement on any platform. They
weren't forgotten here, they were just considered to be of a higher
level of complexity with lesser actual need and so were left out in
Phase 1. I expect to add them in Phase 3. The introduction in the
documentation will help you to understand why, though this type was
not specifically mentioned.

> Another issue is that of how the code is organized. This appears
in most of
> the source files:
> all the windows code
> all the pthread code
> #endif
> I think it greatly helps readability of the source to separate the
> code into one file and the pthread code into another file. Then
> 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.

I originally did this, and refactored the code right before
submitting this draft. I actually found the file seperation to be
more difficult to maintain and it did cause a (very minor) reduction
in compile time. I'm not against this seperation, but before going
back to it I'd appreciate comments from others.

> 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
> compilation.

I'm less convinced that this would be beneficial. The includes are
all implementation details and don't appear in the header files at
all. So the only people that will see this at all are those
developing/maintaining the library. If the structure is left as is,
with the conditional compilation in one file for platform specific
code as you pointed out above, this extra conditional detracts
nothing to the over all readability of the source. If the
implementations are seperated into platform specific source files the
conditional naturally disappears since the file inclusion is moved to
the platform specific file.

> 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
> with limits of 0 and 1.

Mutexes are also, at least conceptually, restricted to lock/unlock
pairs within a single thread. Many libraries enforce this with it
being an error to unlock a mutex in a thread that did not lock it. A
semaphore, on the other hand, is not restricted in this manner at all.

That said, if the wording here still seems misleading or inacurate
I'm willing to change it.

> You also mention that the semaphore is dangerous, but don't explain

The semaphore isn't, only the interface given for it is. I thought
this was appearant, but I'm too close to this stuff now to be
objective. I'll try and rework this section of the documentation.

> The docs don't explain the arguments to the semaphore constructor.
> understood max, but count wasn't too obvious, is it the initial
value of the
> semaphore?

Yes. Another oversight. I'll rework this section as well.

> 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
> wait and signal.

Actually, the Windows API does not. Waiting is done through the
generic WaitFor* APIs, which could be taken to be the same as "wait"
since the word is found in the longer name, but the "signal"
functionality is named ReleaseSemaphore which has no relation at
all ;).

The point is, the only "common" names for these methods are P and V,
which make little sense to someone unfamiliar with the concept. It
was decided (and not by me) early on during the design of this
library that up and down convey the actual functionality better than
any other names. I'm not stuck with this, but because it's already
been discussed I think you'll find few people who want to change the
names here. So, with out compelling reasons why, I think the names
will stay.

> The example you provided for the semaphore is more appropriate for
a mutex.
> I think that a bounded buffer would show the semaphore concept

I very much agree that the example is contrived and does not show the
benefits of the semaphore at all. I don't think a bounded buffer is
a better example, however, as that example is better suited for (and
given in the documentation for) a condition. I'll think about a
better example for this one.

As I think you can tell, I put little effort into the documentation
for the semaphore, being more concerned with the mutexes and
condition. I wrote the docs for the semaphore near the end, when I
was sick of writing documentation ;).

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

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

I did not say they "shouldn't", just that they normally "won't". The
types are useful for developing other mutex concepts, and as such I
don't want to place them in a detail namespace. Maybe I can convey
this better in the documentation... I'll give that some thought.
> 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);
> }

This would require the "impl" class to be visable to client code,
which is exactly the opposite of the purpose. The implementation is
supposed to be hidden completely from client code. Specifically, I
don't want client code to include files such as Windows.h, even
indirectly, since this will polute the namespace and slow
compilation. All refactoring that will place these functions in the
header and yet insure my goal of hiding the implementation will
result in no speed increase from inline optimizations.
> 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
> 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
> faster compile?

It's more than "slightly" on many platforms, Windows being one of
them. Further, I don't want to pollute the namespace. The only
overhead occurs within construction/destruction, which are not likely
to occur in time critical locations and so shouldn't be noticed in
most applications.
> > 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
> > 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

I'd appreciate the help. We need to coordinate things like this so
that multiple people aren't working on the same thing. Are you
volunteering for specific platforms?

Thanks for the comments.

Bill Kempf

Boost list run by bdawes at, gregod at, cpdaniel at, john at