Re: [boost] a new release of the Join asynchronous concurrency library, please schedule a review

From: Mathias Gaunard (mathias.gaunard_at_[hidden])
Date: 2007-07-20 10:59:04

Yigong Liu wrote:

> I am not sure which specific places in code you are referring to.

> Boost.Function is a generalization of function pointers and callbacks, which
> could be free functions, methods, result of boost.bind() and of course
> function objects

boost::function<signature> is simply a polymorphic wrapper that allows 
to contain any type that is callable with the signature 'signature'.
It's only useful if you really need a single unique type for your 
function objects.

It introduces quite some overhead over using a function object directly.
I was simply wondering if it was really needed to have that facility.

It seems you need it here though because actor<>::capture_events is 
actually virtual. (why it is virtual, I do not know)

> The question is kind of vague so i can only reply in general. Each object
> which define async /synch methods and chords will use only one mutex to
> protect the internal synchronization status and all data in async / synch
> methods. Besides this mutex, each synch method also has a condition variable
> for possible blocking of calling threads.
> When messages come in and more than one chords become ready to fire, there
> are 3 kinds of scheduling to decide which chord will fire and consume the
> messages: fire the first chord which is ready, fire the ready chord with
> most methods which will consume most buffered messages,  and round-robin
> scheduling.

I see.
Shouldn't the scheduling policy be part of the type though? That would 
allow usage of compile-time polymorphism instead of run-time one.

> You can find all the above info and more details in the design document.

Indeed, however it's not so easy to navigate that document.
Maybe the documentation could be improved. You might want to look at the 
latest documentations for some boost libraries, which have started to 
become quite usable.

> Again i am not sure if this is exactly what your question is about. If not,
> please clarify your questions a little bit.

That was more or less it.

I have a a few rather random comments/questions, mostly stylistic, after 
I browsed the code for a few minutes:
(I admit I didn't understood how the code works at all, unfortunately I 
do not have the time to study it at the moment)

Why isn't executor passed by reference instead of by pointer in actor's 
constructor?
Why are pointers so much used everywhere in the implementation even when 
references seem more suited?
It also seems to me like some places lack const. Do you consider your 
code const-correct?

I saw that line in the code:

std::vector<boost::shared_ptr<chord_type> > chords_; //actor owns 
chords_ and will destroy them

shared_ptr is better used for sharing. If actor owns the chords_, 
boost::ptr_vector may be more suited.

I see that your code does some C-style casts with non-PODs, and quite a 
few of them.
These are great sources of unsafety. Aren't there ways to not need them, 
or at least to restrict them to one place?