[Windows] One TimeTicks clock: efficient/reliable high-res, with low-res fallback.

base/time/time_win.cc

Index: base/time/time_win.cc
diff --git a/base/time/time_win.cc b/base/time/time_win.cc
index 9b4f17d6613c7e60b8576adc9ae3b2a6afc8c8a3..a10d80587fe2955d611fb6e90f745edbe8d19535 100644
--- a/base/time/time_win.cc
+++ b/base/time/time_win.cc
@@ -307,13 +307,13 @@ DWORD timeGetTimeWrapper() {
   return timeGetTime();
 }
 
-DWORD (*tick_function)(void) = &timeGetTimeWrapper;
+DWORD (*g_tick_function)(void) = &timeGetTimeWrapper;
 
 // Accumulation of time lost due to rollover (in milliseconds).
-int64 rollover_ms = 0;
+int64 g_rollover_ms = 0;
 
 // The last timeGetTime value we saw, to detect rollover.
-DWORD last_seen_now = 0;
+DWORD g_last_seen_now = 0;
 
 // Lock protecting rollover_ms and last_seen_now.
 // Note: this is a global object, and we usually avoid these. However, the time
@@ -321,31 +321,26 @@ DWORD last_seen_now = 0;
 // easy to use a Singleton without even knowing it, and that may lead to many
 // gotchas). Its impact on startup time should be negligible due to low-level
 // nature of time code.
-base::Lock rollover_lock;
+base::Lock g_rollover_lock;
 
 // We use timeGetTime() to implement TimeTicks::Now().  This can be problematic
 // because it returns the number of milliseconds since Windows has started,
 // which will roll over the 32-bit value every ~49 days.  We try to track
 // rollover ourselves, which works if TimeTicks::Now() is called at least every
 // 49 days.
-TimeDelta RolloverProtectedNow() {
-  base::AutoLock locked(rollover_lock);
+TimeTicks RolloverProtectedNow() {
+  base::AutoLock locked(g_rollover_lock);
   // We should hold the lock while calling tick_function to make sure that
   // we keep last_seen_now stay correctly in sync.
-  DWORD now = tick_function();
-  if (now < last_seen_now)
-    rollover_ms += 0x100000000I64;  // ~49.7 days.
-  last_seen_now = now;
-  return TimeDelta::FromMilliseconds(now + rollover_ms);
+  DWORD now = g_tick_function();
+  if (now < g_last_seen_now)
+    g_rollover_ms += 0x100000000I64;  // ~49.7 days.
+  g_last_seen_now = now;
+  return TimeTicks() + TimeDelta::FromMilliseconds(now + g_rollover_ms);
 }
 
-bool IsBuggyAthlon(const base::CPU& cpu) {
-  // On Athlon X2 CPUs (e.g. model 15) QueryPerformanceCounter is
-  // unreliable.  Fallback to low-res clock.
-  return cpu.vendor_name() == "AuthenticAMD" && cpu.family() == 15;
-}
-
-// Overview of time counters:
+// Discussion of tick counter options on Windows:
+//
 // (1) CPU cycle counter. (Retrieved via RDTSC)
 // The CPU counter provides the highest resolution time stamp and is the least
 // expensive to retrieve. However, the CPU counter is unreliable and should not
@@ -373,117 +368,91 @@ bool IsBuggyAthlon(const base::CPU& cpu) {
 // (3) System time. The system time provides a low-resolution (typically 10ms
 // to 55 milliseconds) time stamp but is comparatively less expensive to
 // retrieve and more reliable.
-class HighResNowSingleton {
- public:
-  HighResNowSingleton()
-      : ticks_per_second_(0),
-        skew_(0) {
-
-    base::CPU cpu;
-    if (IsBuggyAthlon(cpu))
-      return;
-
-    // Synchronize the QPC clock with GetSystemTimeAsFileTime.
-    LARGE_INTEGER ticks_per_sec = {0};
-    if (!QueryPerformanceFrequency(&ticks_per_sec))
-      return; // QPC is not available.
-    ticks_per_second_ = ticks_per_sec.QuadPart;
-
-    skew_ = UnreliableNow() - ReliableNow();
-  }
 
-  bool IsUsingHighResClock() {
-    return ticks_per_second_ != 0;
-  }
+using NowFunction = TimeTicks (*)(void);
 
-  TimeDelta Now() {
-    if (IsUsingHighResClock())
-      return TimeDelta::FromMicroseconds(UnreliableNow());
-
-    // Just fallback to the slower clock.
-    return RolloverProtectedNow();
-  }
-
-  int64 GetQPCDriftMicroseconds() {
-    if (!IsUsingHighResClock())
-      return 0;
-    return abs((UnreliableNow() - ReliableNow()) - skew_);
-  }
+TimeTicks InitialNowFunction();
+TimeTicks InitialSystemTraceNowFunction();
 
-  int64 QPCValueToMicroseconds(LONGLONG qpc_value) {
-    if (!ticks_per_second_)
-      return 0;
-    // If the QPC Value is below the overflow threshold, we proceed with
-    // simple multiply and divide.
-    if (qpc_value < Time::kQPCOverflowThreshold)
-      return qpc_value * Time::kMicrosecondsPerSecond / ticks_per_second_;
-    // Otherwise, calculate microseconds in a round about manner to avoid
-    // overflow and precision issues.
-    int64 whole_seconds = qpc_value / ticks_per_second_;
-    int64 leftover_ticks = qpc_value - (whole_seconds * ticks_per_second_);
-    int64 microseconds = (whole_seconds * Time::kMicrosecondsPerSecond) +
-                         ((leftover_ticks * Time::kMicrosecondsPerSecond) /
-                          ticks_per_second_);
-    return microseconds;
-  }
+volatile NowFunction g_now_function = &InitialNowFunction;
+volatile NowFunction g_system_trace_now_function =
+    &InitialSystemTraceNowFunction;
+int64 g_qpc_ticks_per_second = 0;
 
- private:
-  // Get the number of microseconds since boot in an unreliable fashion.
-  int64 UnreliableNow() {
-    LARGE_INTEGER now;
-    QueryPerformanceCounter(&now);
-    return QPCValueToMicroseconds(now.QuadPart);
-  }
+TimeDelta QPCValueToTimeDelta(LONGLONG qpc_value) {
+  DCHECK_GT(g_qpc_ticks_per_second, 0);
 
-  // Get the number of microseconds since boot in a reliable fashion.
-  int64 ReliableNow() {
-    return RolloverProtectedNow().InMicroseconds();
+  // If the QPC Value is below the overflow threshold, we proceed with
+  // simple multiply and divide.
+  if (qpc_value < Time::kQPCOverflowThreshold) {
+    return TimeDelta::FromMicroseconds(
+        qpc_value * Time::kMicrosecondsPerSecond / g_qpc_ticks_per_second);
   }
-
-  int64 ticks_per_second_;  // 0 indicates QPF failed and we're broken.
-  int64 skew_;  // Skew between lo-res and hi-res clocks (for debugging).
-};
-
-static base::LazyInstance<HighResNowSingleton>::Leaky

[brianderson, 2015/01/07 00:53:44]

You are no longer using a LazyInstance for QPC. We'll need to be extra careful
that there are no races in your new code.

[miu, 2015/01/07 05:22:45]

Acknowledged.

-    leaky_high_res_now_singleton = LAZY_INSTANCE_INITIALIZER;
-
-HighResNowSingleton* GetHighResNowSingleton() {
-  return leaky_high_res_now_singleton.Pointer();
+  // Otherwise, calculate microseconds in a round about manner to avoid
+  // overflow and precision issues.
+  int64 whole_seconds = qpc_value / g_qpc_ticks_per_second;
+  int64 leftover_ticks = qpc_value - (whole_seconds * g_qpc_ticks_per_second);
+  return TimeDelta::FromMicroseconds(
+      (whole_seconds * Time::kMicrosecondsPerSecond) +
+      ((leftover_ticks * Time::kMicrosecondsPerSecond) /
+       g_qpc_ticks_per_second));
 }
 
-TimeDelta HighResNowWrapper() {
-  return GetHighResNowSingleton()->Now();
+TimeTicks QPCNow() {
+  LARGE_INTEGER now;
+  QueryPerformanceCounter(&now);
+  return TimeTicks() + QPCValueToTimeDelta(now.QuadPart);
 }
 
-typedef TimeDelta (*NowFunction)(void);
-
-bool CPUReliablySupportsHighResTime() {
+void InitializeNowFunctionPointers() {
+  LARGE_INTEGER ticks_per_sec = {0};
+  if (QueryPerformanceFrequency(&ticks_per_sec))
+    g_qpc_ticks_per_second = ticks_per_sec.QuadPart;

[brianderson, 2015/01/07 00:53:44]

g_qpc_ticks_per_second is 64-bits.

According to
http://msdn.microsoft.com/en-us/library/windows/desktop/ms684122(v=vs.85).aspx
simple
64-bit reads/writes are not guaranteed to be atomic on 32-bit Windows.

Would you need to use InterlockedExchange64 to set it and something like
InterlockedAdd64(*,0) to read it?

QueryPerformanceFrequency likely returns the same value no matter how often it
is called, but is that an assumption we want to make? If it doesn't (for example
by returning 0 then non-0), then it would not work. If it does, we're probably
okay, but it's hard to reason about.

[DaleCurtis, 2015/01/07 01:38:36]

Yeah this seems fishy, for the same reason that the InterlockedExchange calls
are required for the pointers below.  You may have multiple
InitializeNowFunctionPointers() calls in flight. You should probably use a local
value for most of the function and an InterlockedExchange64() below with the
pointer setup functions.

Presumably that's safe for the same reason the pointer setup is safe: these
values aren't going to change between calls.

[miu, 2015/01/07 05:22:45]

Fixed.  My solution is to get rid of all the InterlockedExchangeXXX() mess and
simply put a single memory barrier in-place to guarantee visibility ordering. 
This resolves both the atomicity problem Brian pointed out as well as the
parallel InitializeNowFunctionPointers() calls.  See comments I added to the
code for further explanation.
The MSDN documentation for this function states, "The frequency of the
performance counter is fixed at system boot and is consistent across all
processors."  Also, this is what the old code assumed.  So, I think we're good
here.

+  else
+    g_qpc_ticks_per_second = 0;
+
+  // If Windows does not offer a working QPC implementation, both Now() and
+  // NowFromSystemTraceTime() must use the low-resolution clock. Note that
+  // Windows may report a working QPC implementation even on certain Athlon X2
+  // CPUs (where QPC has been shown to be unreliable).
+  //
+  // Otherwise, if the CPU does not have a non-stop time counter, assume Windows
+  // will provide an alternate QPC implementation that works, but is expensive
+  // to use. In this case, Now() should use the inexpensive, low-resolution
+  // clock and NowFromSystemTraceTime() will use the expensive-but-working QPC
+  // clock.
+  //
+  // Otherwise, both Now functions can use the high-resolution QPC clock. As of
+  // 4 January 2015, ~68% of users fall within this category.
+  NowFunction now_function;
+  NowFunction system_trace_now_function;
   base::CPU cpu;
-  if (!cpu.has_non_stop_time_stamp_counter() ||
-      !GetHighResNowSingleton()->IsUsingHighResClock())
-    return false;
-
-  if (IsBuggyAthlon(cpu))
-    return false;
+  if ((g_qpc_ticks_per_second <= 0) ||
+      (cpu.vendor_name() == "AuthenticAMD" && cpu.family() == 15)) {

[brianderson, 2015/01/07 00:53:44]

I liked the IsBuggyAthlon function, it would make this if easier to understand.

[miu, 2015/01/07 05:22:45]

Done.

+    now_function = system_trace_now_function = &RolloverProtectedNow;

[brianderson, 2015/01/07 00:53:44]

I also think system trace should always use HighResNow, even if it is slow or
unreliable.

[miu, 2015/01/07 05:22:45]

Done.

+  } else if (!cpu.has_non_stop_time_stamp_counter()) {
+    now_function = &RolloverProtectedNow;
+    system_trace_now_function = &QPCNow;
+  } else {
+    now_function = system_trace_now_function = &QPCNow;
+  }
 
-  return true;
+  InterlockedExchangePointer(
+      reinterpret_cast<void* volatile*>(&g_now_function),
+      now_function);
+  InterlockedExchangePointer(
+      reinterpret_cast<void* volatile*>(&g_system_trace_now_function),
+      system_trace_now_function);
 }
 
-TimeDelta InitialNowFunction();
-
-volatile NowFunction now_function = InitialNowFunction;
+TimeTicks InitialNowFunction() {
+  InitializeNowFunctionPointers();
+  return g_now_function();
+}
 
-TimeDelta InitialNowFunction() {
-  if (!CPUReliablySupportsHighResTime()) {
-    InterlockedExchangePointer(
-        reinterpret_cast<void* volatile*>(&now_function),
-        &RolloverProtectedNow);
-    return RolloverProtectedNow();
-  }
-  InterlockedExchangePointer(
-        reinterpret_cast<void* volatile*>(&now_function),
-        &HighResNowWrapper);
-  return HighResNowWrapper();
+TimeTicks InitialSystemTraceNowFunction() {
+  InitializeNowFunctionPointers();
+  return g_system_trace_now_function();
 }
 
 }  // namespace
@@ -491,27 +460,24 @@ TimeDelta InitialNowFunction() {
 // static
 TimeTicks::TickFunctionType TimeTicks::SetMockTickFunction(
     TickFunctionType ticker) {
-  base::AutoLock locked(rollover_lock);
-  TickFunctionType old = tick_function;
-  tick_function = ticker;
-  rollover_ms = 0;
-  last_seen_now = 0;
+  base::AutoLock locked(g_rollover_lock);
+  TickFunctionType old = g_tick_function;
+  g_tick_function = ticker;
+  g_rollover_ms = 0;
+  g_last_seen_now = 0;
   return old;
 }
 
 // static
 TimeTicks TimeTicks::Now() {
-  return TimeTicks() + now_function();
+  return g_now_function();
 }
 
 // static
-TimeTicks TimeTicks::HighResNow() {
-  return TimeTicks() + HighResNowWrapper();
-}
-
-// static
-bool TimeTicks::IsHighResNowFastAndReliable() {
-  return CPUReliablySupportsHighResTime();
+bool TimeTicks::IsHighResolution() {
+  if (g_now_function == &InitialNowFunction)

[DaleCurtis, 2015/01/07 01:38:36]

This needs an interlocked read.

[miu, 2015/01/07 05:22:45]

With the new, simpler memory-ordering model (PS3), it doesn't anymore.  The new
value for |g_now_function| is either visible to the thread, or it isn't and a
parallel call to InitializeNowFunctionPointers() could result (which is okay).

+    InitializeNowFunctionPointers();
+  return g_now_function == &QPCNow;
 }
 
 // static
@@ -522,27 +488,17 @@ TimeTicks TimeTicks::ThreadNow() {
 
 // static
 TimeTicks TimeTicks::NowFromSystemTraceTime() {
-  return HighResNow();
-}
-
-// static
-int64 TimeTicks::GetQPCDriftMicroseconds() {
-  return GetHighResNowSingleton()->GetQPCDriftMicroseconds();
+  return g_system_trace_now_function();
 }
 
 // static
 TimeTicks TimeTicks::FromQPCValue(LONGLONG qpc_value) {
-  return TimeTicks(GetHighResNowSingleton()->QPCValueToMicroseconds(qpc_value));
-}
-
-// static
-bool TimeTicks::IsHighResClockWorking() {
-  return GetHighResNowSingleton()->IsUsingHighResClock();
+  return TimeTicks() + QPCValueToTimeDelta(qpc_value);

[DaleCurtis, 2015/01/07 01:38:36]

Any chance someone might call FromQPCValue() below any of the above calls?

[miu, 2015/01/07 05:22:45]

No.  I changed the header comment in time.h to explicitly read "Do NOT attempt
to use this if IsHighResolution() returns false."  This means IsHighResolution()
must be called before this function, which will guarantee all the globals are
initialized first.  I've confirmed all uses of this function make the
IsHighResolution() call first, and there's a DCHECK in QPCValueToTimeDelta() to
ensure it stays that way for the future.

 }
 
 // TimeDelta ------------------------------------------------------------------
 
 // static
 TimeDelta TimeDelta::FromQPCValue(LONGLONG qpc_value) {
-  return TimeDelta(GetHighResNowSingleton()->QPCValueToMicroseconds(qpc_value));
+  return QPCValueToTimeDelta(qpc_value);
 }