Time::getMonotonicClockMillisecond()

Hi,

I’ve come to a bug that was difficult to reproduce. I’ve found a way to reproduce it.
Currently the code does basically this:

double currentTime = Time::getMillisecondCounter();
const double targetTime = currentTime + 30ms;
while (!threadShouldExit() && currentTime < targetTime) 
{
   // Do something...
   currentTime = Time::getMillisecondCounter();
}

However, I got a crash when the user decided to set his clock back from few days. The loop wouldn’t exit, and the Thread was force to stop (which is bad).
There are possible workaround, like adding a test ( && currentTime + 30 > targetTime) to detect clock change, but it’s just a workaround with multiple defects.

Since all OS provide monotonic clock, could you add a Time::getMonotonicMillisecondCounter() or something like for this case ?
Under linux and mac, you only need to call clock_gettime(CLOCK_MONOTONIC), Windows is GetTickCount64().
That way, we don’t need to bother (or even think) about the system time in any duration based test.

Very interesting!

The intention was always that getMillisecondCounter() should return a monotonic time… On the mac, it’s correct, because it uses mach_absolute_time().

I’m extremely surprised that timeGetTime on windows doesn’t behave that way - I always thought it returned the milliseconds since startup, which you’d kind of expect to be monotonic…! At least that’s an easy fix: just a straight swap with GetTickCount().

And I wish I’d known about the clock_gettime function when I wrote the linux code originally - that’s a better fit for the code than the way it’s currently done.

Ok, then, a change is required. On Windows, don’t use GetTickCount(), but GetTickCount64() (the former is not monotonic in human scales).
I think clock_gettime is also available on Mac, since it’s in the posix standard. It might worth merging this for both linux and mac, in order to simplify the code, don’t you think ?

GetTickCount will do the job, as the class uses a uint32 anyway, so GetTickCount64 would just get truncated. At some point I should move to a 64-bit number for those values, but that’d break a lot of user’s code so isn’t a high priority.

Just doing some checking, and I think you may be a bit confused, cyril… timeGetTime is monotonic, and I just tested it by messing with my clock, and it keeps on counting upwards. GetTickCount is actually too low-resolution to be any use as a replacement anyway, and according to MSDN GetTickCount IS affected by changes to the system time!

I’m guessing that your original problem happened on linux?

Yes, I’ve seen the problem on linux. It’s strange about the Win32 function, as MSDN says "The resolution of the GetTickCount function is not affected by adjustments made by the GetSystemTimeAdjustment function."
Anyway, if timeGetTime is also not changed when modifying the system’s time, it’s ok (and a lot less work).