Fix logic on high Windows resolution timer and have

base/time/time_win.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 
 // Windows Timer Primer
 //
 // A good article:  http://www.ddj.com/windows/184416651
 // A good mozilla bug:  http://bugzilla.mozilla.org/show_bug.cgi?id=363258
 //
@@ skipping 11 matching lines @@
 // 1ms, but only if you call APIs (timeBeginPeriod()) which affect all other
 // applications on the system.  By default, precision is only 15.5ms.
 // Unfortunately, we don't want to call timeBeginPeriod because we don't
 // want to affect other applications.  Further, on mobile platforms, use of
 // faster multimedia timers can hurt battery life.  See the intel
 // article about this here:
 // http://softwarecommunity.intel.com/articles/eng/1086.htm
 //
 // To work around all this, we're going to generally use timeGetTime().  We
 // will only increase the system-wide timer if we're not running on battery
-// power.  Using timeBeginPeriod(1) is a requirement in order to make our
-// message loop waits have the same resolution that our time measurements
-// do.  Otherwise, WaitForSingleObject(..., 1) will no less than 15ms when
-// there is nothing else to waken the Wait.
+// power.
 
 #include "base/time/time.h"
 
 #pragma comment(lib, "winmm.lib")
 #include <windows.h>
 #include <mmsystem.h>
 
 #include "base/basictypes.h"
 #include "base/cpu.h"
 #include "base/lazy_instance.h"
@@ skipping 34 matching lines @@
 const int kMaxMillisecondsToAvoidDrift = 60 * Time::kMillisecondsPerSecond;
 
 int64 initial_time = 0;
 TimeTicks initial_ticks;
 
 void InitializeClock() {
   initial_ticks = TimeTicks::Now();
   initial_time = CurrentWallclockMicroseconds();
 }
 
+// The two values that ActivateHighResolutionTimer uses to set the systemwide
+// timer interrupt frequency on Windows. It controls how precise timers are
+// but also has a big impact on battery life.
+const int kMinTimerIntervalHighResMs = 1;
+const int kMinTimerIntervalLowResMs = 4;
+// Track if kMinTimerIntervalHighResMs or kMinTimerIntervalLowResMs is active.
+bool g_high_res_timer_enabled = false;
+// How many times the high resolution timer has been called.
+int g_high_res_timer_count = 0;
+// The lock to control access to the above two variables.
+base::LazyInstance<base::Lock> g_high_res_lock = LAZY_INSTANCE_INITIALIZER;

[Nico, 2014/08/26 20:15:58]

Can you make this ::Leaky?

+
 }  // namespace
 
 // Time -----------------------------------------------------------------------
 
 // The internal representation of Time uses FILETIME, whose epoch is 1601-01-01
 // 00:00:00 UTC.  ((1970-1601)*365+89)*24*60*60*1000*1000, where 89 is the
 // number of leap year days between 1601 and 1970: (1970-1601)/4 excluding
 // 1700, 1800, and 1900.
 // static
 const int64 Time::kTimeTToMicrosecondsOffset = GG_INT64_C(11644473600000000);
 
-bool Time::high_resolution_timer_enabled_ = false;
-int Time::high_resolution_timer_activated_ = 0;
-
 // static
 Time Time::Now() {
   if (initial_time == 0)
     InitializeClock();
 
   // We implement time using the high-resolution timers so that we can get
   // timeouts which are smaller than 10-15ms.  If we just used
   // CurrentWallclockMicroseconds(), we'd have the less-granular timer.
   //
   // To make this work, we initialize the clock (initial_time) and the
@@ skipping 44 matching lines @@
     result.dwLowDateTime = std::numeric_limits<DWORD>::max();
     return result;
   }
   FILETIME utc_ft;
   MicrosecondsToFileTime(us_, &utc_ft);
   return utc_ft;
 }
 
 // static
 void Time::EnableHighResolutionTimer(bool enable) {
-  // Test for single-threaded access.
-  static PlatformThreadId my_thread = PlatformThread::CurrentId();
-  DCHECK(PlatformThread::CurrentId() == my_thread);
-
-  if (high_resolution_timer_enabled_ == enable)
+  base::AutoLock lock(g_high_res_lock.Get());
+  if (g_high_res_timer_enabled == enable)
     return;
-
-  high_resolution_timer_enabled_ = enable;
+  g_high_res_timer_enabled = enable;
+  if (!g_high_res_timer_count)
+    return;
+  if (enable) {

[jamesr, 2014/08/26 19:47:40]

this bit might be worth a comment explaining the state transitions. rough
attempt:

Since g_high_res_timer_count is nonzero, an earlier call to
ActivateHighResolutionTimer(true) must have called timeBeginPeriod(). If we were
in high resolution mode beforehand, it would have called
timeBeginPeriod(kMinTimerIntervalHighResMs) otherwise it called
timeBeginPeriod(kMinTimerIntervalLowResMs). We need to end that period and start
a new one with the new desired resolution. A future call to
ActivateHighResolutionTimer(false) will end this period.

+    timeEndPeriod(kMinTimerIntervalLowResMs);
+    timeBeginPeriod(kMinTimerIntervalHighResMs);
+  } else {
+    timeEndPeriod(kMinTimerIntervalHighResMs);
+    timeBeginPeriod(kMinTimerIntervalLowResMs);
+  }
 }
 
 // static
 bool Time::ActivateHighResolutionTimer(bool activating) {
-  if (!high_resolution_timer_enabled_ && activating)
-    return false;
+  // We only do work on the transition from zero to one or one to zero so we
+  // can easily undo the effect (if necessary) when EnableHighResolutionTimer is
+  // called.
+  base::AutoLock lock(g_high_res_lock.Get());
+  UINT period = g_high_res_timer_enabled ? kMinTimerIntervalHighResMs
+                                         : kMinTimerIntervalLowResMs;
+  int high_res_count =
+      activating ? ++g_high_res_timer_count : --g_high_res_timer_count;
 
-  // Using anything other than 1ms makes timers granular
-  // to that interval.
-  const int kMinTimerIntervalMs = 1;
-  MMRESULT result;
   if (activating) {
-    result = timeBeginPeriod(kMinTimerIntervalMs);
-    high_resolution_timer_activated_++;
+    if (high_res_count == 1)
+      timeBeginPeriod(period);
   } else {
-    result = timeEndPeriod(kMinTimerIntervalMs);
-    high_resolution_timer_activated_--;
+    if (high_res_count == 0)
+      timeEndPeriod(period);
   }
-  return result == TIMERR_NOERROR;
+  return (period == kMinTimerIntervalHighResMs);
 }
 
 // static
 bool Time::IsHighResolutionTimerInUse() {
-  // Note:  we should track the high_resolution_timer_activated_ value
-  // under a lock if we want it to be accurate in a system with multiple
-  // message loops.  We don't do that - because we don't want to take the
-  // expense of a lock for this.  We *only* track this value so that unit
-  // tests can see if the high resolution timer is on or off.
-  return high_resolution_timer_enabled_ &&
-      high_resolution_timer_activated_ > 0;
+  base::AutoLock lock(g_high_res_lock.Get());
+  return g_high_res_timer_enabled && g_high_res_timer_count > 0;
 }
 
 // static
 Time Time::FromExploded(bool is_local, const Exploded& exploded) {
   // Create the system struct representing our exploded time. It will either be
   // in local time or UTC.
   SYSTEMTIME st;
   st.wYear = exploded.year;
   st.wMonth = exploded.month;
   st.wDayOfWeek = exploded.day_of_week;
@@ skipping 315 matching lines @@
     return TimeTicks() + TimeDelta::FromMilliseconds(timeGetTime());
   }
 }
 
 // TimeDelta ------------------------------------------------------------------
 
 // static
 TimeDelta TimeDelta::FromQPCValue(LONGLONG qpc_value) {
   return TimeDelta(GetHighResNowSingleton()->QPCValueToMicroseconds(qpc_value));
 }