----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60830/#review180619 -----------------------------------------------------------
3rdparty/libprocess/src/process.cpp Line 3806 (original), 3806 (patched) <https://reviews.apache.org/r/60830/#comment255819> A couple of questions: (1) Is MessageHandler cheap to copy? I assume in most cases, yes. But if the message handler came from a lambda with copy captures, then it seems like copying it here is expensive? (2) More importantly, if the handler is a lambda with copy captures, then I think copying it each time we execute it is actually incorrect: ``` #include <iostream> #include <functional> using namespace std; int main() { int x = 1; function<void()> f1 = [=]() mutable { cout << x << endl; x *= 2; }; auto f2 = f1; cout << "calling f1:" << endl; f1(); f1(); f1(); cout << "calling copy of f1:" << endl; f2(); f2(); return 0; } ``` This prints: ``` calling f1: 1 2 4 calling copy of f1: 1 2 ``` So by copying the function each time we're messing with the caller's expectation of the capture semantics. Ideally, hashmap::get returned an `Option<Value&>` rather than `Option<Value>` but it's probably a little tedious for you to make that change right now. - Benjamin Mahler On July 15, 2017, 12:13 a.m., Benjamin Hindman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60830/ > ----------------------------------------------------------- > > (Updated July 15, 2017, 12:13 a.m.) > > > Review request for mesos and Benjamin Mahler. > > > Bugs: MESOS-7798 > https://issues.apache.org/jira/browse/MESOS-7798 > > > Repository: mesos > > > Description > ------- > > Replaced std::map with hashmap for ProcessBase::handlers. > > > Diffs > ----- > > 3rdparty/libprocess/include/process/process.hpp > 944fcc6449edfd022db4048f70a13aff4a1ff345 > 3rdparty/libprocess/src/process.cpp > 7ce6d2b13baa68906e091a95c0dd58ee1ca2bc7d > > > Diff: https://reviews.apache.org/r/60830/diff/1/ > > > Testing > ------- > > make check > > > Thanks, > > Benjamin Hindman > >
