> On Oct. 6, 2014, 10:02 p.m., Ben Mahler wrote: > > 3rdparty/libprocess/src/reap.cpp, lines 124-127 > > <https://reviews.apache.org/r/26229/diff/1/?file=710088#file710088line124> > > > > Why do you need a variable for this? Can't this just be a 'return' > > statement? > > > > If there's a reason to keep this statement separate from the return, > > please avoid 'auto', we'd like to start using it very conservatively. > > Alexander Rukletsov wrote: > Naming the result of the expression serves two purposes here: clarity > about what we calculate and return and facilitating NRWO. Since auto is > replaced, I mark this issue as fixed, please reopen if you think further > discussion about `return` is needed. > > Michael Park wrote: > My thoughts on the two purposes: > > 1. I actually think a comment would do serve the purpose better in terms > of being clear on what we're calculating. `adjustedInterval` doesn't actually > give me that much more information. > 2. Facilitating NRVO I don't think is a good reason. It's much more > likely for URVO to kick in than NRVO. > > Alexander Rukletsov wrote: > 1. We already have a top-level comment describing the approach and one > more right before interpolation code. That's why I think third comment is > redundant and naming a variable suffices. > 2. After the closer look at the code I agree with you: NRVO won't kick in > here. I'll amend it to facilitate URVO.
1. Sorry! I missed the above comments. 2. Well, in this case it will. But it also doesn't matter since the only member in `Duration` is a single `int64_t`. All I was saying is that in general URVO is more likely to kick in than NRVO. So if there's an option to choose, we should prefer URVO (unless of course, naming a variable helps readability). - Michael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26229/#review55582 ----------------------------------------------------------- On Oct. 8, 2014, 9:30 a.m., Alexander Rukletsov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26229/ > ----------------------------------------------------------- > > (Updated Oct. 8, 2014, 9:30 a.m.) > > > Review request for mesos, Benjamin Hindman, Ben Mahler, Ian Downes, Jie Yu, > and Till Toenshoff. > > > Repository: mesos-git > > > Description > ------- > > Lower and upper bounds for the poll interval are refactored as static > functions visible to outer world. > > > Diffs > ----- > > 3rdparty/libprocess/include/process/reap.hpp 9de5336 > 3rdparty/libprocess/src/reap.cpp ac14a86 > > Diff: https://reviews.apache.org/r/26229/diff/ > > > Testing > ------- > > make check (Mac OS 10.9.4, Ubuntu 14.04) > > > Thanks, > > Alexander Rukletsov > >
