> On June 6, 2017, 9:37 p.m., Zhitao Li wrote: > > 3rdparty/libprocess/include/process/metrics/gauge.hpp > > Line 34 (original), 35-39 (patched) > > <https://reviews.apache.org/r/59858/diff/2/?file=1743153#file1743153line35> > > > > I am somehow against this general guideline in the long term, because > > metrics collection should be 1) constant for each metrics, and 2) not > > generating extra overhead. Using a `Deferred` violates both argument. > > > > I hope that most (if not all) gauges can work with an `const > > std::atomic<float>&`, and metrics collection code can simply call a > > `load()` function and return it immediately. Whatever functor callback we > > write right now should be moved to the producing libprocess instead. > > > > With the above said, I am still happy to see this happening.
We'll take out these defer details and instead just document the lifetime semantics for `f`. We can follow up with documentation at the top level of `Gauge` that shows some examples or patterns for safely getting `Process`-based metrics (both with and without dispatching). - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59858/#review177100 ----------------------------------------------------------- On June 6, 2017, 9:16 p.m., James Peach wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59858/ > ----------------------------------------------------------- > > (Updated June 6, 2017, 9:16 p.m.) > > > Review request for mesos, Benjamin Bannier and Benjamin Mahler. > > > Repository: mesos > > > Description > ------- > > A `Deferred` function object is always defined to be a > `std::function`, so when we are constructing a `Gauge` metrics > is it not strictly required for the callback to be a `Deferred` > object. In cases where the value is a constant or can be calculated > by reading constant data, a metrics producer can emit a `Gauge` > without the taking the additional dispatch cost. > > > Diffs > ----- > > 3rdparty/libprocess/include/process/metrics/gauge.hpp > c5bbeaf5d53c60f3636d1778db47cdb6cdda34e8 > 3rdparty/libprocess/src/tests/metrics_tests.cpp > c13520d23ca17144f553fb4588fb8a747ea46e72 > > > Diff: https://reviews.apache.org/r/59858/diff/3/ > > > Testing > ------- > > make check (Fedora 25) > > > Thanks, > > James Peach > >
