Work around broken clock_getres()

[fugalh]

On some linux kernels, clock_getres() returns a bogus resolution (e.g. 999848 nanoseconds, when it should be 1ns). Work around it.

[fugalh]

I can rebase into a new pull request with a single commit if that's preferable.

[fugalh]

On further thought, maybe this should unconditionally be nanoseconds when using clock_gettime(). This avoids unnecessary confusion and rounding errors during conversions. It doesn't seem like ticksPerSec is meant to be a definitive answer on clock resolution, just the time base for calculating conversions.

[jmdavis]

I'm not at all comfortable with setting ticksPerSec to a value that doesn't match what we're getting back from clock_gettres and then still using clock_gettime. That seems completely wrong to me. It does make some sense to use gettimeofday if clock_gettime has poor resolution on a particular system but not to change ticksPerSec and use clock_gettime anyway.

And even if clock_gettime has poor resolution, I'm still not convinced that using gettimeofday is a good move for TickDuration. clock_gettime is being used to get a monotonic clock for TickDuration, which is what you definitely want for timing. Having to use gettimeofday for that is not good. If that's all we have, then that's all we have, but using a non-monotonic clock for timing is asking for trouble, because if the clock changes at all during the timing, it's going to screw up the result. And having a timer with poorer resolution is likely preferable to one that is outright wrong (which is what will happen if the clock gets adjusted while timing). So, for TickDuration, I'd be very much inclined to just stick with clock_gettime if it's available.

Changing std.datetime so that it uses gettimeofday instead of clock_gettime with CLOCK_REALTIME when the resolution is poor does make some sense (though I have to wonder if you're really getting a better result in that case rather than just getting the poor result from a different function). But using CLOCK_MONOTONIC with clock_gettime seems preferable to me over gettimeofday even with a poorer resolution, because it's monotonic.

As for your comment about misunderstanding clock_gettres, it's not like you can expect that each call to clock_gettime is going to be faster than the resolution of the clock. So, I don't think that it's at all surprising that you'd be getting times 217 nanoseconds apart with two calls to clock_gettime even with nanosecond resolution.

In any case, I think that you need to come up with examples of clock_gettres giving really poor resolutions before making any change like this makes sense, because gettimeofday does not use a monotonic clock and you really want to be using a monotonic clock for timing.

[fugalh]

Thanks for looking at this. I'm sorry for the confusing series of diffs.

I came to understand what's going on much better last night. Let me explain.

1. clock_gettime() IS returning nanosecond resolution on this system
   (linux 2.6.33 with CONFIG_HIGH_RES_TIMERS UNSET.) gettimeofday() IS
   returning microsecond resolution. So, clock_gettime() is definitely
   better. I demonstrated both of these empirically as I mentioned. The
   answer of ~300ns is not surprising because it is larger than 1ns, but
   because it is so much smaller than 1ms (the reported resolution from
   clock_getres()).
2. HOWEVER, clock_getres() is reporting a resolution of 999848 ns, or
   about 1ms. I confirmed that calls to usleep(), nanosleep(), and
   clock_nanosleep() are using this resolution, i.e. if you ask to sleep
   for one nanosecond you are liable to sleep for as much as one
   millisecond, and about half a millisecond on average.
3. Calls to select() and poll() are using the higher resolution timers
   (I found the kernel diffs enabling this). Although I didn't find the
   diffs responsible, I think clock_gettime() and gettimeofday() are also
   using the hi-res timers.
4. ticksPerSecond does not seem to be used for anything other than the
   timebase. When using gettimeofday() it is fixed at µsec resolution
   though we know that gettimeofday() makes no guarantee of µsec
   resolution. I think this is sensible. Just like you said even if we have
   nanosecond resolution nominally we can't expect a call to
   clock_gettime() to return in one nanosecond, but we use nanoseconds as
   the timebase in that case. Since struct timespec uses nanoseconds, I
   think that ticksPerSecond should use nanoseconds when using
   clock_gettime() no matter what clock_getres() returns, to avoid rounding
   errors and coincidentally fix the problem I was seeing.

To get back to the beginning, I noticed this problem because
Clock.currSystemTicks() was giving me even milliseconds. This is
worthless for timing what I need to time (I need at least microsecond
resolution) and didn't match what I knew the kernel and hardware could
do with a lowly gettimeofday() call in C. I agree with you about
CLOCK_MONOTONIC vs gettimeofday(), but pragmatically I don't see how
monotonically increasing millisecond resolution is better than
not-guaranteed-monotonic microsecond resolution.

Here's my C benchmark:

clock_getres(CLOCK_MONOTONIC, &ts1);
printf("{%d, %d}\n", ts1.tv_sec, ts1.tv_nsec);

clock_gettime(CLOCK_MONOTONIC, &ts1);
// clock_nanosleep(), or usleep() or select() etc.
clock_gettime(CLOCK_MONOTONIC, &ts2);
printf("%d.%09d\n", ts2.tv_sec - ts1.tv_sec, ts2.tv_nsec - ts2.tv_nsec);

I was seeing clock_getres() return {0, 999848} (about 1ms resolution),
clock_gettime() consistently returns a difference about 217 nanoseconds
(impossible if it's really tied to 1ms resolution), and
clock_nanosleep() sleeps for 1/2-1 millisecond (consistent with 1ms
resolution). Replacing calls go clock_gettime() with gettimeofday()
gives results consistent with at least microsecond resolution.

On 11/27/11 8:45 PM, Jonathan M Davis wrote:

I'm not at all comfortable with setting ticksPerSec to a value that doesn't match what we're getting back from clock_gettres and then still using clock_gettime. That seems completely wrong to me. It does make some sense to use gettimeofday if clock_gettime has poor resolution on a particular system but not to change ticksPerSec and use clock_gettime anyway.

And even if clock_gettime has poor resolution, I'm still not convinced that using gettimeofday is a good move for TickDuration. clock_gettime is being used to get a monotonic clock for TickDuration, which is what you definitely want for timing. Having to use gettimeofday for that is not good. If that's all we have, then that's all we have, but using a non-monotonic clock for timing is asking for trouble, because if the clock changes at all during the timing, it's going to screw up the result. And having a timer with poorer resolution is likely preferable to one that is outright wrong (which is what will happen if the clock gets adjusted while timing). So, for TickDuration, I'd be very much inclined to just stick with clock_gettime if it's available.

Changing std.datetime so that it uses gettimeofday instead of clock_gettime with CLOCK_REALTIME when the resolution is poor does make some sense (though I have to wonder if you're really getting a better result in that case rather than just getting the poor result from a different function). But using CLOCK_MONOTONIC with clock_gettime seems preferable to me over gettimeofday even with a poorer resolution, because it's monotonic.

As for your comment about misunderstanding clock_gettres, it's not like you can expect that each call to clock_gettime is going to be faster than the resolution of the clock. So, I don't think that it's at all surprising that you'd be getting times 217 nanoseconds apart with two calls to clock_gettime even with nanosecond resolution.

In any case, I think that you need to come up with examples of clock_gettres giving really poor resolutions before making any change like this makes sense, because gettimeofday does not use a monotonic clock and you really want to be using a monotonic clock for timing.

---

Reply to this email directly or view it on GitHub:
#88 (comment)

[jmdavis]

There are plenty of cases where it is far more important to have a monotonic clock than to have higher precision, much as the higher precision my be desired. One common case would be when playing video. The clock used must be monotonic, or you're going to run into issues when the clock gets changed and/or the time drifts. Frames won't be played with the proper timing and the video isn't going to play properly.

In other cases, you care more about precision. You'd rather get the time back in microseconds than milliseconds, and the fact that you may occasionally get incorrect results due to the clock being non-monotonic isn't a big deal.

In general though, there's no question IMHO, that the clock used for timing needs to be monotonic. The problem, of course, is what to do when the monotonic clock has issues and the choice is either monotonic or high precision. The best choice in that situation depends entirely on the use case.

Now, looking at the situation further, it does look like setting ticksPerSec to nanoseconds would work, though it would arguably mean that it would be giving incorrect information for those who actually cared about the clock's resolution. Still, I like the idea better than using gettimeofday. Honestly, I'm actually tempted to completely remove the possibility of using gettimeofday in TickDuration. As I understand it, all of the platforms that we support have a monotonic clock and none of the Posix systems need gettimeofday. So, if we can remove it, we can then guarantee that the clock used by TickDuration is monotonic, as timers should be.

So, you're currently suggested change is probably the correct way to go. I'm not enthused about ticksPerSec having nothing to do with the actual resolution of the clock (barring bugs with clock_gettres, I believe that it currently actually is the actual resolution on all platforms), but it does seem like it's probably the best choice given the bizarre issues with clock_gettres. I sure wish I knew why it was doing that though.

[jmdavis]

I just committed a slightly adjusted version of this fix.

[fugalh]

Awesome.