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

From: Yigong Liu (yigongliu_at_[hidden])
Date: 2007-07-21 04:00:01

Hello,

On 7/20/07, Mathias Gaunard <mathias.gaunard_at_[hidden]> wrote:
>
>
> Shouldn't the scheduling policy be part of the type though? That would
> allow usage of compile-time polymorphism instead of run-time one.

Indeed, i thought about separating the dispatching / scheduling logic out as
separate "policy" classes, and never be able to get back to it. thanks for
pointing it out and I'll consider it.

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

Could you point out which libraries' documents you are mostly fond of? so i
can have a clear idea.

> 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?

 Not all actors need the executors thread pool to run its chord bodies. If
the chord header contains a synch method, the chord body will run in the
calling thread of synch method; only when all methods in the chord header
are async, the chord body has to be dispatched to another thread (maybe in
pool, maybe Asio's completion queue) to execute. So executor only need to be
specified when the actor class contains a chord, all of whose methods are
async. That is why i am using a pointer to executor in actor's constructor,
its default value is NULL, meaning no executor is need. For example, class
buffer only has one chord which has a synch method, so no executor is
specified.

Why are pointers so much used everywhere in the implementation even when
> references seem more suited?

I have been trying to use references where it is appropriate, for example,
all ports are stored in chords' as references. I could miss something, could
you point out the exact places?

It also seems to me like some places lack const. Do you consider your
> code const-correct?

Again could you please point out the exact places in code?

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'll check out boost::ptr_vector to see if it is more suitable. My other
consideration in implementation is using (or confining to) TR1's libraries
as much as possible, since TR1 will become standard first and more widely
available.

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?

I'll look into it and clean them up.

Thanks for your comments.
Yigong