Hey Adam,
I updated my branch based on your recommendations. Thanks.
I'm continuing my edit.
----- Mail original -----
De: "Adam C. Emerson" <aemerson@xxxxxxxxxx>
À: "Erwan Velu" <evelu@xxxxxxxxxx>
Cc: "Sage Weil" <sweil@xxxxxxxxxx>, "The Sacred Order of the Squid Cybernetic" <ceph-devel@xxxxxxxxxxxxxxx>
Envoyé: Vendredi 22 Janvier 2016 18:35:22
Objet: Re: About ceph_clock_now()
On 22/01/2016, Erwan Velu wrote:
> Hey,
>
> I've been able to continue this work and updated by branch accordingly.
> I understand the benefit of using the ceph_time work but I feel that it makes the change pretty verbose for a not a so big change (CLOCK_REALTIME vs CLOCK_MONO).
>
> This imply a change of all utime_t and makes the computations a little bit more complex to read.
There are a few changes one can use to make it less verbose. If you're not in a
header file (or inside a function), you might like to do:
using ceph::coarse_mono_clock;
(Don't do that in headers outside of functions, though.)
Then you can just do things like
auto now = mono_clock_coarse::now();
Which should be a bit less verbose.
Or even:
using cm_clock = ceph::coarse_mono_clock;
auto now = cm_clock::now();
> Does it worth the time spent on it ? If yes, I don't have any issue continuing this way.
> I'm pretty new to the project and would like to make the best PR as possible.
> So your insights on the under-work patch would be very helpful.
I think so. We definitely wnat to use monotonic clocks where appropriate. And we
can get a performance benefit from using coarse clocks where appropriate, too.
And we want to switch to the ceph_time stuff anyway just because it rules out a
whole class of bugs and integrates with standard and third-party libraries.
--
Senior Software Engineer Red Hat Storage, Ann Arbor, MI, US
IRC: Aemerson@{RedHat, OFTC, Freenode}
0x80F7544B90EDBFB9 E707 86BA 0C1B 62CC 152C 7C12 80F7 544B 90ED BFB9
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
