Page MenuHomePhabricator

[Chrono][Darwin] On Darwin use CLOCK_UPTIME_RAW instead of CLOCK_MONOTONIC
ClosedPublic

Authored by bruno on Dec 5 2016, 2:36 PM.

Details

Summary

CLOCK_MONOTONIC is only defined on Darwin on libc versions >= 1133 and its behaviour differs from Linux. CLOCK_UPTIME on Darwin actually matches
CLOCK_MONOTONIC on Linux, due to historical coincidence (Linux doesn't match POSIX here but Darwin does). Use CLOCK_UPTIME_RAW on Darwin since the _RAW version gives nanosecond precision and is lower overhead.

Diff Detail

Event Timeline

bruno updated this revision to Diff 80328.Dec 5 2016, 2:36 PM
bruno retitled this revision from to [Chrono][Darwin] On Darwin use CLOCK_UPTIME_RAW instead of CLOCK_MONOTONIC.
bruno updated this object.
bruno added reviewers: mclow.lists, dexonsmith, EricWF.
bruno added a subscriber: cfe-commits.
EricWF edited edge metadata.Dec 7 2016, 9:47 PM

I'm not sure CLOCK_UPTIME_RAW meets the requirements of steady_clock. The manpage for clock_gettime on OS X specifies CLOCK_UPTIME_RAW as:

CLOCK_UPTIME_RAW clock that increments monotonically, in the same manner as CLOCK_MONOTONIC_RAW, but that does not increment while the system is asleep.

And C++17 [time.clock.steady]p1 says:

Objects of class steady_clock represent clocks for which values of time_point never decrease as physical time advances and for which values of time_point advance at a steady rate relative to real time. That is, the clock may not be adjusted.

So if CLOCK_UPTIME_RAW doesn't update while the system is asleep does it advance at a steady rate relative to real time? I'm not convinced it does.

@mclow.lists Do you agree with this interpretation?

mclow.lists edited edge metadata.Dec 9 2016, 9:10 AM

I agree with @EricWF . If CLOCK_UPTIME_RAW doesn't meet the requirements of steady_clock (i.e, monotonically increasing, and advances in real time), then we can't use it.

EricWF added a comment.Dec 9 2016, 9:15 AM

CLOCK_MONOTONIC_RAW however seems to meet the requirements..

howard.hinnant added a subscriber: howard.hinnant.EditedDec 9 2016, 5:38 PM

I would like to offer two thoughts:

  1. Timing with steady_clock is often used to time very short events. It should be respected as the "high_resolution_clock". This means that two consecutive calls to steady_clock::now() should return nanoseconds resolution and should not be equal unless the implementation is actually able to complete a call to steady_clock::now() in less than 1ns.
  1. Here's test of thought 1:
#include <chrono>
#include <time.h>
#include <errno.h>
#include <sys/sysctl.h>

std::chrono::milliseconds
uptime()
{
    using namespace std::chrono;
    timeval ts;
    auto ts_len = sizeof(ts);
    int mib[2] = { CTL_KERN, KERN_BOOTTIME };
    auto constexpr mib_len = sizeof(mib)/sizeof(mib[0]);
    if (sysctl(mib, mib_len, &ts, &ts_len, nullptr, 0) == 0)
    {
        system_clock::time_point boot{seconds{ts.tv_sec} + microseconds{ts.tv_usec}};
        return duration_cast<milliseconds>(system_clock::now() - boot);
    }
    return 0ms;
}

std::chrono::nanoseconds
get_uptime_raw()
{
    using namespace std::chrono;
    struct timespec tp;
    clock_gettime(CLOCK_UPTIME_RAW, &tp);
    return seconds(tp.tv_sec) + nanoseconds(tp.tv_nsec);
}

std::chrono::nanoseconds
get_monotonic()
{
    using namespace std::chrono;
    struct timespec tp;
    clock_gettime(CLOCK_MONOTONIC, &tp);
    return seconds(tp.tv_sec) + nanoseconds(tp.tv_nsec);
}

#include "date.h"
#include <iostream>

template <class Duration>
void
display(Duration time)
{
    using namespace date;
    auto d = floor<days>(time);
    time -= d;
    std::cout << d.count() << " days " << make_time(time) << '\n';
}

int
main()
{
    using namespace std::chrono;
    for (int i = 0; i < 4; ++i)
    {
        std::cout << i << '\n';
        {
            auto t0 = uptime();
            auto t1 = uptime();
            std::cout << "boot time                     : ";  display(t0);
            std::cout << "boot time                     : ";  display(t1);
            std::cout << "delta boot time               : " << nanoseconds{t1 - t0}.count() << "ns\n";
        }
        {
            auto t0 = get_uptime_raw();
            auto t1 = get_uptime_raw();
            std::cout << "CLOCK_UPTIME_RAW              : "; display(t0);
            std::cout << "CLOCK_UPTIME_RAW              : "; display(t1);
            std::cout << "delta CLOCK_UPTIME_RAW time   : " << nanoseconds{t1 - t0}.count() << "ns\n";
        }
        {
            auto t0 = get_monotonic();
            auto t1 = get_monotonic();
            std::cout << "CLOCK_MONOTONIC               : "; display(t0);
            std::cout << "CLOCK_MONOTONIC               : "; display(t1);
            std::cout << "delta CLOCK_MONOTONIC time    : " << nanoseconds{t1 - t0}.count() << "ns\n";
        }
        {
            auto t0 = std::chrono::steady_clock::now().time_since_epoch();
            auto t1 = std::chrono::steady_clock::now().time_since_epoch();
            std::cout << "mach_absolute_time            : "; display(t0);
            std::cout << "mach_absolute_time            : "; display(t1);
            std::cout << "delta mach_absolute_time time : " << nanoseconds{t1 - t0}.count() << "ns\n";
        }
        std::cout << '\n';
    }
}

Sorry, it requires "date.h" from https://github.com/HowardHinnant/date . It is header-only and portable. It's just used for formatting purposes if it really stresses you out.

For me this outputs (at -O3):

Jade:~/Development/cljunk> a.out
0
boot time                     : 11 days 22:30:42.827
boot time                     : 11 days 22:30:42.827
delta boot time               : 0ns
CLOCK_UPTIME_RAW              : 11 days 22:22:28.960672112
CLOCK_UPTIME_RAW              : 11 days 22:22:28.960672266
delta CLOCK_UPTIME_RAW time   : 154ns
CLOCK_MONOTONIC               : 11 days 22:30:42.827318000
CLOCK_MONOTONIC               : 11 days 22:30:42.827318000
delta CLOCK_MONOTONIC time    : 0ns
mach_absolute_time            : 11 days 22:22:28.960714394
mach_absolute_time            : 11 days 22:22:28.960714504
delta mach_absolute_time time : 110ns

1
boot time                     : 11 days 22:30:42.827
boot time                     : 11 days 22:30:42.827
delta boot time               : 0ns
CLOCK_UPTIME_RAW              : 11 days 22:22:28.960761867
CLOCK_UPTIME_RAW              : 11 days 22:22:28.960761932
delta CLOCK_UPTIME_RAW time   : 65ns
CLOCK_MONOTONIC               : 11 days 22:30:42.827402000
CLOCK_MONOTONIC               : 11 days 22:30:42.827402000
delta CLOCK_MONOTONIC time    : 0ns
mach_absolute_time            : 11 days 22:22:28.960793667
mach_absolute_time            : 11 days 22:22:28.960793747
delta mach_absolute_time time : 80ns

2
boot time                     : 11 days 22:30:42.827
boot time                     : 11 days 22:30:42.827
delta boot time               : 0ns
CLOCK_UPTIME_RAW              : 11 days 22:22:28.960835164
CLOCK_UPTIME_RAW              : 11 days 22:22:28.960835227
delta CLOCK_UPTIME_RAW time   : 63ns
CLOCK_MONOTONIC               : 11 days 22:30:42.827476000
CLOCK_MONOTONIC               : 11 days 22:30:42.827476000
delta CLOCK_MONOTONIC time    : 0ns
mach_absolute_time            : 11 days 22:22:28.960867852
mach_absolute_time            : 11 days 22:22:28.960867944
delta mach_absolute_time time : 92ns

3
boot time                     : 11 days 22:30:42.827
boot time                     : 11 days 22:30:42.827
delta boot time               : 0ns
CLOCK_UPTIME_RAW              : 11 days 22:22:28.960911646
CLOCK_UPTIME_RAW              : 11 days 22:22:28.960911737
delta CLOCK_UPTIME_RAW time   : 91ns
CLOCK_MONOTONIC               : 11 days 22:30:42.827553000
CLOCK_MONOTONIC               : 11 days 22:30:42.827553000
delta CLOCK_MONOTONIC time    : 0ns
mach_absolute_time            : 11 days 22:22:28.960945129
mach_absolute_time            : 11 days 22:22:28.960945196
delta mach_absolute_time time : 67ns
  1. Third thought (off by one errors are rampant! ;-) ):

CLOCK_MONOTONIC gives a more accurate report of system uptime, and thus more accurately respects the intent of steady_clock's definition. However the difference is slight. Only CLOCK_UPTIME_RAW and mach_absolute_time are able to time functions in the nanosecond range, which is the most important use case for steady_clock.

Imho, only CLOCK_UPTIME_RAW or mach_absolute_time are acceptable implementations of steady_clock on macOS. I feel strongly enough about this that I would like to see a static_assert that the CLOCK_MONOTONIC is never accidentally chosen by the preprocessor when targeting macOS, or iOS. I can't directly speak to other platforms. But I would like to see tests such as this applied to other platforms. steady_clock should be able to measure short events without returning 0ns. The Windows experience with <chrono> has taught us well that this would be a dissatisfying experience to customers.

One more comment: steady_clock::now() is not allowed to throw an exception because it shall satisfy the requirements of TrivialClock ([time.clock]/p1). And [time.clock.req]/p4/b4 says that a TrivialClock::now() does not throw exceptions.

Howard Thank you for your excellent analysis. Although I still don't think that CLOCK_UPTIME_RAW meets the requirements of steady_clock but I would rather relax the standard here than provide a poor implementation.

I feel strongly enough about this that I would like to see a static_assert that the CLOCK_MONOTONIC is never accidentally chosen by the preprocessor when targeting macOS, or iOS.

I agree. This patch LGTM after ensuring that CLOCK_MONOTONIC is never selected on Apple platforms.

Thanks Eric.

Fwiw CLOCK_MONOTONIC won't meet the requirements of the standard either. The standard appears to require steady_clock to be a perfect clock and there is no such thing. The wording used to only require monotonic, but the committee got a little too enthusiastic. :-)

EricWF requested changes to this revision.Dec 23 2016, 6:00 PM
EricWF edited edge metadata.

Marking as changes requested.

Please make sure we fall back to the old implementation on Apple platforms when CLOCK_UPTIME_RAW isn't available.

This revision now requires changes to proceed.Dec 23 2016, 6:00 PM
bruno updated this revision to Diff 83158.Jan 4 2017, 4:30 PM
bruno edited edge metadata.

Thanks for the reviews. @howard.hinnant thanks for the great explanation & examples.

Attached a new version of the patch, addressing all suggestions. The logic became a bit simpler after Saleem's r290804, which already moved the CLOCK_MONOTONIC to be the last condition.

On top of that, I also conditionalized the presence of clock_gettime, which isn't available before 10.12 on macosx and specific versions on other apple platforms. Check that by looking at the deployment target.

howard.hinnant added inline comments.Jan 4 2017, 5:08 PM
src/chrono.cpp
218

Nice, thanks!

bruno added a comment.Jan 5 2017, 3:19 PM

@EricWF ok to commit?

EricWF accepted this revision.Jan 6 2017, 12:30 PM
EricWF edited edge metadata.

@EricWF ok to commit?

Yes sorry This LGTM. (I thought I had approved this yesterday, but I forgot to hit enter).

src/chrono.cpp
218

This code is currently dead due to the structure of the #ifdefs. However I think it's great documentation and great future proofing!

This revision is now accepted and ready to land.Jan 6 2017, 12:30 PM
bruno closed this revision.Apr 13 2017, 12:51 PM

r291466 & r291517