> On June 20, 2015, 7:35 p.m., Alexander Rukletsov wrote: > > Why do we use different approaches (emplace() and insert()) in c-tors? Is > > it possible to unify them for clarity? If not, mind leaving a comment > > explaining the reasoning?
Great idea, I added a comment as to why we use 'insert' instead of 'emplace' since we use 'emplace' everywhere else. > On June 20, 2015, 7:35 p.m., Alexander Rukletsov wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/hashmap.hpp, line 52 > > <https://reviews.apache.org/r/35694/diff/3/?file=988938#file988938line52> > > > > Let's avoid re-creating iterator: > > > > ``` > > for (auto iterator = map.begin(), auto endIterator = map.end(); > > iterator != endIterator; > > ++iterator) { > > ``` > > Till Toenshoff wrote: > We never really do this kind of optimzation within mesos, or do we? I > briefly checked a couple of stout-files which dont try to avoid re-getting > the end of a list. > > Given that it does not increase readability, I'ld suggest to first check > if this really was a serious gainer (some neat little profiling) and if so, > adapt our styleguide. > > Alexander Rukletsov wrote: > It does not reduce readability either, why not gaining free performance > then? > > And actually legal precedent is set: `slave.cpp:1262`, commit > ccf6c254a1620e512ec66f1e20644d47c12c6832 This isn't a precedent that we've set in the past. The example you showed Alex looks like they were actually trying to account for the fact that the thing we're iterating over is actually modified during the operation, which can cause issues with determining what 'begin' and 'end' are. I'm going to keep this simple for now. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35694/#review88661 ----------------------------------------------------------- On June 24, 2015, 10 p.m., Benjamin Hindman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35694/ > ----------------------------------------------------------- > > (Updated June 24, 2015, 10 p.m.) > > > Review request for mesos and Till Toenshoff. > > > Bugs: MESOS-2832 > https://issues.apache.org/jira/browse/MESOS-2832 > > > Repository: mesos > > > Description > ------- > > See summary. > > > Diffs > ----- > > 3rdparty/libprocess/3rdparty/stout/include/stout/hashmap.hpp > 4f90d3dcd880b95f22ea13c56a61c7f981eea57d > 3rdparty/libprocess/3rdparty/stout/tests/hashmap_tests.cpp > 6a26d93a9a68ab18b7c9b25039a96b663a73a309 > > Diff: https://reviews.apache.org/r/35694/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Benjamin Hindman > >
