> On Feb. 17, 2014, 6:32 a.m., Jie Yu wrote: > > src/log/replica.cpp, line 721 > > <https://reviews.apache.org/r/18175/diff/1/?file=486922#file486922line721> > > > > I am a little worried here because the 'learned' set could be quite > > large. Copying the entire set maybe quite expensive. > > > > I don't think gcc can get rid of the copying here as > > state.get().learned is an lvalue. > > > > What about leaving a NOTE here?
I had updated this one because we were already performing a copy of the learned set as a result of calling 'state.get()' with the old Try semantics. That said, the code with const& is correct and you're right that the copy I've introduced cannot be removed by gcc, so I'll revert. - Ben ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18175/#review34619 ----------------------------------------------------------- On Feb. 16, 2014, 11:38 p.m., Ben Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18175/ > ----------------------------------------------------------- > > (Updated Feb. 16, 2014, 11:38 p.m.) > > > Review request for mesos, Benjamin Hindman, Jie Yu, Niklas Nielsen, and Vinod > Kone. > > > Bugs: MESOS-1008 > https://issues.apache.org/jira/browse/MESOS-1008 > > > Repository: mesos-git > > > Description > ------- > > See: https://reviews.apache.org/r/18173/ > > I also killed instances of our old pattern: > > const Try<T>& t = foo(); > > // Replaced with: > Try<T> t = foo(); > > I did this for two reasons: > 1. gcc is often already eliding the copy. > 2. This will be obviated when we add move constructors to Try, since foo() > is returning an rvalue. > > > Diffs > ----- > > src/log/replica.cpp 746d6c35c9255775ab6e70b0daf1dcecf63c16a0 > src/master/contender.cpp e3b0737195aba42c805a05c96b054cec1471b502 > src/messages/messages.hpp 98038c017a7d02c7c4f04f91c89ca56f566f707b > src/slave/containerizer/isolators/cgroups/cpushare.cpp > 989d3848343d4255c25796aa5be81dbb93a29b6e > src/slave/containerizer/isolators/cgroups/mem.cpp > a01e114c168b820e6c54af9257716b753940d510 > src/slave/slave.cpp 8ad955a71957421ae51e1becc4b06a4b6b091eb2 > src/slave/state.cpp 9af6c5b0582e972523028d226703070293b92f8b > src/tests/cgroups_tests.cpp f0dead7c09fe3cb4355c5b3b4603a2cb8c2cd3d5 > src/tests/mesos.cpp 98333a7d3a2c7c8004aacc43dda3add31d5b5487 > src/tests/values_tests.cpp c72dbc2ec05e12541c03196577dff3561bff08ed > src/tests/zookeeper_url_tests.cpp b143c539530e3319f603d9276ebf84dfecfcf6f2 > > Diff: https://reviews.apache.org/r/18175/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Ben Mahler > >
