> On Sept. 23, 2015, 12:58 a.m., Michael Park wrote: > > src/common/values.cpp, line 301 > > <https://reviews.apache.org/r/38158/diff/13/?file=1081516#file1081516line301> > > > > Can we just take a `Value::Ranges` here rather than > > `initializer_list<Value::Ranges>`? > > > > It looks like the only place we actually use this is in `operator+`, > > for which we can just follow the pattern for `operator-`. > > > > ```cpp > > coalesce(&result, left); > > return result += result; > > ``` > > > > I think it would also clean up this code significantly, as we wouldn't > > need the `rangesSum` loop, The `fill` function wouldn't have to be factored > > out, wouldn't need the `offset` math, etc. > > Joerg Schad wrote: > Would like to avoid coalescing twice, but couldnt we do something along > the lines > ```c++ > void coalesce(Value::Ranges* result, Value::Ranges ranges) { > size_t rangesSum = result->range_size() + ranges->ranges_size(); > ``` > > In that case I would probably leave fill factored out, but the rest > (rangesSum, offset) would be simplified. > > Joerg Schad wrote: > Na doesn't really help, will add a more detailed discussion of tradeoffs.
As discussed. - Joerg ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38158/#review100091 ----------------------------------------------------------- On Sept. 22, 2015, 8:55 p.m., Joerg Schad wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38158/ > ----------------------------------------------------------- > > (Updated Sept. 22, 2015, 8:55 p.m.) > > > Review request for mesos, Bernd Mathiske, Joris Van Remoortere, and Till > Toenshoff. > > > Bugs: MESOS-3051 > https://issues.apache.org/jira/browse/MESOS-3051 > > > Repository: mesos > > > Description > ------- > > The goal of this refactoring was to reuse the Ranges objects as much as > possible, as prior there was substantial time spend in allocation/destruction > (MESOS-3051). > > > Diffs > ----- > > src/common/values.cpp 750264e603b4cde2011f07f4434a4b34fe3e512f > src/tests/resources_tests.cpp 0318885336409f7cc9dbd4a3daa9b52db197bbd1 > src/tests/values_tests.cpp fc35d97894a2de6207b9337180e2160e6f2cb1f5 > > Diff: https://reviews.apache.org/r/38158/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Joerg Schad > >
