Re: [boost] Futures Review Starts Today - January 5, 2009

Subject: Re: [boost] Futures Review Starts Today - January 5, 2009
From: vicente.botet (vicente.botet_at_[hidden])
Date: 2009-01-18 17:56:59

Hi, here is my review. 

* What is your evaluation of the design?

Both libraries have a good design. The fact that the porposal of Anthony is closer to the C++0x proposal let me think that we should base the Boost.Library on it and see how to add the other features.

For the exception propagation mechanism both libraries must use Boost.Exception (as is already the case of Anthony version).

The library should use the forthcomming Boost.Move library to emulate the move semantics.
The library should use the forthcomming Boost.Chrono library for time and duration.

With wait_for_all and wait_for_any we don't need the Braddock completion callback setting, even if it could be less efficient, it is safer.

I have however, some points that seems to me necessary for acceptance:

++ C++0x adaptations:
    ++ adapt the exceptions hierarchy to a single exception.
    ++ Change the names of timed_wait to follow the  stndard wait_until and wait_for.
+ Add a typeof registerig (See http://www.nabble.com/forum/ViewPost.jtp?post=21388525&framed=y)

++ Allows partial specialization of wait_for_any (see http://www.nabble.com/forum/ViewPost.jtp?post=21504868&framed=y) 

+ Use a get_future traits on the default implementation of wait_for_any and wait_for_all

++ Futures are asynchronous completion tokens. I would like the library defines this concept. Functions like wait_for_any, wait_for_all should work with every model of an ACT. If the implementation could not provide this, these functions must be moved to a specific future namespace.

+ The one or many wait callbacks setting subject come back regularly. I would like the Boost.Future future library supports wait callbacks but not always, i.e. I would like a design that don't supprots callbacks (current standard recomendation). On top of it we must be able to implement 1-callbacks, and on top of it implement n-callbacks (or signals).

+ Add wait_for_ functions be overloaded for fusion sequences, e.g.

fusion::vector<shared_future<int>, shared_future<string> > seq;

unsigned i = wait_for_any(seq);

See http://www.nabble.com/forum/ViewPost.jtp?post=21517138&framed=y

+ Add a typedef or a trait class to get the value type of a future

- No implicit conversion must be provided

+ It will be great to have a DSEL defining the operators && and || as proposed by Hartmut Kaiser and others.

* What is your evaluation of the implementation?

Both implementations seem to be of good quality. Only one deep remark:
++ take in account my proposal on wait_for_any complexity (See http://www.nabble.com/forum/ViewPost.jtp?post=21494006&framed=y)

and some minor ones (see http://www.nabble.com/forum/ViewPost.jtp?post=21320253&framed=y)

* What is your evaluation of the documentation?

I have spent quite a lot of time reviewing them, and both seem fine but William's should add a more extensive tutorial and examples that show to the user how the most useful uses cases of the library can be done without dificulty. In general it will be good to see how the specific features of the Braddock proposal are managed by the standard proposal.

+ A Reference to the ThreadPool library documentation could see how complex a thread_pool can be .

+ Add an example of use with containers of futures and why not with fusion sequences.

+ Add a schedule with dependencies example (wait_for_any,wait_for_all).

+ Add an example of scheduling a function taking futures as parameters.

I think that this documentation should be added before complete acceptance.

* What is your evaluation of the potential usefulness of the library?

Extremely useful. This will be the base for a more high level concurrency libraries as ThreadPool.

* Did you try to use the library?  With what compiler?
  Did you have any problems?

I have used the Anthony version, I used gcc 3.4.4 with cygwin and I have found just the problem with the move of packaged_task.

My library interthreads use the Anthony Future library for the asynchronous execution framework.

* How much effort did you put into your evaluation?
  A glance? A quick reading? In-depth study?

I've spent a lot of hours.

* Are you knowledgeable about the problem domain?

Quite. I've used the ACE futures on production code since more that 5 years.
I have proposed changes on the wait_for_any function improving the complexity.
I have implementated an asynchronous execution framework recently.

* Do you think the library should be accepted as a Boost library?

The fact that Olivier has adapted its ThreadPool library without not too much problems and that I was able to use the futures as ACT in my ae-act framework let me think that the interface is quite stable.

My fork_after function is a prove of concept that we can schedule executions once a number of ACT are completed (I have no performance measures).

My vote will be Yes for the Anthony proposal once we find consensus on my suggestions and the documentation is completed.

So only provisional acceptance.

Vicente