----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60003/#review181717 -----------------------------------------------------------
Looks good overall! 3rdparty/libprocess/include/process/defer.hpp Lines 62 (patched) <https://reviews.apache.org/r/60003/#comment257367> Let's just call this `FORWARD`. Ideally we'd take the `A` and `a` as parameters into the macro, but I don't think it's worth it since we'll be removing all of these macros anyway. Meanwhile, could you just leave a comment here mentioning the implicit use of `A` and `a` please? 3rdparty/libprocess/include/process/defer.hpp Lines 83-85 (original), 93-97 (patched) <https://reviews.apache.org/r/60003/#comment257368> Could we just fix the formatting here? Specifically, something like: ```cpp return defer(process.self(), method, ENUM(N, FORWARD, _)); ``` Here and below. 3rdparty/libprocess/include/process/deferred.hpp Line 83 (original), 85 (patched) <https://reviews.apache.org/r/60003/#comment257371> We should be able to save an extra copy/move here by taking it by doing `F&& f_ = std::forward<F>(f);` 3rdparty/libprocess/include/process/dispatch.hpp Lines 193-197 (original), 201-225 (patched) <https://reviews.apache.org/r/60003/#comment257370> Could you explain how this is any different? It looks to me like the arguemnts are captured by-value in `Dispatcher`, which is what happens with the lambda anyway. Here and below. - Michael Park On July 17, 2017, 4:42 a.m., Dmitry Zhuk wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60003/ > ----------------------------------------------------------- > > (Updated July 17, 2017, 4:42 a.m.) > > > Review request for mesos, Benjamin Hindman, haosdent huang, James Peach, and > Michael Park. > > > Bugs: MESOS-7713 > https://issues.apache.org/jira/browse/MESOS-7713 > > > Repository: mesos > > > Description > ------- > > This reduces number of copies made for each parameter in a code like this: > future.then(defer(pid, &SomeProcess::someMethod, param1, param2)); > > For the objects that do not support move semantics (e.g. protobuf messages), > number of copies is reduced from 8-10 to 6. If move semantics is supported, > then number of copies is reduced from 6-7 to 1 if parameter is passed with > std::move, or 2 otherwise. > > > Diffs > ----- > > 3rdparty/libprocess/include/process/defer.hpp > 7f3369e723cb244e97930621cbba89cf7873567d > 3rdparty/libprocess/include/process/deferred.hpp > e446621be11ac51f5f91c417cc8975e363c2f715 > 3rdparty/libprocess/include/process/dispatch.hpp > 3a0793888dc0df5e3ec31b06f47cd920c71e0db9 > 3rdparty/libprocess/include/process/future.hpp > cce950509f58022e79bb51a6e72ea1a005b9cb50 > 3rdparty/libprocess/include/process/http.hpp > f637999174d92a98208b5fc49a65f9929efb11a0 > > > Diff: https://reviews.apache.org/r/60003/diff/2/ > > > Testing > ------- > > make check > > Number of copies was checked by using defer to subscribe process for Future > callbacks, and passing parameters that count number of copies made. > > > Thanks, > > Dmitry Zhuk > >
