Currently if there is an impl-side invalidation request, the only point where we create a pending tree is inside the BeginImplFrameDeadline. This is too late in the pipeline since inside the deadline, we also do a draw for this frame. If an impl-side invalidation is being used to animate content, each animation update will end up taking 2 frames, the first where the pending tree is created and the second where it is actually drawn. This will end up being true *even* for the most common cases where there is no main frame request or the commit was aborted (which triggers the impl frame deadline immediately).

I chatted with Brian a bit about this on how to handle the different cases:

1) If there is no main frame request or the commit was aborted, we can create the pending tree right away and not trigger the impl frame deadline.

2) For the case where a response from the main thread is pending, we can take the activation duration estimate and set a deadline for creating the pending tree if the main thread doesn't respond before draw_deadline - activation_time?

This change [1] should handle the common case where the main thread is idle and the compositor thread is animating. It makes sure we perform the invalidation at the beginning of the impl frame if there is no main frame or the last one was aborted (accounting for the scrolling case).

I have still seen cases where the main thread would be busy for hundreds of ms and we would end up scheduling an invalidation only at draw deadline. The solution for this would be to either have an earlier deadline (suggested in #1) or may be the simpler thing might be to just perform the invalidation at the beginning of the frame if we can predict the main thread wouldn't respond before the draw deadline? Using estimates from CompositorTimingHistory.

Marking this available since I wouldn't be able to come back to this soon.

[1]: https://chromium-review.googlesource.com/c/chromium/src/+/736023

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c3b3a5fd6326109faff86d3f6750629f0369541d

commit c3b3a5fd6326109faff86d3f6750629f0369541d
Author: Khushal <khushalsagar@chromium.org>
Date: Fri Oct 27 23:40:10 2017

cc: Perform impl-side invalidations at the beginning of impl frame.

Currently an impl-side invalidation request is deferred until the
deadline to accomodate cases where the main thread might request a
frame later in the impl frame timeline. This is not true in most cases
and is harmful for animations using impl-side invalidation in the most
common cases, where the main thread is inactive. This is because
defering the invalidation until the deadline implies that the raster
work for this invalidation will not be finished within the deadline
and the update will miss this frame's draw.

This change avoids the case above by modifying this pipelining logic
to perform the invalidation at the beginning of the impl frame, if there
is no main frame request and the last main frame was aborted.

R=brianderson@chromium.org, sunnyps@chromium.org, vmiura@chromium.org

Bug:  691735 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I28c6c84856b8914dd5e9379cb5742086461ad7e3
Reviewed-on: https://chromium-review.googlesource.com/736023
Commit-Queue: Khushal <khushalsagar@chromium.org>
Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512351}
[modify] https://crrev.com/c3b3a5fd6326109faff86d3f6750629f0369541d/cc/scheduler/scheduler_state_machine.cc
[modify] https://crrev.com/c3b3a5fd6326109faff86d3f6750629f0369541d/cc/scheduler/scheduler_state_machine.h
[modify] https://crrev.com/c3b3a5fd6326109faff86d3f6750629f0369541d/cc/scheduler/scheduler_state_machine_unittest.cc
[modify] https://crrev.com/c3b3a5fd6326109faff86d3f6750629f0369541d/cc/scheduler/scheduler_unittest.cc

I'm putting this back on my plate. The compositor image animation study showed an increase in pending tree duration above the 95th percentile. Drilling down on the subcomponents of what is captured by this metric, there is an increase in ReadyToActivateToActivationDuration. Here are the graphs:

https://uma.googleplex.com/p/chrome/variations/?sid=63dcb99f62dc425d3d03c1f3156a927a and https://uma.googleplex.com/p/chrome/variations/?sid=f72c8b2eed787eb87dc2289718bd7f29.

This is definitely a result of scheduling deferring the invalidation until the draw deadline. It means you miss that deadline and activate after it which blocks the pipeline.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ee432e360c202a47eaf132b33d7c88155f52b6d6

commit ee432e360c202a47eaf132b33d7c88155f52b6d6
Author: Khushal <khushalsagar@chromium.org>
Date: Thu Dec 07 22:46:18 2017

cc: Alter duration used in prioritizing impl latency in Scheduler.

Currently the Scheduler uses an estimate of BeginMainFrameStartToCommit
duration to prioritize impl thread draws over main frames during
smoothness mode. This duration also includes the time the main thread
is blocked from committing as a result of a previous update not being
flushed through the pipeline.

Since prioritizing an impl thread draw will cause the pending tree to
miss the frame's draw and as a result block further updates from the
main thread, including this duration in our decision making can cause
a bad feedback loop where prioritizing the impl thread causes the main
thread to be assumed to be slow and it is perpetually de-prioritized.

This change ensures that we only use the duration from
BeginMainFrameToReadyToCommit in the estimate above. The UMA is still
recorded including this duration. We should consider breaking this in
the UMA recordings as well, so there is a clear understanding of what
component of the duration can be attributed to main thread work vs
back pressure in the pipeline.

R=brianderson@chromium.org, vmpstr@chromium.org

Bug:  691735 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: Ic6ea77f132e8e2a1ab07b85d223120cc71e5e069
Reviewed-on: https://chromium-review.googlesource.com/813074
Reviewed-by: Brian Anderson <brianderson@chromium.org>
Commit-Queue: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522582}
[modify] https://crrev.com/ee432e360c202a47eaf132b33d7c88155f52b6d6/cc/scheduler/compositor_timing_history.cc
[modify] https://crrev.com/ee432e360c202a47eaf132b33d7c88155f52b6d6/cc/scheduler/compositor_timing_history.h
[modify] https://crrev.com/ee432e360c202a47eaf132b33d7c88155f52b6d6/cc/scheduler/compositor_timing_history_unittest.cc
[modify] https://crrev.com/ee432e360c202a47eaf132b33d7c88155f52b6d6/cc/scheduler/scheduler.cc
[modify] https://crrev.com/ee432e360c202a47eaf132b33d7c88155f52b6d6/cc/scheduler/scheduler_state_machine.h
[modify] https://crrev.com/ee432e360c202a47eaf132b33d7c88155f52b6d6/cc/scheduler/scheduler_unittest.cc
[modify] https://crrev.com/ee432e360c202a47eaf132b33d7c88155f52b6d6/cc/test/scheduler_test_common.cc
[modify] https://crrev.com/ee432e360c202a47eaf132b33d7c88155f52b6d6/cc/test/scheduler_test_common.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/17b77d94eb6f27e6cac263969fe9999522f2ade2

commit 17b77d94eb6f27e6cac263969fe9999522f2ade2
Author: Khushal <khushalsagar@chromium.org>
Date: Mon Dec 11 19:54:02 2017

cc: Change heuristics for prioritizing impl-side invalidations.

Currently we defer the impl-side invalidation until the draw deadline
in order to merge them with the incoming main frame. But delaying until
the deadline implies that this invalidation's raster will miss this
frame's draw and result in a pending tree that has to wait until the
next impl frame, which will block the pipeline.

Instead use data from CompositorTimingHistory to decide whether
invalidations should be prioritized over the main frame at the
beginning of the impl frame.

R=brianderson@chromium.org

Bug:  691735 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I7764cb2934aa2d3bf2d72534e128391341f75d10
Reviewed-on: https://chromium-review.googlesource.com/802414
Commit-Queue: Khushal <khushalsagar@chromium.org>
Reviewed-by: Brian Anderson <brianderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523179}
[modify] https://crrev.com/17b77d94eb6f27e6cac263969fe9999522f2ade2/cc/base/rolling_time_delta_history.cc
[modify] https://crrev.com/17b77d94eb6f27e6cac263969fe9999522f2ade2/cc/base/rolling_time_delta_history.h
[modify] https://crrev.com/17b77d94eb6f27e6cac263969fe9999522f2ade2/cc/scheduler/compositor_timing_history.h
[modify] https://crrev.com/17b77d94eb6f27e6cac263969fe9999522f2ade2/cc/scheduler/scheduler.cc
[modify] https://crrev.com/17b77d94eb6f27e6cac263969fe9999522f2ade2/cc/scheduler/scheduler_state_machine.cc
[modify] https://crrev.com/17b77d94eb6f27e6cac263969fe9999522f2ade2/cc/scheduler/scheduler_state_machine.h
[modify] https://crrev.com/17b77d94eb6f27e6cac263969fe9999522f2ade2/cc/scheduler/scheduler_state_machine_unittest.cc
[modify] https://crrev.com/17b77d94eb6f27e6cac263969fe9999522f2ade2/cc/scheduler/scheduler_unittest.cc

Looking at the latest UMA on Scheduling.Renderer.ReadyToActivateToActivationDuration2.Main/Impl, the change above reduced the value slightly at the 95th percentile (comparing [1] and [2]). There isn't any follow up work left now. Even with the best heuristics, there is bound to be some regression because we'll have cases where our predictions are not accurate.

[1]: https://uma.googleplex.com/p/chrome/variations/?sid=73e4476560e7de5009474c73c62e1d00
[2]: https://uma.googleplex.com/p/chrome/variations/?sid=0a4c967aaacebeac48583d146714786a