> On June 7, 2017, 12:18 a.m., Benjamin Bannier 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 believe the gist of this change is to recognize that a `Gauge` has no > > dependency on the production of its value to be deferred; instead this > > decision should be should be left to the creator of the `Gauge`. > > > > For most existing `Gauge`s in the Mesos code base we not only depend on > > some process to carry out calculcation of values, but also introduce a > > life-time dependency made safe by using `Deferred`. This is inherent to the > > functions passed in here (they could e.g., be implicitly converted from a > > `_Deferred`. > > > > Given that, I am not sure we need to explain `Deferred` here. I would > > instead update the existing comment, e.g., `s/deferred object/producer > > function/` or similar. > > James Peach wrote: > I wanted to capture something about the common usage in the codebase and > give some guidance without second-guessing how this should be used. I would > like to make the comment a bit more expansive so we can explain how to use > `Gauge` safely, but I'm open to suggestions :)
I cannot see how the caveats of function objects used with a `Gauge` are any different from uses elsewhere (e.g., in continuations). If anything the previous version should have explained why it choose that particular, restricted interface, but the form here seems natural to me and requiring no references to possible data or lifetime dependencies. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59858/#review177103 ----------------------------------------------------------- On June 6, 2017, 11: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, 11: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 > >
