cc: Add MainAndImplFrameTimeDelta UMA.

cc: Add MainAndImplFrameTimeDelta UMA.

Records how often the main and impl threads are synchronized
when the compositor draws with a new active tree.

Some DCHECKS are disabled for the synchronous compositor
because draws can come in at any time or not at all.

BUG=582749
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/bb917ddb01a8c3e373d6df02046c69ff00b07254
Cr-Commit-Position: refs/heads/master@{#376628}

[brianderson, 2016-02-10 02:12:29]

Description was changed from

==========
cc: Add MainAndImplSynchronized UMA.

Records how often the main and impl threads are synchronized
when the compositor draws with a new active tree.

BUG=582749
==========

to

==========
cc: Add MainAndImplSynchronized UMA.

Records how often the main and impl threads are synchronized
when the compositor draws with a new active tree.

BUG=582749
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[brianderson, 2016-02-10 02:14:10]

brianderson@chromium.org changed reviewers:
+ isherman@chromium.org, skyostil@chromium.org, sunnyps@chromium.org,
tdresser@chromium.org

[brianderson, 2016-02-10 02:14:10]

ptal!

[tdresser, 2016-02-10 17:15:20]

cc vollick.

LGTM, though reporting the number of deadlines the main thread missed might be
more useful.

https://codereview.chromium.org/1681393003/diff/1/cc/scheduler/compositor_tim...
File cc/scheduler/compositor_timing_history.h (right):

https://codereview.chromium.org/1681393003/diff/1/cc/scheduler/compositor_tim...
cc/scheduler/compositor_timing_history.h:68: bool
main_thread_missed_last_deadline);
Would it make sense to record how many deadlines the main thread missed?

[tdresser, 2016-02-10 17:15:36]

Description was changed from

==========
cc: Add MainAndImplSynchronized UMA.

Records how often the main and impl threads are synchronized
when the compositor draws with a new active tree.

BUG=582749
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc: Add MainAndImplSynchronized UMA.

Records how often the main and impl threads are synchronized
when the compositor draws with a new active tree.

BUG=582749
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[Sami, 2016-02-10 17:28:54]

lgtm.

https://codereview.chromium.org/1681393003/diff/1/cc/scheduler/compositor_tim...
File cc/scheduler/compositor_timing_history.h (right):

https://codereview.chromium.org/1681393003/diff/1/cc/scheduler/compositor_tim...
cc/scheduler/compositor_timing_history.h:68: bool
main_thread_missed_last_deadline);
On 2016/02/10 17:15:20, tdresser wrote:
> Would it make sense to record how many deadlines the main thread missed?

Do you mean the lateness in milliseconds? I guess with this patch an overrun of
1ms and 100ms would be in the same bucket. I feel like those numbers might be
derivable from the BeginMainFrame timing UMAs, but I don't have proof :)

[tdresser, 2016-02-10 18:02:22]

On 2016/02/10 17:28:54, Sami wrote:
> lgtm.
> 
>
https://codereview.chromium.org/1681393003/diff/1/cc/scheduler/compositor_tim...
> File cc/scheduler/compositor_timing_history.h (right):
> 
>
https://codereview.chromium.org/1681393003/diff/1/cc/scheduler/compositor_tim...
> cc/scheduler/compositor_timing_history.h:68: bool
> main_thread_missed_last_deadline);
> On 2016/02/10 17:15:20, tdresser wrote:
> > Would it make sense to record how many deadlines the main thread missed?
> 
> Do you mean the lateness in milliseconds? I guess with this patch an overrun
of
> 1ms and 100ms would be in the same bucket. I feel like those numbers might be
> derivable from the BeginMainFrame timing UMAs, but I don't have proof :)

I was thinking lateness in terms of the number of compositor thread deadlines it
missed, though I'm not positive that makes sense.

[Ilya Sherman, 2016-02-10 20:10:58]

LGTM % the below comment:

https://codereview.chromium.org/1681393003/diff/1/tools/metrics/histograms/hi...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1681393003/diff/1/tools/metrics/histograms/hi...
tools/metrics/histograms/histograms.xml:43015: +<histogram
name="Scheduling.MainAndImplSynchronized">
Please associate a custom enum (probably BooleanSynchronized) with this
histogram.

[brianderson, 2016-02-10 23:04:55]

On 2016/02/10 17:28:54, Sami wrote:
> lgtm.
> 
>
https://codereview.chromium.org/1681393003/diff/1/cc/scheduler/compositor_tim...
> File cc/scheduler/compositor_timing_history.h (right):
> 
>
https://codereview.chromium.org/1681393003/diff/1/cc/scheduler/compositor_tim...
> cc/scheduler/compositor_timing_history.h:68: bool
> main_thread_missed_last_deadline);
> On 2016/02/10 17:15:20, tdresser wrote:
> > Would it make sense to record how many deadlines the main thread missed?
> 
> Do you mean the lateness in milliseconds? I guess with this patch an overrun
of
> 1ms and 100ms would be in the same bucket. I feel like those numbers might be
> derivable from the BeginMainFrame timing UMAs, but I don't have proof :)

I can record:
1) Delta between frame times of the main frame and impl frames.
2) How late after the missed draw activation actually occurred.

1 is the user-visible delta, which is what matters.
2 is how badly the main thread missed, which can help show us how much more
often we *could* have been synchronized if the impl thread only waited a little
longer.

[brianderson, 2016-02-11 01:43:45]

https://codereview.chromium.org/1681393003/diff/40001/cc/scheduler/compositor...
File cc/scheduler/compositor_timing_history.cc (right):

https://codereview.chromium.org/1681393003/diff/40001/cc/scheduler/compositor...
cc/scheduler/compositor_timing_history.cc:719:
DCHECK_EQ(main_thread_missed_last_deadline, !main_and_impl_delta.is_zero());
Good thing we tried recording the delta. I added this DCHECK to sanity check the
delta approach v.s the bool approach and I noticed a discrepancy where it was
triggering every once and a while.

Turns out main_thread_missed_last_deadline stays true after aborting a
BeginMainFrame, which causes:
1) The scheduler to needlessly skip the next BeginMainFrame in some cases.
2) This DCHECK to trigger if the next BeginMainFrame isn't aborted (because of
impl latency takes priority or long estimates).

I fixed that bug and added a unit test as part of this patch, but can split it
out.

Also: still need to fix some other issues with the WebView, since it breaks some
assumptions in CompositorTimeingHistory.

[tdresser, 2016-02-11 14:11:49]

Metrics part of things LGTM.

[brianderson, 2016-02-19 01:27:27]

Description was changed from

==========
cc: Add MainAndImplSynchronized UMA.

Records how often the main and impl threads are synchronized
when the compositor draws with a new active tree.

BUG=582749
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc: Add MainAndImplFrameTimeDelta UMA.

Records how often the main and impl threads are synchronized
when the compositor draws with a new active tree.

Some DCHECKS are disabled for the synchronous compositor
because draws can come in at any time or not at all.

BUG=582749
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[brianderson, 2016-02-19 01:52:17]

Patch changed significantly. Sunny, can you take a look before I land?

https://codereview.chromium.org/1681393003/diff/60001/cc/scheduler/scheduler_...
File cc/scheduler/scheduler_state_machine.cc (right):

https://codereview.chromium.org/1681393003/diff/60001/cc/scheduler/scheduler_...
cc/scheduler/scheduler_state_machine.cc:650: main_thread_missed_last_deadline_ =
CommitPending() || has_pending_tree_;
Need to add a unit test for this, but probably won't split it out to a separate
patch this time.

I think this could cause us to unintentionally recover main latency when
can_draw_ goes from false to true.

Without this line, the HudWithRootLayerChange was failing the following DCHECK
in CompositorTimingHistory:

DCHECK_EQ(main_thread_missed_last_deadline,
                !main_and_impl_delta.is_zero());

[brianderson, 2016-02-19 02:18:32]

The CQ bit was checked by brianderson@chromium.org to run a CQ dry run

[brianderson, 2016-02-19 02:19:45]

Added unit test that fails without the changes to the state machine regarding
aborted draws when can_draw_ is false.

This likely affects resize performance, though I haven't tested.

[commit-bot: I haz the power, 2016-02-19 02:20:41]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1681393003/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1681393003/80001

[commit-bot: I haz the power, 2016-02-19 05:07:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-19 05:07:44]

Dry run: This issue passed the CQ dry run.

[brianderson, 2016-02-19 23:36:52]

Had time, so split out the CanDraw change + test in
https://codereview.chromium.org/1715863002/ and rebased on that.

[brianderson, 2016-02-20 04:10:45]

The CQ bit was checked by brianderson@chromium.org

[brianderson, 2016-02-20 04:10:46]

The patchset sent to the CQ was uploaded after l-g-t-m from
isherman@chromium.org, skyostil@chromium.org, tdresser@chromium.org
Link to the patchset: https://codereview.chromium.org/1681393003/#ps120001
(title: "- active_tree_needs_first_draw_ = true")

[commit-bot: I haz the power, 2016-02-20 04:11:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1681393003/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1681393003/120001

[commit-bot: I haz the power, 2016-02-20 05:21:25]

Description was changed from

==========
cc: Add MainAndImplFrameTimeDelta UMA.

Records how often the main and impl threads are synchronized
when the compositor draws with a new active tree.

Some DCHECKS are disabled for the synchronous compositor
because draws can come in at any time or not at all.

BUG=582749
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc: Add MainAndImplFrameTimeDelta UMA.

Records how often the main and impl threads are synchronized
when the compositor draws with a new active tree.

Some DCHECKS are disabled for the synchronous compositor
because draws can come in at any time or not at all.

BUG=582749
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[commit-bot: I haz the power, 2016-02-20 05:21:26]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-02-20 05:22:25]

Description was changed from

==========
cc: Add MainAndImplFrameTimeDelta UMA.

Records how often the main and impl threads are synchronized
when the compositor draws with a new active tree.

Some DCHECKS are disabled for the synchronous compositor
because draws can come in at any time or not at all.

BUG=582749
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc: Add MainAndImplFrameTimeDelta UMA.

Records how often the main and impl threads are synchronized
when the compositor draws with a new active tree.

Some DCHECKS are disabled for the synchronous compositor
because draws can come in at any time or not at all.

BUG=582749
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/bb917ddb01a8c3e373d6df02046c69ff00b07254
Cr-Commit-Position: refs/heads/master@{#376628}
==========

[commit-bot: I haz the power, 2016-02-20 05:22:26]

Patchset 7 (id:??) landed as
https://crrev.com/bb917ddb01a8c3e373d6df02046c69ff00b07254
Cr-Commit-Position: refs/heads/master@{#376628}