----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10718/#review19730 -----------------------------------------------------------
partial review third_party/libprocess/include/process/statistics.hpp <https://reviews.apache.org/r/10718/#comment40693> "is be caorsened over time" doesn't read correct third_party/libprocess/src/statistics.cpp <https://reviews.apache.org/r/10718/#comment40698> s/const// ? third_party/libprocess/src/statistics.cpp <https://reviews.apache.org/r/10718/#comment40699> s/Option<size_t>::none()/None()/ third_party/libprocess/src/statistics.cpp <https://reviews.apache.org/r/10718/#comment40701> Since 'capacity' is an external input, this should be an EXIT (w/ a helpful message) rather than CHECK? third_party/libprocess/src/statistics.cpp <https://reviews.apache.org/r/10718/#comment40703> can you do return None()? third_party/libprocess/src/statistics.cpp <https://reviews.apache.org/r/10718/#comment40706> can't you do (values.rbegin()->first, values.rbegin()->second) instead pulling out the iterator? third_party/libprocess/src/statistics.cpp <https://reviews.apache.org/r/10718/#comment40708> Just use None(), here and everywhere else. third_party/libprocess/src/statistics.cpp <https://reviews.apache.org/r/10718/#comment40717> s/Seconds(0.0)/values.begin()->first/ ? third_party/libprocess/src/statistics.cpp <https://reviews.apache.org/r/10718/#comment40718> s/Seconds(DBL_MAX)/values.rbegin()->first/? third_party/libprocess/src/statistics.cpp <https://reviews.apache.org/r/10718/#comment40723> how come truncate() is outside but 'values[time] = value' inside if and else? can you simplify this? third_party/libprocess/src/statistics.cpp <https://reviews.apache.org/r/10718/#comment40722> s/. We/, we/ third_party/libprocess/src/statistics.cpp <https://reviews.apache.org/r/10718/#comment40724> s/half/halve/ third_party/libprocess/src/statistics.cpp <https://reviews.apache.org/r/10718/#comment40725> should be an EXIT instead? third_party/libprocess/src/statistics.cpp <https://reviews.apache.org/r/10718/#comment40726> None third_party/libprocess/src/statistics.cpp <https://reviews.apache.org/r/10718/#comment40728> return result.isSome() ? result.get().second : None() ? third_party/libprocess/src/statistics.cpp <https://reviews.apache.org/r/10718/#comment40751> add logging info, context and name to the CHECK third_party/libprocess/src/statistics.cpp <https://reviews.apache.org/r/10718/#comment40752> ditto - Vinod Kone On April 23, 2013, 12:39 a.m., Ben Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/10718/ > ----------------------------------------------------------- > > (Updated April 23, 2013, 12:39 a.m.) > > > Review request for mesos, Benjamin Hindman, Vinod Kone, and Jiang Yan Xu. > > > Description > ------- > > See TimeSeries::truncate() for the "sparsification" technique. > > Sorry for the size of this review, in retrospect I should have split the > addition of TimeSeries and sparsifcation algorithm into separate reviews, but > I had this change lying around for quite a long time before I got good at > splitting apart my changes. > > > Diffs > ----- > > third_party/libprocess/include/process/statistics.hpp > 3f56c078d8609041c80c312aa9a42ed24ca42e60 > third_party/libprocess/src/statistics.cpp > e1fb319eff63e4b7a4c18dba43e4818018b0dd50 > third_party/libprocess/src/tests/statistics_tests.cpp > bb7a94757eddc522f1ab444ee12c9f237e2042a5 > > Diff: https://reviews.apache.org/r/10718/diff/ > > > Testing > ------- > > Added tests for window expiry and sparsifcation. > > > Thanks, > > Ben Mahler > >
