High resolution timer fix reland

On Windows the message pump code tried to manage the systemwide timer
resolution to fire delayed tasks with better than 15ms resolution but
it is buggy.

This is https://codereview.chromium.org/395913006

please see that review for rationale.

BUG=153139
TBR=jamesr,darin
TEST=included, also see bug for manual verification.

Review URL: https://codereview.chromium.org/509223002

Cr-Commit-Position: refs/heads/master@{#292493}

base/message_loop/incoming_task_queue.cc

@@ -7,12 +7,14 @@
 #include "base/location.h"
 #include "base/message_loop/message_loop.h"
 #include "base/synchronization/waitable_event.h"
+#include "base/time/time.h"
 
 namespace base {
 namespace internal {
 
 IncomingTaskQueue::IncomingTaskQueue(MessageLoop* message_loop)
-    : message_loop_(message_loop),
+    : high_res_task_count_(0),
+      message_loop_(message_loop),
       next_sequence_num_(0) {
 }
 
@@ -24,15 +26,23 @@ bool IncomingTaskQueue::AddToIncomingQueue(
   AutoLock locked(incoming_queue_lock_);
   PendingTask pending_task(
       from_here, task, CalculateDelayedRuntime(delay), nestable);
+#if defined(OS_WIN)
+  // We consider the task needs a high resolution timer if the delay is
+  // more than 0 and less than 32ms. This caps the relative error to
+  // less than 50% : a 33ms wait can wake at 48ms since the default
+  // resolution on Windows is between 10 and 15ms.
+  if (delay > TimeDelta() &&
+      delay.InMilliseconds() < (2 * Time::kMinLowResolutionThresholdMs)) {
+    ++high_res_task_count_;
+    pending_task.is_high_res = true;
+  }
+#endif
   return PostPendingTask(&pending_task);
 }
 
-bool IncomingTaskQueue::IsHighResolutionTimerEnabledForTesting() {
-#if defined(OS_WIN)
-  return !high_resolution_timer_expiration_.is_null();
-#else
-  return true;
-#endif
+bool IncomingTaskQueue::HasHighResolutionTasks() {
+  AutoLock lock(incoming_queue_lock_);
+  return high_res_task_count_ > 0;
 }
 
 bool IncomingTaskQueue::IsIdleForTesting() {
@@ -40,29 +50,22 @@ bool IncomingTaskQueue::IsIdleForTesting() {
   return incoming_queue_.empty();
 }
 
-void IncomingTaskQueue::ReloadWorkQueue(TaskQueue* work_queue) {
+int IncomingTaskQueue::ReloadWorkQueue(TaskQueue* work_queue) {
   // Make sure no tasks are lost.
   DCHECK(work_queue->empty());
 
   // Acquire all we can from the inter-thread queue with one lock acquisition.
   AutoLock lock(incoming_queue_lock_);
   if (!incoming_queue_.empty())
-    incoming_queue_.Swap(work_queue);  // Constant time
+    incoming_queue_.Swap(work_queue);
 
-  DCHECK(incoming_queue_.empty());
+  // Reset the count of high resolution tasks since our queue is now empty.
+  int high_res_tasks = high_res_task_count_;
+  high_res_task_count_ = 0;
+  return high_res_tasks;
 }
 
 void IncomingTaskQueue::WillDestroyCurrentMessageLoop() {
-#if defined(OS_WIN)
-  // If we left the high-resolution timer activated, deactivate it now.
-  // Doing this is not-critical, it is mainly to make sure we track
-  // the high resolution timer activations properly in our unit tests.
-  if (!high_resolution_timer_expiration_.is_null()) {
-    Time::ActivateHighResolutionTimer(false);
-    high_resolution_timer_expiration_ = TimeTicks();
-  }
-#endif
-
   AutoLock lock(incoming_queue_lock_);
   message_loop_ = NULL;
 }
@@ -74,40 +77,10 @@ IncomingTaskQueue::~IncomingTaskQueue() {
 
 TimeTicks IncomingTaskQueue::CalculateDelayedRuntime(TimeDelta delay) {
   TimeTicks delayed_run_time;
-  if (delay > TimeDelta()) {
+  if (delay > TimeDelta())
     delayed_run_time = TimeTicks::Now() + delay;
-
-#if defined(OS_WIN)
-    if (high_resolution_timer_expiration_.is_null()) {
-      // Windows timers are granular to 15.6ms.  If we only set high-res
-      // timers for those under 15.6ms, then a 18ms timer ticks at ~32ms,
-      // which as a percentage is pretty inaccurate.  So enable high
-      // res timers for any timer which is within 2x of the granularity.
-      // This is a tradeoff between accuracy and power management.
-      bool needs_high_res_timers = delay.InMilliseconds() <
-          (2 * Time::kMinLowResolutionThresholdMs);
-      if (needs_high_res_timers) {
-        if (Time::ActivateHighResolutionTimer(true)) {
-          high_resolution_timer_expiration_ = TimeTicks::Now() +
-              TimeDelta::FromMilliseconds(
-                  MessageLoop::kHighResolutionTimerModeLeaseTimeMs);
-        }
-      }
-    }
-#endif
-  } else {
+  else
     DCHECK_EQ(delay.InMilliseconds(), 0) << "delay should not be negative";
-  }
-
-#if defined(OS_WIN)
-  if (!high_resolution_timer_expiration_.is_null()) {
-    if (TimeTicks::Now() > high_resolution_timer_expiration_) {
-      Time::ActivateHighResolutionTimer(false);
-      high_resolution_timer_expiration_ = TimeTicks();
-    }
-  }
-#endif
-
   return delayed_run_time;
 }
 

base/message_loop/incoming_task_queue.h

@@ -38,16 +38,17 @@ class BASE_EXPORT IncomingTaskQueue
                           TimeDelta delay,
                           bool nestable);
 
-  // Returns true if the message loop has high resolution timers enabled.
-  // Provided for testing.
-  bool IsHighResolutionTimerEnabledForTesting();
+  // Returns true if the queue contains tasks that require higher than default
+  // timer resolution. Currently only needed for Windows.
+  bool HasHighResolutionTasks();
 
   // Returns true if the message loop is "idle". Provided for testing.
   bool IsIdleForTesting();
 
   // Loads tasks from the |incoming_queue_| into |*work_queue|. Must be called
-  // from the thread that is running the loop.
-  void ReloadWorkQueue(TaskQueue* work_queue);
+  // from the thread that is running the loop. Returns the number of tasks that
+  // require high resolution timers.
+  int ReloadWorkQueue(TaskQueue* work_queue);
 
   // Disconnects |this| from the parent message loop.
   void WillDestroyCurrentMessageLoop();
@@ -65,12 +66,11 @@ class BASE_EXPORT IncomingTaskQueue
   // does not retain |pending_task->task| beyond this function call.
   bool PostPendingTask(PendingTask* pending_task);
 
-#if defined(OS_WIN)
-  TimeTicks high_resolution_timer_expiration_;
-#endif
+  // Number of tasks that require high resolution timing. This value is kept
+  // so that ReloadWorkQueue() completes in constant time.
+  int high_res_task_count_;
 
-  // The lock that protects access to |incoming_queue_|, |message_loop_| and
-  // |next_sequence_num_|.
+  // The lock that protects access to the members of this class.
   base::Lock incoming_queue_lock_;
 
   // An incoming queue of tasks that are acquired under a mutex for processing

base/message_loop/message_loop.cc

@@ -127,6 +127,8 @@ MessageLoop::DestructionObserver::~DestructionObserver() {
 
 MessageLoop::MessageLoop(Type type)
     : type_(type),
+      pending_high_res_tasks_(0),
+      in_high_res_mode_(false),
       nestable_tasks_allowed_(true),
 #if defined(OS_WIN)
       os_modal_loop_(false),
@@ -141,6 +143,8 @@ MessageLoop::MessageLoop(Type type)
 MessageLoop::MessageLoop(scoped_ptr<MessagePump> pump)
     : pump_(pump.Pass()),
       type_(TYPE_CUSTOM),
+      pending_high_res_tasks_(0),
+      in_high_res_mode_(false),
       nestable_tasks_allowed_(true),
 #if defined(OS_WIN)
       os_modal_loop_(false),
@@ -155,7 +159,10 @@ MessageLoop::~MessageLoop() {
   DCHECK_EQ(this, current());
 
   DCHECK(!run_loop_);
-
+#if defined(OS_WIN)
+  if (in_high_res_mode_)
+    Time::ActivateHighResolutionTimer(false);
+#endif
   // Clean up any unprocessed tasks, but take care: deleting a task could
   // result in the addition of more tasks (e.g., via DeleteSoon).  We set a
   // limit on the number of times we will allow a deleted task to generate more
@@ -369,8 +376,8 @@ bool MessageLoop::is_running() const {
   return run_loop_ != NULL;
 }
 
-bool MessageLoop::IsHighResolutionTimerEnabledForTesting() {
-  return incoming_task_queue_->IsHighResolutionTimerEnabledForTesting();
+bool MessageLoop::HasHighResolutionTasks() {
+  return incoming_task_queue_->HasHighResolutionTasks();
 }
 
 bool MessageLoop::IsIdleForTesting() {
@@ -425,6 +432,10 @@ bool MessageLoop::ProcessNextDelayedNonNestableTask() {
 void MessageLoop::RunTask(const PendingTask& pending_task) {
   DCHECK(nestable_tasks_allowed_);
 
+  if (pending_task.is_high_res) {
+    pending_high_res_tasks_--;
+    CHECK(pending_high_res_tasks_ >= 0);
+  }
   // Execute the task and assume the worst: It is probably not reentrant.
   nestable_tasks_allowed_ = false;
 
@@ -493,8 +504,10 @@ void MessageLoop::ReloadWorkQueue() {
   // |*work_queue| by waiting until the last minute (|*work_queue| is empty) to
   // load. That reduces the number of locks-per-task significantly when our
   // queues get large.
-  if (work_queue_.empty())
-    incoming_task_queue_->ReloadWorkQueue(&work_queue_);
+  if (work_queue_.empty()) {
+    pending_high_res_tasks_ +=
+        incoming_task_queue_->ReloadWorkQueue(&work_queue_);
+  }
 }
 
 void MessageLoop::ScheduleWork(bool was_empty) {
@@ -597,6 +610,15 @@ bool MessageLoop::DoIdleWork() {
   if (run_loop_->quit_when_idle_received_)
     pump_->Quit();
 
+  // When we return we will do a kernel wait for more tasks.
+#if defined(OS_WIN)
+  // On Windows we activate the high resolution timer so that the wait
+  // _if_ triggered by the timer happens with good resolution. If we don't
+  // do this the default resolution is 15ms which might not be acceptable
+  // for some tasks.
+  in_high_res_mode_ = pending_high_res_tasks_ > 0;
+  Time::ActivateHighResolutionTimer(in_high_res_mode_);
+#endif
   return false;
 }
 

base/message_loop/message_loop.h

@@ -371,10 +371,6 @@ class BASE_EXPORT MessageLoop : public MessagePump::Delegate {
   void AddTaskObserver(TaskObserver* task_observer);
   void RemoveTaskObserver(TaskObserver* task_observer);
 
-  // When we go into high resolution timer mode, we will stay in hi-res mode
-  // for at least 1s.
-  static const int kHighResolutionTimerModeLeaseTimeMs = 1000;
-
 #if defined(OS_WIN)
   void set_os_modal_loop(bool os_modal_loop) {
     os_modal_loop_ = os_modal_loop;
@@ -390,7 +386,7 @@ class BASE_EXPORT MessageLoop : public MessagePump::Delegate {
 
   // Returns true if the message loop has high resolution timers enabled.
   // Provided for testing.
-  bool IsHighResolutionTimerEnabledForTesting();
+  bool HasHighResolutionTasks();
 
   // Returns true if the message loop is "idle". Provided for testing.
   bool IsIdleForTesting();
@@ -459,6 +455,14 @@ class BASE_EXPORT MessageLoop : public MessagePump::Delegate {
   // this queue is only accessed (push/pop) by our current thread.
   TaskQueue work_queue_;
 
+  // How many high resolution tasks are in the pending task queue. This value
+  // increases by N every time we call ReloadWorkQueue() and decreases by 1
+  // every time we call RunTask() if the task needs a high resolution timer.
+  int pending_high_res_tasks_;
+  // Tracks if we have requested high resolution timers. Its only use is to
+  // turn off the high resolution timer upon loop destruction.
+  bool in_high_res_mode_;
+
   // Contains delayed tasks, sorted by their 'delayed_run_time' property.
   DelayedTaskQueue delayed_work_queue_;
 

base/message_loop/message_loop_unittest.cc

@@ -730,30 +730,20 @@ TEST(MessageLoopTest, HighResolutionTimer) {
   const TimeDelta kFastTimer = TimeDelta::FromMilliseconds(5);
   const TimeDelta kSlowTimer = TimeDelta::FromMilliseconds(100);
 
-  EXPECT_FALSE(loop.IsHighResolutionTimerEnabledForTesting());
-
+  EXPECT_FALSE(loop.HasHighResolutionTasks());
   // Post a fast task to enable the high resolution timers.
   loop.PostDelayedTask(FROM_HERE, Bind(&PostNTasksThenQuit, 1),
                        kFastTimer);
+  EXPECT_TRUE(loop.HasHighResolutionTasks());
   loop.Run();
-  EXPECT_TRUE(loop.IsHighResolutionTimerEnabledForTesting());
-
-  // Post a slow task and verify high resolution timers
-  // are still enabled.
-  loop.PostDelayedTask(FROM_HERE, Bind(&PostNTasksThenQuit, 1),
-                       kSlowTimer);
-  loop.Run();
-  EXPECT_TRUE(loop.IsHighResolutionTimerEnabledForTesting());
-
-  // Wait for a while so that high-resolution mode elapses.
-  PlatformThread::Sleep(TimeDelta::FromMilliseconds(
-      MessageLoop::kHighResolutionTimerModeLeaseTimeMs));
-
-  // Post a slow task to disable the high resolution timers.
+  EXPECT_FALSE(loop.HasHighResolutionTasks());
+  EXPECT_FALSE(Time::IsHighResolutionTimerInUse());
+  // Check that a slow task does not trigger the high resolution logic.
   loop.PostDelayedTask(FROM_HERE, Bind(&PostNTasksThenQuit, 1),
                        kSlowTimer);
+  EXPECT_FALSE(loop.HasHighResolutionTasks());
   loop.Run();
-  EXPECT_FALSE(loop.IsHighResolutionTimerEnabledForTesting());
+  EXPECT_FALSE(loop.HasHighResolutionTasks());
 }
 
 #endif  // defined(OS_WIN)

base/message_loop/message_pump.h

@@ -39,7 +39,8 @@ class BASE_EXPORT MessagePump : public NonThreadSafe {
     virtual bool DoDelayedWork(TimeTicks* next_delayed_work_time) = 0;
 
     // Called from within Run just before the message pump goes to sleep.
-    // Returns true to indicate that idle work was done.
+    // Returns true to indicate that idle work was done. Returning false means
+    // the pump will now wait.
     virtual bool DoIdleWork() = 0;
   };
 

base/pending_task.cc

@@ -8,19 +8,14 @@
 
 namespace base {
 
-#if _MSC_VER >= 1700
-// This a temporary fix for compiling on VS2012. http://crbug.com/154744
-PendingTask::PendingTask() : sequence_num(-1), nestable(false) {
-}
-#endif
-
 PendingTask::PendingTask(const tracked_objects::Location& posted_from,
                          const base::Closure& task)
     : base::TrackingInfo(posted_from, TimeTicks()),
       task(task),
       posted_from(posted_from),
       sequence_num(0),
-      nestable(true) {
+      nestable(true),
+      is_high_res(false) {
 }
 
 PendingTask::PendingTask(const tracked_objects::Location& posted_from,
@@ -31,7 +26,8 @@ PendingTask::PendingTask(const tracked_objects::Location& posted_from,
       task(task),
       posted_from(posted_from),
       sequence_num(0),
-      nestable(nestable) {
+      nestable(nestable),
+      is_high_res(false) {
 }
 
 PendingTask::~PendingTask() {

base/pending_task.h

@@ -18,9 +18,6 @@ namespace base {
 // Contains data about a pending task. Stored in TaskQueue and DelayedTaskQueue
 // for use by classes that queue and execute tasks.
 struct BASE_EXPORT PendingTask : public TrackingInfo {
-#if _MSC_VER >= 1700
-  PendingTask();
-#endif
   PendingTask(const tracked_objects::Location& posted_from,
               const Closure& task);
   PendingTask(const tracked_objects::Location& posted_from,
@@ -43,6 +40,9 @@ struct BASE_EXPORT PendingTask : public TrackingInfo {
 
   // OK to dispatch from a nested loop.
   bool nestable;
+
+  // Needs high resolution timers.
+  bool is_high_res;
 };
 
 // Wrapper around std::queue specialized for PendingTask which adds a Swap