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

Subject: Re: [boost] [afio] Formal review of Boost.AFIO
From: Agustín K-ballo Bergé (kaballo86_at_[hidden])
Date: 2015-08-30 17:04:20

On 8/30/2015 5:06 PM, Niall Douglas wrote:
> On 30 Aug 2015 at 15:05, Agustín K-ballo Bergé wrote:
>
>> On 8/30/2015 1:01 PM, Niall Douglas wrote:
>>> I appreciate that from your perspective, it's a question of good
>>> design principles, and splashing shared_ptr all over the place is not
>>> considered good design. For the record, I*agree*  where the overhead
>>> of a shared_ptr*could*  be important - an*excellent*  example of that
>>> case is std::future<T> which it is just plain stupid that those use
>>> memory allocation at all, and I have a non memory allocating
>>> implementation which proves it in Boost.Outcome. But for AFIO, where
>>> the cost of a shared_ptr will always be utterly irrelevant compared
>>> to the operation cost, this isn't an issue.
>>
>> Let's get this memory allocation concern out of the way. One just can't
>> have a conforming implementation of `std::future` that does not allocate
>> memory. Assume that you could, by embedding the storage for the result
>> (value-or-exception) inside either of the `future/promise`:
>
> I'll just limit my comments to your text to what my Boost.Outcome
> library does if that's okay. I should stress before I begin that I
> would not expect my non-allocating futures to be a total replacement
> for STL futures, but rather a complement to them (they are in fact
> dependent on STL futures because they use them internally) which
> might be useful as a future quality-of-implementation optimisation if
> and only if certain constraints are satisified.

As long as we agree that a non-allocating standard-conformant future is 
an oxymoron...

> 7. future.wait() very rarely blocks in your use scenario i.e. most if
> not nearly all the time the future is ready. If you are blocking, the
> cost of any thread sleep will always dwarf the cost of any future.

`future::wait` should not be a concern, you just spawn a 
`condition_variable` and attach a fake continuation to the future that 
will wake it up. The cv will be stored in the thread stack, which is 
guaranteed to stay around since we will be sleeping. This allows the 
fake continuation to simply be `reference_wrapper`, for which you only 
need `sizeof(void*)` embedded storage. Since the (unique) future cannot 
be used concurrently with `wait`, there can only ever be at most one 
fake continuation. The situation gets trickier for `wait_for/until`, 
where you need to remove the fake continuation on a timeout without racing.

> These circumstances are common enough in low latency applications
> such as ASIO and using them is a big win in ASIO type applications
> over STL futures. These circumstances are not common in general
> purpose C++ code, and probably deliver little benefit except maybe a
> portable continuations implementation on an older STL.

Have you tried a custom allocator in this scenario? Together with a 
decent implementation of `std::future`, which granted does not exist AFAIK.

>> And finally, let's not forget that the Concurrency TS (or actually the
>> futures continuation section of it) complicates matters even more. The
>> addition of `.then` requires implementations to store an arbitrary
>> Callable around until the future to which it was attached becomes ready.
>> Arguably, this Callable has to be stored regardless of whether the
>> future is already ready, but I'm checking the final wording and it
>> appears that you can as-if run the continuation in the calling thread
>> despite not being required (and at least discouraged in an initial
>> phase).
>
> I read the postconditions as meaning:
>
> if(future.is_ready())
>    callable(future);
> else
>    store_in_promise_for_later(callable);
>
> ... which is what I've implemented.
>
> I *do* allocate memory for continuations, one malloc per continuation
> added.

I assume this is just rushed pseudocode, but anyways with my pedant hat on:

- Callable means _INVOKE_, if you are not using _INVOKE_ then you do not 
have a callable. You might have a function object at best.

- You must do the _DECAY_COPY_ dance, even for the immediate execution 
case. Not doing it means different types, different overloads, different 
semantics, so you could end up computing the wrong return type.

- The argument to the continuation shall be an xvalue for `std::future`, 
and a const lvalue for `std::shared_future`.

- You must do "implicit unwrapping", ideally not in a naive way which 
would involve two memory allocations instead of one.

- If you were to execute immediately and were in an "implicit 
unwrapping" case, you'd have to be careful to handle the invalid source 
case, and to catch any exception and store it in the returned future 
instead of letting it leave the scope.

Finally, about executing immediately, an earlier draft of the TS 
specified the semantics in terms of launch policies, which implied 
execution of the continuation happening "as-if" on a new thread. That 
would preclude running the continuation immediately, as it would be 
observable. The current wording says "is called on an unspecified thread 
of execution", which technically allows to call the continuation right 
here right now. That's not necessarily a good idea, and was initially 
explicitly discouraged, since you are basically executing sequentially 
which was clearly designed to execute concurrently. If that were to be 
what you want, you could always do it (as shown in examples in earlier 
drafts):

     if (f.is_ready()) {
        ... f.get() ...
     } else {
         f.then(...);
     }

Regards,
-- 
Agustín K-ballo Bergé.-
http://talesofcpp.fusionfenix.com