Sounds great, thanks!

Sent from my iPhone

> On Nov 19, 2014, at 3:12 PM, Benjamin Hindman <[email protected]> wrote:
> 
> 
> 
>>> On Nov. 19, 2014, 7:58 p.m., Ben Mahler wrote:
>>> 3rdparty/libprocess/include/process/future.hpp, lines 568-572
>>> <https://reviews.apache.org/r/28195/diff/1/?file=768159#file768159line568>
>>> 
>>>    Should these be CHECKing the appropriate state invariants? Or, at the 
>>> very least we should have a comment here to describe that these should only 
>>> be called when the future is transitioned out of pending (or discard is 
>>> true for the one case), otherwise since these do not acquire the locks!
>>> 
>>>    Seems a little unfortunate to make the reasoning around the 
>>> synchronization invariants less local.
>> 
>> Joris Van Remoortere wrote:
>>    Hey Ben. I've added asserts to check for the corresponding states within 
>> the callbacks, and also adjust the comment above the declarations of the 
>> functions with the expectation.
> 
> That's a really good point Ben. Thanks for bringing this up. How about a 
> static helper:
> 
> template <typename C, typename Arguments...>
> void run(const std::vector<C>& callbacks, Arguments... arguments) const
> {
>  for (size_t i = 0; i < callbacks.size(); i++) {
>     callbacks[i](std::forward<Arguments>(arguments)...);   
>  }
> }
> 
> And then at the onReady callsite:
> 
> run(data->onReadyCallbacks, *data->t);
> data->onReadyCallbacks.clear();
> 
> And then in the future when we have lambdas we can replace 'run' and just do:
> 
> map(data->onReadyCallbacks, [=] (const ReadyCallback& callback) { 
> callback(*data->t); });
> data->onReadyCallbacks.clear();
> 
> 
> - Benjamin
> 
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28195/#review62213
> -----------------------------------------------------------
> 
> 
>> On Nov. 19, 2014, 10:16 p.m., Joris Van Remoortere wrote:
>> 
>> -----------------------------------------------------------
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/28195/
>> -----------------------------------------------------------
>> 
>> (Updated Nov. 19, 2014, 10:16 p.m.)
>> 
>> 
>> Review request for mesos, Benjamin Hindman, Dominic Hamon, Niklas Nielsen, 
>> and Timothy Chen.
>> 
>> 
>> Bugs: MESOS-2126
>>    https://issues.apache.org/jira/browse/MESOS-2126
>> 
>> 
>> Repository: mesos-git
>> 
>> 
>> Description
>> -------
>> 
>> Factor out callback invocation in Future to make the logic easier to read. 
>> It also de-duplicates some code.
>> 
>> 
>> Diffs
>> -----
>> 
>>  3rdparty/libprocess/include/process/future.hpp 
>> 2e4f9efe53e2e9966f23bd516e61fd9d83ed6b33 
>> 
>> Diff: https://reviews.apache.org/r/28195/diff/
>> 
>> 
>> Testing
>> -------
>> 
>> make check
>> 
>> 
>> Thanks,
>> 
>> Joris Van Remoortere
> 

Reply via email to