Avoid spurious descheduling when posting message loop tasks. (flutter…

…#3812) Closes dart-lang/sdk#29971
agray86 · Jul 5, 2017 · 2062ca1 · 2062ca1
1 parent 8d5372a
commit 2062ca1
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 5 deletions.
diff --git a/fml/message_loop_impl.cc b/fml/message_loop_impl.cc
@@ -111,19 +111,33 @@ void MessageLoopImpl::RegisterTask(ftl::Closure task,
     // |task| synchronously within this function.
     return;
   }
-  ftl::MutexLocker lock(&delayed_tasks_mutex_);
-  delayed_tasks_.push({++order_, std::move(task), target_time});
-  WakeUp(delayed_tasks_.top().target_time);
+
+  ftl::TimePoint previous_wakeup, new_wakeup;
+  {
+    ftl::MutexLocker lock(&delayed_tasks_mutex_);
+    if (delayed_tasks_.empty()) {
+      previous_wakeup = ftl::TimePoint::Max();
+    } else {
+      previous_wakeup = delayed_tasks_.top().target_time;
+    }
+    delayed_tasks_.push({++order_, std::move(task), target_time});
+    new_wakeup = delayed_tasks_.top().target_time;
+  }
+  if (new_wakeup < previous_wakeup) {
+    WakeUp(new_wakeup);
+  }
 }
 
 void MessageLoopImpl::RunExpiredTasks() {
   TRACE_EVENT0("fml", "MessageLoop::RunExpiredTasks");
   std::vector<ftl::Closure> invocations;
 
+  ftl::TimePoint new_wakeup;
   {
     ftl::MutexLocker lock(&delayed_tasks_mutex_);
 
     if (delayed_tasks_.empty()) {
+      FTL_DCHECK(terminated_);  // No spurious wakeups except shutdown.
       return;
     }
 
@@ -137,9 +151,13 @@ void MessageLoopImpl::RunExpiredTasks() {
       delayed_tasks_.pop();
     }
 
-    WakeUp(delayed_tasks_.empty() ? ftl::TimePoint::Max()
-                                  : delayed_tasks_.top().target_time);
+    if (delayed_tasks_.empty()) {
+      new_wakeup = ftl::TimePoint::Max();
+    } else {
+      new_wakeup = delayed_tasks_.top().target_time;
+    }
   }
+  WakeUp(new_wakeup);
 
   for (const auto& invocation : invocations) {
     invocation();

diff --git a/fml/message_loop_unittests.cc b/fml/message_loop_unittests.cc
@@ -140,6 +140,34 @@ TEST(MessageLoop, CheckRunsTaskOnCurrentThread) {
   thread.join();
 }
 
+TEST(MessageLoop, TIME_SENSITIVE(NewTaskDueBeforePendingTask)) {
+  intptr_t tasks_run = 0;
+  std::thread thread([&tasks_run]() {
+    fml::MessageLoop::EnsureInitializedForCurrentThread();
+    auto& loop = fml::MessageLoop::GetCurrent();
+    auto begin = ftl::TimePoint::Now();
+    loop.GetTaskRunner()->PostDelayedTask(
+        [&tasks_run]() {
+          ASSERT_EQ(tasks_run, 1);
+          tasks_run++;
+          fml::MessageLoop::GetCurrent().Terminate();
+        },
+        ftl::TimeDelta::FromMilliseconds(15));
+    loop.GetTaskRunner()->PostDelayedTask(
+        [begin, &tasks_run]() {
+          ASSERT_EQ(tasks_run, 0);
+          tasks_run++;
+          auto delta = ftl::TimePoint::Now() - begin;
+          auto ms = delta.ToMillisecondsF();
+          ASSERT_LE(ms, 15);  // Did not wait for previous wakeup time.
+        },
+        ftl::TimeDelta::FromMilliseconds(5));
+    loop.Run();
+  });
+  thread.join();
+  ASSERT_EQ(tasks_run, 2);
+}
+
 TEST(MessageLoop, TIME_SENSITIVE(SingleDelayedTaskByDelta)) {
   bool checked = false;
   std::thread thread([&checked]() {

diff --git a/testing/run_tests.sh b/testing/run_tests.sh
@@ -3,6 +3,7 @@
 set -ex
 
 out/host_debug_unopt/ftl_unittests
+out/host_debug_unopt/fml_unittests
 out/host_debug_unopt/synchronization_unittests
 out/host_debug_unopt/wtf_unittests