> On Sept. 13, 2017, 5:10 a.m., Benjamin Hindman wrote: > > Thanks for taking this on Chun! A few high level comments to start. > > > > (1) I don't think we need to implement a version of `execute()` that takes > > arguments that we'll apply to the function. With lambda captures we can > > easily and cleanly accomplish that and any of the corner cases will be > > cleanly solved for with C++14 which we should move too sooner rather than > > later. If folks really want that functionality I'd rather just see them use > > `std::bind()` and do `executor.execute(std::bind(f, arg1, arg2))`. It's not > > that many more characters and it greately simplifies your implementation! > > > > (2) Given the simplification of (1) I feel like you probably don't need a > > separate `Executor::Process` class, and instead can just use `dispatch()` > > on the existing `process`. You should be able to simplify your SFINAE by > > leveraging the return type `dispatch()` since it already takes care of the > > `void` issue for you.
Thanks for your comments! (1) makes a lot sense to me and I also don't like the current approach of using macros. For (2), I don't think the way `dispatch()` taking care of `void` is what we want, because `dispatch(pid, f)` returns `void` if `f` returns `void` and thus we cannot wait on it. That's the intention of the `Executor::Process::execute` method: turning `void` to `Nothing`. Am I missing something? - Chun-Hung ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62252/#review185261 ----------------------------------------------------------- On Sept. 12, 2017, 8:12 p.m., Chun-Hung Hsiao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62252/ > ----------------------------------------------------------- > > (Updated Sept. 12, 2017, 8:12 p.m.) > > > Review request for mesos, Benjamin Hindman and Benjamin Mahler. > > > Bugs: MESOS-7970 > https://issues.apache.org/jira/browse/MESOS-7970 > > > Repository: mesos > > > Description > ------- > > This patch adds a convenient interface to `process::Executor` to > asynchronously execute arbitrary functions. > > > Diffs > ----- > > 3rdparty/libprocess/include/process/executor.hpp > cd2f2f03cba8a435f142b31def9f89a6cd61af73 > 3rdparty/libprocess/src/tests/process_tests.cpp > 82efb2f8449e4cb13620cae1a49321fc42e2db60 > > > Diff: https://reviews.apache.org/r/62252/diff/2/ > > > Testing > ------- > > make check > > > Thanks, > > Chun-Hung Hsiao > >
