[scheduler]: Avoid waking up the scheduler to end long idle periods.

components/scheduler/child/idle_helper.cc

Index: components/scheduler/child/idle_helper.cc
diff --git a/components/scheduler/child/idle_helper.cc b/components/scheduler/child/idle_helper.cc
index 54939d23c6c1804b327a7d90aabc9cfa7d116e6e..b5594e7991c2d6f5fb33c9f4fb891cc557f242fb 100644
--- a/components/scheduler/child/idle_helper.cc
+++ b/components/scheduler/child/idle_helper.cc
@@ -21,35 +21,31 @@ IdleHelper::IdleHelper(
     : helper_(helper),
       delegate_(delegate),
       idle_queue_index_(idle_queue_index),
-      idle_period_state_(IdlePeriodState::NOT_IN_IDLE_PERIOD),
+      state_(helper,
+             tracing_category,
+             disabled_by_default_tracing_category,
+             idle_period_tracing_name),
       quiescence_monitored_task_queue_mask_(
           helper_->GetQuiescenceMonitoredTaskQueueMask() &
           ~(1ull << idle_queue_index_)),
       required_quiescence_duration_before_long_idle_period_(
           required_quiescence_duration_before_long_idle_period),
-      tracing_category_(tracing_category),
       disabled_by_default_tracing_category_(
           disabled_by_default_tracing_category),
-      idle_period_tracing_name_(idle_period_tracing_name),
       weak_factory_(this) {
   weak_idle_helper_ptr_ = weak_factory_.GetWeakPtr();
-  end_idle_period_closure_.Reset(
-      base::Bind(&IdleHelper::EndIdlePeriod, weak_idle_helper_ptr_));
   enable_next_long_idle_period_closure_.Reset(
       base::Bind(&IdleHelper::EnableLongIdlePeriod, weak_idle_helper_ptr_));
-  enable_next_long_idle_period_after_wakeup_closure_.Reset(base::Bind(
-      &IdleHelper::EnableLongIdlePeriodAfterWakeup, weak_idle_helper_ptr_));
 
   idle_task_runner_ = make_scoped_refptr(new SingleThreadIdleTaskRunner(
       helper_->TaskRunnerForQueue(idle_queue_index_),
-      helper_->ControlAfterWakeUpTaskRunner(),
-      base::Bind(&IdleHelper::CurrentIdleTaskDeadlineCallback,
-                 weak_idle_helper_ptr_),
-      tracing_category));
+      helper_->ControlAfterWakeUpTaskRunner(), this, tracing_category));
 
   helper_->DisableQueue(idle_queue_index_);
   helper_->SetPumpPolicy(idle_queue_index_,
                          TaskQueueManager::PumpPolicy::MANUAL);
+
+  helper_->AddTaskObserver(this);

[Sami, 2015/05/29 09:38:06]

Please also remove this in the destructor.

[rmcilroy, 2015/06/01 15:33:22]

Done.

 }
 
 IdleHelper::~IdleHelper() {
@@ -66,12 +62,6 @@ scoped_refptr<SingleThreadIdleTaskRunner> IdleHelper::IdleTaskRunner() {
   return idle_task_runner_;
 }
 
-void IdleHelper::CurrentIdleTaskDeadlineCallback(
-    base::TimeTicks* deadline_out) const {
-  helper_->CheckOnValidThread();
-  *deadline_out = idle_period_deadline_;
-}
-
 IdleHelper::IdlePeriodState IdleHelper::ComputeNewLongIdlePeriodState(
     const base::TimeTicks now,
     base::TimeDelta* next_long_idle_period_delay_out) {
@@ -134,6 +124,8 @@ bool IdleHelper::ShouldWaitForQuiescence() {
 void IdleHelper::EnableLongIdlePeriod() {
   TRACE_EVENT0(disabled_by_default_tracing_category_, "EnableLongIdlePeriod");
   helper_->CheckOnValidThread();
+  if (helper_->IsShutdown())
+    return;
 
   // End any previous idle period.
   EndIdlePeriod();
@@ -152,98 +144,129 @@ void IdleHelper::EnableLongIdlePeriod() {
       ComputeNewLongIdlePeriodState(now, &next_long_idle_period_delay);
   if (IsInIdlePeriod(new_idle_period_state)) {
     StartIdlePeriod(new_idle_period_state, now,
-                    now + next_long_idle_period_delay, false);
-  }
-
-  if (helper_->IsQueueEmpty(idle_queue_index_)) {
-    // If there are no current idle tasks then post the call to initiate the
-    // next idle for execution after wakeup (at which point after-wakeup idle
-    // tasks might be eligible to run or more idle tasks posted).
-    helper_->ControlAfterWakeUpTaskRunner()->PostDelayedTask(
-        FROM_HERE,
-        enable_next_long_idle_period_after_wakeup_closure_.callback(),
-        next_long_idle_period_delay);
+                    now + next_long_idle_period_delay);
   } else {
-    // Otherwise post on the normal control task queue.
+    // Otherwise wait for the next long idle period delay before trying again.
     helper_->ControlTaskRunner()->PostDelayedTask(
         FROM_HERE, enable_next_long_idle_period_closure_.callback(),
         next_long_idle_period_delay);
   }
 }
 
-void IdleHelper::EnableLongIdlePeriodAfterWakeup() {
-  TRACE_EVENT0(disabled_by_default_tracing_category_,
-               "EnableLongIdlePeriodAfterWakeup");
-  helper_->CheckOnValidThread();
-
-  if (IsInIdlePeriod(idle_period_state_)) {
-    // Since we were asleep until now, end the async idle period trace event at
-    // the time when it would have ended were we awake.
-    TRACE_EVENT_ASYNC_END_WITH_TIMESTAMP0(
-        tracing_category_, idle_period_tracing_name_, this,
-        std::min(idle_period_deadline_, helper_->Now()).ToInternalValue());
-    idle_period_state_ = IdlePeriodState::ENDING_LONG_IDLE_PERIOD;
-    EndIdlePeriod();
-  }
-
-  // Post a task to initiate the next long idle period rather than calling it
-  // directly to allow all pending PostIdleTaskAfterWakeup tasks to get enqueued
-  // on the idle task queue before the next idle period starts so they are
-  // eligible to be run during the new idle period.
-  helper_->ControlTaskRunner()->PostTask(
-      FROM_HERE, enable_next_long_idle_period_closure_.callback());
-}
-
 void IdleHelper::StartIdlePeriod(IdlePeriodState new_state,
                                  base::TimeTicks now,
-                                 base::TimeTicks idle_period_deadline,
-                                 bool post_end_idle_period) {
+                                 base::TimeTicks idle_period_deadline) {
   DCHECK_GT(idle_period_deadline, now);
-  TRACE_EVENT_ASYNC_BEGIN0(tracing_category_, idle_period_tracing_name_, this);
   helper_->CheckOnValidThread();
   DCHECK(IsInIdlePeriod(new_state));
+  TRACE_EVENT0(disabled_by_default_tracing_category_, "StartIdlePeriod");
 
   helper_->EnableQueue(idle_queue_index_,
                        PrioritizingTaskQueueSelector::BEST_EFFORT_PRIORITY);
   helper_->PumpQueue(idle_queue_index_);
-  idle_period_state_ = new_state;
 
-  idle_period_deadline_ = idle_period_deadline;
-  if (post_end_idle_period) {
-    helper_->ControlTaskRunner()->PostDelayedTask(
-        FROM_HERE, end_idle_period_closure_.callback(),
-        idle_period_deadline_ - now);
-  }
+  state_.UpdateState(new_state, idle_period_deadline, now);
 }
 
 void IdleHelper::EndIdlePeriod() {
   helper_->CheckOnValidThread();
+  TRACE_EVENT0(disabled_by_default_tracing_category_, "EndIdlePeriod");
 
-  end_idle_period_closure_.Cancel();
   enable_next_long_idle_period_closure_.Cancel();
-  enable_next_long_idle_period_after_wakeup_closure_.Cancel();
 
   // If we weren't already within an idle period then early-out.
-  if (!IsInIdlePeriod(idle_period_state_))
+  if (!IsInIdlePeriod(state_.idle_period_state()))
     return;
 
-  // If we are in the ENDING_LONG_IDLE_PERIOD state we have already logged the
-  // trace event.
-  if (idle_period_state_ != IdlePeriodState::ENDING_LONG_IDLE_PERIOD) {
-    bool is_tracing;
-    TRACE_EVENT_CATEGORY_GROUP_ENABLED(tracing_category_, &is_tracing);
-    if (is_tracing && !idle_period_deadline_.is_null() &&
-        helper_->Now() > idle_period_deadline_) {
-      TRACE_EVENT_ASYNC_STEP_INTO_WITH_TIMESTAMP0(
-          tracing_category_, idle_period_tracing_name_, this, "DeadlineOverrun",
-          idle_period_deadline_.ToInternalValue());
+  helper_->DisableQueue(idle_queue_index_);
+  state_.UpdateState(IdlePeriodState::NOT_IN_IDLE_PERIOD, base::TimeTicks(),
+                     base::TimeTicks());
+}
+
+void IdleHelper::WillProcessTask(const base::PendingTask& pending_task) {
+}
+
+void IdleHelper::DidProcessTask(const base::PendingTask& pending_task) {
+  helper_->CheckOnValidThread();
+  TRACE_EVENT0(disabled_by_default_tracing_category_, "DidProcessTask");
+  if (IsInIdlePeriod(state_.idle_period_state()) &&
+      state_.idle_period_state() !=
+          IdlePeriodState::IN_LONG_IDLE_PERIOD_PAUSED &&
+      helper_->Now() >= state_.idle_period_deadline()) {
+    // If the idle period deadline has now been reached, either end the idle
+    // period or trigger a new long-idle period.
+    if (IsInLongIdlePeriod(state_.idle_period_state())) {
+      EnableLongIdlePeriod();
+    } else {
+      DCHECK(IdlePeriodState::IN_SHORT_IDLE_PERIOD ==
+             state_.idle_period_state());
+      EndIdlePeriod();
     }
-    TRACE_EVENT_ASYNC_END0(tracing_category_, idle_period_tracing_name_, this);
   }
+}
 
-  helper_->DisableQueue(idle_queue_index_);
-  idle_period_state_ = IdlePeriodState::NOT_IN_IDLE_PERIOD;
-  idle_period_deadline_ = base::TimeTicks();
+void IdleHelper::UpdateLongIdlePeriodStateAfterIdleTask() {
+  helper_->CheckOnValidThread();
+  DCHECK(IsInLongIdlePeriod(state_.idle_period_state()));
+  TRACE_EVENT0(disabled_by_default_tracing_category_,
+               "UpdateLongIdlePeriodStateAfterIdleTask");
+
+  TaskQueueManager::QueueState queue_state =
+      helper_->GetQueueState(idle_queue_index_);
+  if (queue_state == TaskQueueManager::QueueState::EMPTY) {
+    // If there are no more idle tasks then pause long idle period ticks until a
+    // new idle task is posted.
+    state_.UpdateState(IdlePeriodState::IN_LONG_IDLE_PERIOD_PAUSED,
+                       state_.idle_period_deadline(), base::TimeTicks());
+  } else if (queue_state == TaskQueueManager::QueueState::NEEDS_PUMPING) {
+    // If there is still idle work to do then just start the next idle period.
+    base::TimeDelta next_long_idle_period_delay;
+    if (state_.idle_period_state() ==
+        IdlePeriodState::IN_LONG_IDLE_PERIOD_WITH_MAX_DEADLINE) {
+      // If we are in a max deadline long idle period then start the next
+      // idle period immediately.
+      next_long_idle_period_delay = base::TimeDelta();
+    } else {
+      // Otherwise ensure that we kick the scheduler at the right time to
+      // initiate the next idle period.
+      next_long_idle_period_delay = std::max(
+          base::TimeDelta(), state_.idle_period_deadline() - helper_->Now());
+    }
+    helper_->ControlTaskRunner()->PostDelayedTask(
+        FROM_HERE, enable_next_long_idle_period_closure_.callback(),
+        next_long_idle_period_delay);
+  }
+}
+
+base::TimeTicks IdleHelper::CurrentIdleTaskDeadline() const {
+  helper_->CheckOnValidThread();
+  return state_.idle_period_deadline();
+}
+
+void IdleHelper::OnIdleTaskPosted() {
+  if (state_.idle_period_paused_flag()) {
+    // Restart long idle period ticks.
+    helper_->ControlTaskRunner()->PostTask(
+        FROM_HERE, enable_next_long_idle_period_closure_.callback());
+  }
+}
+
+base::TimeTicks IdleHelper::WillProcessIdleTask() const {
+  helper_->CheckOnValidThread();
+  DCHECK(IsInIdlePeriod(state_.idle_period_state()));
+
+  state_.WillProcessIdleTask();
+  return CurrentIdleTaskDeadline();
+}
+
+void IdleHelper::DidProcessIdleTask() {
+  helper_->CheckOnValidThread();
+  DCHECK(IsInIdlePeriod(state_.idle_period_state()));
+
+  state_.DidProcessIdleTask();
+  if (IsInLongIdlePeriod(state_.idle_period_state())) {
+    UpdateLongIdlePeriodStateAfterIdleTask();
+  }
 }
 
 // static
@@ -251,15 +274,152 @@ bool IdleHelper::IsInIdlePeriod(IdlePeriodState state) {
   return state != IdlePeriodState::NOT_IN_IDLE_PERIOD;
 }
 
+// static
+bool IdleHelper::IsInLongIdlePeriod(IdlePeriodState state) {
+  return state == IdlePeriodState::IN_LONG_IDLE_PERIOD ||
+         state == IdlePeriodState::IN_LONG_IDLE_PERIOD_WITH_MAX_DEADLINE ||
+         state == IdlePeriodState::IN_LONG_IDLE_PERIOD_PAUSED;
+}
+
 bool IdleHelper::CanExceedIdleDeadlineIfRequired() const {
-  TRACE_EVENT0(tracing_category_, "CanExceedIdleDeadlineIfRequired");
+  TRACE_EVENT0(disabled_by_default_tracing_category_,
+               "CanExceedIdleDeadlineIfRequired");
   helper_->CheckOnValidThread();
-  return idle_period_state_ ==
+  return state_.idle_period_state() ==
          IdlePeriodState::IN_LONG_IDLE_PERIOD_WITH_MAX_DEADLINE;
 }
 
 IdleHelper::IdlePeriodState IdleHelper::SchedulerIdlePeriodState() const {
-  return idle_period_state_;
+  return state_.idle_period_state();
+}
+
+IdleHelper::State::State(SchedulerHelper* helper,
+                         const char* tracing_category,
+                         const char* disabled_by_default_tracing_category,
+                         const char* idle_period_tracing_name)
+    : helper_(helper),
+      idle_period_state_(IdlePeriodState::NOT_IN_IDLE_PERIOD),
+      idle_period_paused_flag_(&write_lock_),
+      nestable_events_started_(false),
+      tracing_category_(tracing_category),
+      disabled_by_default_tracing_category_(
+          disabled_by_default_tracing_category),
+      idle_period_tracing_name_(idle_period_tracing_name) {
+}
+
+IdleHelper::State::~State() {
+}
+
+void IdleHelper::State::UpdateState(IdlePeriodState new_state,
+                                    base::TimeTicks new_deadline,
+                                    base::TimeTicks optional_now) {
+  helper_->CheckOnValidThread();
+  if (new_state == idle_period_state_) {
+    DCHECK(new_deadline == idle_period_deadline_);
+    return;
+  }
+
+  base::AutoLock lock(write_lock_);
+
+  bool is_tracing;
+  TRACE_EVENT_CATEGORY_GROUP_ENABLED(tracing_category_, &is_tracing);
+  if (is_tracing) {
+    base::TimeTicks now(optional_now.is_null() ? helper_->Now() : optional_now);
+    TraceEventIdlePeriodStateChange(new_state, new_deadline, now);
+    idle_period_deadline_for_tracing_ =
+        base::TimeTicks::NowFromSystemTraceTime() + (new_deadline - now);
+  }
+
+  idle_period_state_ = new_state;
+  idle_period_deadline_ = new_deadline;
+  idle_period_paused_flag_.SetWhileLocked(
+      new_state == IdlePeriodState::IN_LONG_IDLE_PERIOD_PAUSED);
+}
+
+void IdleHelper::State::WillProcessIdleTask() const {

[Sami, 2015/05/29 09:38:06]

Should we call these something like TraceIdleTaskStart/End() since they're not
really doing anything else as far as I can tell? Generally it feels like the
state should be dumb without any complex logic in it and naming the functions
accordingly could help to keep it that way.

[rmcilroy, 2015/06/01 15:33:22]

SGTM, done.

+  helper_->CheckOnValidThread();
+
+  bool is_tracing;
+  TRACE_EVENT_CATEGORY_GROUP_ENABLED(tracing_category_, &is_tracing);
+  if (is_tracing && nestable_events_started_) {
+    last_traced_start_running_idle_task_ =
+        base::TimeTicks::NowFromSystemTraceTime();
+    TRACE_EVENT_NESTABLE_ASYNC_BEGIN_WITH_TIMESTAMP0(
+        tracing_category_, "RunningIdleTask", this,
+        last_traced_start_running_idle_task_.ToInternalValue());
+  }
+}
+
+void IdleHelper::State::DidProcessIdleTask() const {
+  helper_->CheckOnValidThread();
+
+  bool is_tracing;
+  TRACE_EVENT_CATEGORY_GROUP_ENABLED(tracing_category_, &is_tracing);
+  if (is_tracing && nestable_events_started_) {
+    if (!idle_period_deadline_for_tracing_.is_null() &&
+        base::TimeTicks::NowFromSystemTraceTime() >
+            idle_period_deadline_for_tracing_) {
+      TRACE_EVENT_NESTABLE_ASYNC_BEGIN_WITH_TIMESTAMP0(
+          tracing_category_, "DeadlineOverrun", this,
+          std::max(idle_period_deadline_for_tracing_,
+                   last_traced_start_running_idle_task_).ToInternalValue());
+      TRACE_EVENT_NESTABLE_ASYNC_END0(tracing_category_, "DeadlineOverrun",
+                                      this);
+    }
+    TRACE_EVENT_NESTABLE_ASYNC_END0(tracing_category_, "RunningIdleTask", this);
+  }
+}
+
+void IdleHelper::State::TraceEventIdlePeriodStateChange(
+    IdlePeriodState new_state,
+    base::TimeTicks new_deadline,
+    base::TimeTicks now) const {
+  TRACE_EVENT2(disabled_by_default_tracing_category_, "SetIdlePeriodState",
+               "old_state",
+               IdleHelper::IdlePeriodStateToString(idle_period_state_),
+               "new_state", IdleHelper::IdlePeriodStateToString(new_state));
+  if (nestable_events_started_) {
+    // End async tracing events for the state we are leaving.
+    if (idle_period_state_ == IdlePeriodState::IN_LONG_IDLE_PERIOD_PAUSED) {
+      TRACE_EVENT_NESTABLE_ASYNC_END0(tracing_category_, "LongIdlePeriodPaused",
+                                      this);
+    }
+    if (IsInLongIdlePeriod(idle_period_state_) &&
+        !IsInLongIdlePeriod(new_state)) {
+      TRACE_EVENT_NESTABLE_ASYNC_END0(tracing_category_, "LongIdlePeriod",
+                                      this);
+    }
+    if (idle_period_state_ == IdlePeriodState::IN_SHORT_IDLE_PERIOD) {
+      TRACE_EVENT_NESTABLE_ASYNC_END0(tracing_category_, "ShortIdlePeriod",
+                                      this);
+    }
+    if (IsInIdlePeriod(idle_period_state_) && !IsInIdlePeriod(new_state)) {
+      TRACE_EVENT_NESTABLE_ASYNC_END0(tracing_category_,
+                                      idle_period_tracing_name_, this);
+      nestable_events_started_ = false;
+    }
+  }
+
+  // Start async tracing events for the state we are entering.
+  if (IsInIdlePeriod(new_state) && !IsInIdlePeriod(idle_period_state_)) {
+    nestable_events_started_ = true;
+    TRACE_EVENT_NESTABLE_ASYNC_BEGIN1(
+        tracing_category_, idle_period_tracing_name_, this,
+        "idle_period_length_ms", (new_deadline - now).ToInternalValue());
+  }
+  if (new_state == IdlePeriodState::IN_SHORT_IDLE_PERIOD) {
+    TRACE_EVENT_NESTABLE_ASYNC_BEGIN0(tracing_category_, "ShortIdlePeriod",
+                                      this);
+  }
+  if (IsInLongIdlePeriod(new_state) &&
+      !IsInLongIdlePeriod(idle_period_state_)) {
+    TRACE_EVENT_NESTABLE_ASYNC_BEGIN0(tracing_category_, "LongIdlePeriod",
+                                      this);
+  }
+  if (new_state == IdlePeriodState::IN_LONG_IDLE_PERIOD_PAUSED) {
+    TRACE_EVENT_NESTABLE_ASYNC_BEGIN0(tracing_category_, "LongIdlePeriodPaused",
+                                      this);
+  }
 }
 
 // static
@@ -274,8 +434,8 @@ const char* IdleHelper::IdlePeriodStateToString(
       return "in_long_idle_period";
     case IdlePeriodState::IN_LONG_IDLE_PERIOD_WITH_MAX_DEADLINE:
       return "in_long_idle_period_with_max_deadline";
-    case IdlePeriodState::ENDING_LONG_IDLE_PERIOD:
-      return "ending_long_idle_period";
+    case IdlePeriodState::IN_LONG_IDLE_PERIOD_PAUSED:
+      return "in_long_idle_period_paused";
     default:
       NOTREACHED();
       return nullptr;