cc: Heuristic for Renderer latency recovery

cc/scheduler/scheduler.cc

Index: cc/scheduler/scheduler.cc
diff --git a/cc/scheduler/scheduler.cc b/cc/scheduler/scheduler.cc
index d2520335c0cd77a605b651ec633c04462f65828c..6807493428c88bfcae9d6d46b8743199d4ca3537 100644
--- a/cc/scheduler/scheduler.cc
+++ b/cc/scheduler/scheduler.cc
@@ -129,8 +129,18 @@ base::TimeTicks Scheduler::Now() const {
   return now;
 }
 
+void Scheduler::SetAuthoritativeVSyncInterval(const base::TimeDelta& interval) {
+  authoritative_vsync_interval_ = interval;
+  if (vsync_observer_)
+    vsync_observer_->OnUpdateVSyncParameters(last_vsync_timebase_, interval);
+}
+
 void Scheduler::CommitVSyncParameters(base::TimeTicks timebase,
                                       base::TimeDelta interval) {
+  TRACE_EVENT2("cc", "Scheduler::CommitVSyncParameters", "timebase",
+               (timebase - base::TimeTicks()).InSecondsF(), "interval",
+               interval.InSecondsF());
+
   if (authoritative_vsync_interval_ != base::TimeDelta()) {
     interval = authoritative_vsync_interval_;
   } else if (interval == base::TimeDelta()) {
@@ -387,12 +397,6 @@ void Scheduler::SetChildrenNeedBeginFrames(bool children_need_begin_frames) {
   ProcessScheduledActions();
 }
 
-void Scheduler::SetAuthoritativeVSyncInterval(const base::TimeDelta& interval) {
-  authoritative_vsync_interval_ = interval;
-  if (vsync_observer_)
-    vsync_observer_->OnUpdateVSyncParameters(last_vsync_timebase_, interval);
-}
-
 void Scheduler::SetVideoNeedsBeginFrames(bool video_needs_begin_frames) {
   state_machine_.SetVideoNeedsBeginFrames(video_needs_begin_frames);
   ProcessScheduledActions();
@@ -498,13 +502,24 @@ void Scheduler::BeginImplFrameWithDeadline(const BeginFrameArgs& args) {
   begin_impl_frame_args_ = args;
   begin_impl_frame_args_.deadline -= client_->DrawDurationEstimate();
 
-  if (!state_machine_.impl_latency_takes_priority() &&
-      main_thread_is_in_high_latency_mode &&
-      CanCommitAndActivateBeforeDeadline()) {
+  BeginImplFrame();
+  // Decide if we want skip any actions before we ProcessScheduledActions
+  // so that they take effect immediately rather than with a 1 frame delay.
+  // Prioritize skipping the main thread's latency first so we can keep the
+  // main and impl threads synchronized. Then skip an impl frame if it makes
+  // sense. This avoids skipping both at the same time to avoid strange
+  // interactions/oscillations since ShouldRecoverImplLatency() depends on
+  // whether the main thread is in a high latency mode or not.
+  if (ShouldRecoverMainLatency()) {
+    TRACE_EVENT_INSTANT1("cc", "SkipBeginMainFrameToReduceLatency",
+                         TRACE_EVENT_SCOPE_THREAD, "state", AsValue());
     state_machine_.SetSkipNextBeginMainFrameToReduceLatency();
+  } else if (ShouldRecoverImplLatency()) {
+    TRACE_EVENT_INSTANT1("cc", "SkipNextSwapToReduceLatency",
+                         TRACE_EVENT_SCOPE_THREAD, "state", AsValue());
+    state_machine_.SetSkipNextSwapToReduceLatency();

[sunnyps, 2015/05/21 23:42:23]

Can we call FinishImplFrame here instead of going through the BeginImplFrame
cycle? That will allows us to get rid of the state tracking code in the state
machine.

[brianderson, 2015/05/22 01:12:53]

That's a good idea. I'll give it a try!

   }
-
-  BeginImplFrame();
+  ProcessScheduledActions();

[sunnyps, 2015/05/21 23:42:23]

Any particular reason for splitting ProcessScheduledActions out of
BeginImplFrame?

[brianderson, 2015/05/22 01:12:53]

SchedulerStateMachine::OnBeginImplFrame resets the skip_next_... variables. Let
me see if there's another way of doing this - it may not be needed if calling
FinishImplFrame works.

 
   // The deadline will be scheduled in ProcessScheduledActions.
   state_machine_.OnBeginImplFrameDeadlinePending();
@@ -516,6 +531,8 @@ void Scheduler::BeginImplFrameSynchronous(const BeginFrameArgs& args) {
                args.AsValue());
   begin_impl_frame_args_ = args;
   BeginImplFrame();
+  ProcessScheduledActions();
+
   FinishImplFrame();
 }
 
@@ -540,8 +557,6 @@ void Scheduler::BeginImplFrame() {
   state_machine_.OnBeginImplFrame();
   devtools_instrumentation::DidBeginFrame(layer_tree_host_id_);
   client_->WillBeginImplFrame(begin_impl_frame_args_);
-
-  ProcessScheduledActions();
 }
 
 void Scheduler::ScheduleBeginImplFrameDeadline() {
@@ -553,7 +568,6 @@ void Scheduler::ScheduleBeginImplFrameDeadline() {
 
   begin_impl_frame_deadline_mode_ =
       state_machine_.CurrentBeginImplFrameDeadlineMode();
-
   base::TimeTicks deadline;
   switch (begin_impl_frame_deadline_mode_) {
     case SchedulerStateMachine::BEGIN_IMPL_FRAME_DEADLINE_MODE_NONE:
@@ -771,6 +785,9 @@ void Scheduler::AsValueInto(base::trace_event::TracedValue* state) const {
   state->BeginDictionary("begin_impl_frame_args");
   begin_impl_frame_args_.AsValueInto(state);
   state->EndDictionary();
+  state->SetString("begin_impl_frame_deadline_mode_",
+                   SchedulerStateMachine::BeginImplFrameDeadlineModeToString(
+                       begin_impl_frame_deadline_mode_));
 
   base::TimeTicks now = Now();
   base::TimeTicks frame_time = begin_impl_frame_args_.frame_time;
@@ -803,6 +820,46 @@ void Scheduler::AsValueInto(base::trace_event::TracedValue* state) const {
   state->EndDictionary();
 }
 
+bool Scheduler::ShouldRecoverMainLatency() const {
+  TRACE_EVENT0("cc", "ShouldRecoverMainLatency");
+  if (!state_machine_.MainThreadIsInHighLatencyMode())
+    return false;
+
+  if (state_machine_.impl_latency_takes_priority())
+    return false;
+
+  return CanCommitAndActivateBeforeDeadline();
+}
+
+bool Scheduler::ShouldRecoverImplLatency() const {
+  TRACE_EVENT1("cc", "ShouldRecoverImplLatency",
+               "swaps_are_likely_high_latency",
+               state_machine_.swaps_are_likely_high_latency());
+  if (!state_machine_.swaps_are_likely_high_latency())

[sunnyps, 2015/05/21 23:42:22]

When is swaps_are_likely_high_latency != pending_swaps >= max_pending_swaps? 

[brianderson, 2015/05/22 01:12:53]

When the swap ack for a BeginImplFrame(1) comes back after the deadline of the
next BeginImplFrame(2).

In that case though, we wouldn't have done anything in the BeginImplFrame(2) and
shouldn't need to skip the next swap in BeginImplFrame(3).

I'll try to simplify the logic.

+    return false;
+
+  bool frame_time_is_before_deadline =

[sunnyps, 2015/05/21 23:42:23]

Maybe we shouldn't adjust the deadline in begin_impl_frame_args_? So that we can
compare DrawDurationEstimate with the real deadline here? It's a little unclear
what's happening here because the deadline already has DrawDurationEstimate
adjusted.

[brianderson, 2015/05/22 01:12:53]

Are you saying the logic would be easier to follow that way? Or is there also
something wrong with this comparison?

I like how the deadline is adjusted by DrawDurationEstimate for tracing purposes
- it's easy to see how much time is left before the deadline.

+      begin_impl_frame_args_.frame_time < begin_impl_frame_args_.deadline;
+
+  auto next_begin_impl_frame_deadline_mode =

[sunnyps, 2015/05/21 23:42:23]

nit: Let's not use auto here.

[brianderson, 2015/05/22 01:12:53]

Done.

+      state_machine_.CurrentBeginImplFrameDeadlineMode();
+  switch (next_begin_impl_frame_deadline_mode) {
+    case SchedulerStateMachine::BEGIN_IMPL_FRAME_DEADLINE_MODE_NONE:
+      return false;
+    case SchedulerStateMachine::BEGIN_IMPL_FRAME_DEADLINE_MODE_IMMEDIATE:
+      return frame_time_is_before_deadline;
+    case SchedulerStateMachine::BEGIN_IMPL_FRAME_DEADLINE_MODE_REGULAR:
+      return frame_time_is_before_deadline;
+    case SchedulerStateMachine::BEGIN_IMPL_FRAME_DEADLINE_MODE_LATE:
+      return CanCommitAndActivateBeforeDeadline();
+    case SchedulerStateMachine::
+        BEGIN_IMPL_FRAME_DEADLINE_MODE_BLOCKED_ON_READY_TO_DRAW:
+      return false;
+  }
+  NOTREACHED();
+  return false;
+}
+
 bool Scheduler::CanCommitAndActivateBeforeDeadline() const {
   // Check if the main thread computation and commit can be finished before the
   // impl thread's deadline.
@@ -811,14 +868,6 @@ bool Scheduler::CanCommitAndActivateBeforeDeadline() const {
       client_->BeginMainFrameToCommitDurationEstimate() +
       client_->CommitToActivateDurationEstimate();
 
-  TRACE_EVENT2(
-      TRACE_DISABLED_BY_DEFAULT("cc.debug.scheduler"),
-      "CanCommitAndActivateBeforeDeadline",
-      "time_left_after_drawing_ms",
-      (begin_impl_frame_args_.deadline - estimated_draw_time).InMillisecondsF(),
-      "state",
-      AsValue());
-
   return estimated_draw_time < begin_impl_frame_args_.deadline;
 }