Re: [boost] [afio] Formal review of Boost.AFIO

Subject: Re: [boost] [afio] Formal review of Boost.AFIO
From: Thomas Heller (thom.heller_at_[hidden])
Date: 2015-08-28 01:47:59

On 08/27/2015 04:04 AM, Niall Douglas wrote:
<snip>
A lot of things about future vs. shared_future...
</snip>
>
> Thoughts?

Ok, after reading all the other messages in this and other threads I am 
starting to understand what the real problem is (all this is IMHO, of 
course):

The biggest mistake in your design is that of the functions taking 
"preconditions". This brought you all this mess. The problem is, those 
are actually not preconditions but actually arguments to the function, 
more specifically, your "precondition" is always (?) the handle (or a 
future to the handle so to speak) to the actual operation that is being 
performend for which you need to get the value. That all is a result of 
the non-existing single responsibility pattern in your code. After 
getting rid of that, you will see that most of the points you brought up 
against using std::future (or boost::future) will actually go away.
Here is my concrete suggestion:
  - Let your async functions return a future to their result! (Not a 
tuple to the handle and the result...)
  - Don't use futures as arguments to your async functions. Say what you 
want as input!

Let's take on your read example... and suppose we have the following two 
function signatures:

future<handle> open(std::string name);
future<std::vector<char>> read(handle const & h, std::size_t bytes, 
std::size_t offset);

See how easy, clear and succinct those functions are? Almost self 
explaining without any further documentation.

Now, on to your example about a 100 parallel reads, so let's encapsulate 
it a bit to concentrate on the important part:

future<void> read_items(handle const & h)
{
     vector<future<vector<char>>> reads;
     reads.reserve(100);
     for(std::size_t i = 0; i != 100; ++i)
     {
         reads.push_back(async_read(h, 4096, i * 4096));
     }
     // handle reads in some form ... for simplicity, just wait on all
     //of them ...
     return when_all(std::move(reads));
}

No continuations, cool, eh?
(Well, that's a lie, when_all attaches continuations to the passed 
futures as well as the conversion from the result of when_all to 
future<void>).

In order to call it, you now have several options, as already outlined 
in another mail:
1. read_items(open("niall.txt").get());
2. open("niall.txt").then([](future<handle> fh){ return 
read_items(fh.get());
3. read_items(await open("niall.txt"));

As you can see, the question about shared_future vs. future doesn't even 
arise!
Was it already mentioned that an rvalue (unique) future can be 
implicitly converted to a shared_future?

Now I hear you saying, but what if one of my operations depend on other 
operations? How do I handle those preconditions?
The answer is simple: Use what's already there!
Utilities you have to your disposal are:
  - when_all, when_any, when_n etc
  - .then

This allows for the greatest possible flexibility and a clean API!

So to repeat myself, as a guideline to write a future based API:
1. You should always return a single (unique) future that represents the 
result of the asynchronous operation.
2. Never return shared_future from your API functions
3. Use the standard wait composure functions instead of trying to be 
clever and hide all that from the user inside your async functions.

(NB: In HPX we have one additional function called dataflow, it models 
await on a library level and executes a passed function whenever all 
input futures are ready, this reminded me a lot of your "preconditions").

>
> Niall