> On Jan. 28, 2013, 8:08 p.m., Vinod Kone wrote: > > third_party/libprocess/src/statistics.cpp, line 50 > > <https://reviews.apache.org/r/9091/diff/1/?file=251530#file251530line50> > > > > Why virtual? Are you expecting this to be subclassed? > > Ben Mahler wrote: > virtual so that deletion of a Process pointer pointing to a > StatisticsProcess will call this destructor. I think destructors should > always be virtual when dealing with inheritance: > http://www.learncpp.com/cpp-tutorial/123-virtual-destructors-virtual-assignment-and-overriding-virtualization/
Yes. I was just wondering if you ever anticipate passing StatiscticsProcess* as Process*. I guess virtual is always safe. > On Jan. 28, 2013, 8:08 p.m., Vinod Kone wrote: > > third_party/libprocess/src/statistics.cpp, line 176 > > <https://reviews.apache.org/r/9091/diff/1/?file=251530#file251530line176> > > > > so is it ok to mutate the list and use its iterator at the same time!? > > Ben Mahler wrote: > This was the original code from Statistics, but reading up on the map > erase: > "Iterators, pointers and references referring to elements removed by the > function are invalidated. > All other iterators, pointers and references keep their validity." > > So technically, we would need to advance to the next element to avoid > invalidation. (see: http://stackoverflow.com/a/5673209) > > But I've included a simpler fix here. clever :) - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9091/#review15763 ----------------------------------------------------------- On Jan. 28, 2013, 11:32 p.m., Ben Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/9091/ > ----------------------------------------------------------- > > (Updated Jan. 28, 2013, 11:32 p.m.) > > > Review request for mesos, Benjamin Hindman and Vinod Kone. > > > Description > ------- > > This adds context into the Statistics model. Contexts provide a clean way to > logically group stats (e.g. contexts: "master", "slave", "monitor", > "libprocess", etc). > > > This addresses bug MESOS-324. > https://issues.apache.org/jira/browse/MESOS-324 > > > Diffs > ----- > > third_party/libprocess/include/process/statistics.hpp > 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9 > third_party/libprocess/src/statistics.cpp > 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f > third_party/libprocess/src/statistics_tests.cpp > 0aaab3526618171c7cfbd11d40d614344bcbfd0a > > Diff: https://reviews.apache.org/r/9091/diff/ > > > Testing > ------- > > make check + libprocess make check > > > Thanks, > > Ben Mahler > >
