Re: [boost] Executor associated to the future continuations (was [thread] boost::future::then)

Subject: Re: [boost] Executor associated to the future continuations (was [thread] boost::future::then)
From: Vicente J. Botet Escriba (vicente.botet_at_[hidden])
Date: 2015-10-29 06:01:57

Le 29/10/15 10:14, Vladimir Prus a écrit :
> On 17-Oct-15 7:41 PM, Vicente J. Botet Escriba wrote:
>> Le 16/10/15 01:19, Vicente J. Botet Escriba a écrit :
>>> Le 13/10/15 19:33, Vicente J. Botet Escriba a écrit :
>>>> Le 13/10/15 10:34, Vladimir Prus a écrit :
>>>>> On 11-Oct-15 9:30 PM, Vicente J. Botet Escriba wrote:
>>>>>> Le 10/10/15 16:58, Vicente J. Botet Escriba a écrit :
>>>>>>> Le 10/10/15 15:26, Vicente J. Botet Escriba a écrit :
>>>>>>>> Le 10/10/15 07:57, Vladimir Prus a écrit :
>>>>>>>>>
>>>>>>>>> So, the function must be executed in different thread from all 
>>>>>>>>> the continuations,
>>>>>>>>> and it would seem I'd need to something set executor on 
>>>>>>>>> promise for that to work?
>>>>>>>>>
>>>>>>>>
>>>>>>>> I will see how adding an Executor parameter to promise, 
>>>>>>>> packaged_task constructors and
>>>>>>>> make_ready_future/make_exceptional_future could be implemented 
>>>>>>>> if this will solve your use case.
>>>>>>>>
>>>>>>> I've create https://svn.boost.org/trac/boost/ticket/11717 to 
>>>>>>> track this feature request
>>>>>>>
>>>>>>>
>>>>>> This commit contains a fix for this issue as well as the addition 
>>>>>> of the VERY-EXPERIMENTAL promise::set_executor and
>>>>>> packaged_task::set_executor. These should be replaced by 
>>>>>> constructors having an executor as parameter.
>>>>>>
>>>>>> https://github.com/boostorg/thread/commit/b8db8fef8b28414d16c66761badc1c6fcadfc38f 
>>>>>>
>>>>>
>>>>> Vicente,
>>>>>
>>>>> thanks for the addition. I'll take a look at implementing 
>>>>> Qt-friendly future on top of this, and report
>>>>> how it goes.
>>>>>
>>>>> Thanks,
>>>>> Volodya
>>>>>
>>>> You are welcome.
>>>>
>>>> Note that I've not implemented the constructors as it need to 
>>>> change the hierarchy of classes, but
>>>> promise::set_executor and packaged_task::set_executor which is more 
>>>> expensive as I need to type-erase the executor.
>>>>
>>>> Any feedback would be much appreciated.
>>>>
>>>>
>>>
>>> After looking at the Concurrency TS, I'm not sure if it is not a 
>>> good idea to store the executor so that
>>>
>>>     f.then(cont)
>>>
>>> launch cont on the executor associated to f.
>>>
>>>
>>> In the Concurrent TS, ::then() can execute the continuation on any 
>>> thread.
>>>
>>>
>>> "
>>>
>>>  * When the object's shared state is ready, the continuation
>>>    |/INVOKE/(/DECAY_COPY/(std::forward<F>(func)), std::move(*this))| is
>>>    called on an unspecified thread of execution with the call to
>>>    |/DECAY_COPY/()| being evaluated in the thread that called |then|.
>>>
>>> "
>>>
>>> What do you think?
>>>
>> Hi again,
>>
>> I believe that it would be better if the use states explicitly that 
>> she wants to inherit the policy unsing a specific
>> policy
>>
>>      f.then(launch::inherit, cont);
>>
>> I have not implemented this already, but I have already the 
>> possibility to use the undocumented executor policy
>>
>>      f.then(launch::executor, cont);
>>
>> This would launch cont using the inherited executor.
>
> I suppose I can implement QtFuture that:
>
> - Holds boost::future and a pointer to my executor
> - Has .then method (or better yet, overrides the >> operator)
> - Calls boost::future::then with whatever parameters
>
> From the code above I'm not sure what
>
>     f.then(launch::inherit, cont);
>
> would do? Launch using the executor that was specified in promise?
Yes.
> It would seem, really, that if an executor was specified in promise, 
> then using that executor should be default?
Well, this is now the default behavior as it corresponds to what was 
documented.

However I believe that the default behavior shouldn't be to inherit.
I want to be able to execute the continuation synchronously by the 
thread that made ready the promise.
I have added an experimental flag BOOST_THREAD_CONTINUATION_SYNC that 
switch the default behavior.
Alternatively I could add a launch::sync policy  (and I will do it).
So the single thing that will remain is to decide what is the default 
behavior.

Volodya, if you use this in production code, please, be ready to have 
changes in the next release.
There will be no promise::set_executor. You will need to give the 
executor at the creation of the promise, as it was planned from the 
beginning (See that the set_executor has not been documented and that 
the ticket is still open. It is just that this need to refactor the core 
more deeply. The advantage is that you will not need anymore to allocate 
the future (no type erasure needed).

Vicente