cc: Record UMA for swap to swap ack latency.

cc: Record UMA for swap to swap ack latency.

BUG=525756
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

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

[brianderson, 2015-11-06 03:00:01]

brianderson@chromium.org changed reviewers:
+ tdresser@chromium.org

[brianderson, 2015-11-06 03:00:02]

This is a non quantized measurement of the swap to swap ack latency. Quantized
versions of swap throttling based on BeginFrame/deadline boundaries coming up.

[tdresser, 2015-11-06 13:00:34]

How sure are you that the 99'th percentile will be under 1 second?

It might make sense to use a somewhat coarser measurement.

Other than that, LGTM.

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

https://codereview.chromium.org/1423803004/diff/1/cc/scheduler/compositor_tim...
cc/scheduler/compositor_timing_history.h:90: base::TimeTicks swap_start_time_;
start_swap_time_, for consistency.

[mithro-old, 2015-11-07 04:13:15]

mithro@mithis.com changed reviewers:
+ mithro@mithis.com

[mithro-old, 2015-11-07 04:13:15]

Generally looks good and definitely think it should be added.

My only concern here is that if there are multiple swaps pending then this
calculation will be wrong?

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

https://codereview.chromium.org/1423803004/diff/1/cc/scheduler/compositor_tim...
cc/scheduler/compositor_timing_history.h:90: base::TimeTicks swap_start_time_;
Won't this fail if we have a max_pending_swaps > 1?

[brianderson, 2015-11-12 03:55:38]

On 2015/11/06 13:00:34, tdresser wrote:
> How sure are you that the 99'th percentile will be under 1 second?

It should be way lower than 1 second. In low latency mode cases, it should only
be a few milliseconds.

> 
> It might make sense to use a somewhat coarser measurement.
> 
> Other than that, LGTM.
> 
>
https://codereview.chromium.org/1423803004/diff/1/cc/scheduler/compositor_tim...
> File cc/scheduler/compositor_timing_history.h (right):
> 
>
https://codereview.chromium.org/1423803004/diff/1/cc/scheduler/compositor_tim...
> cc/scheduler/compositor_timing_history.h:90: base::TimeTicks swap_start_time_;
> start_swap_time_, for consistency.

[brianderson, 2015-11-12 04:01:02]

On 2015/11/07 04:13:15, mithro wrote:
> Generally looks good and definitely think it should be added.
> 
> My only concern here is that if there are multiple swaps pending then this
> calculation will be wrong?

I think it's the case now that all platforms and configurations use a
max_pending_swaps of 1 in the cc::Scheduler.

CrOS uses a max_pending_swaps of 2 in the cc::DisplayScheduler, but that doesn't
apply here.

There's a DCHECK in CompositorTimingHistory::DidSwapBuffers that will hit if we
ever set max_pending_swaps > 1. I'm inclined to keep it like that since we don't
have any plans to increase the max_pending_swaps.

> 
>
https://codereview.chromium.org/1423803004/diff/1/cc/scheduler/compositor_tim...
> File cc/scheduler/compositor_timing_history.h (right):
> 
>
https://codereview.chromium.org/1423803004/diff/1/cc/scheduler/compositor_tim...
> cc/scheduler/compositor_timing_history.h:90: base::TimeTicks swap_start_time_;
> Won't this fail if we have a max_pending_swaps > 1?

[brianderson, 2015-11-12 04:02:58]

brianderson@chromium.org changed reviewers:
+ isherman@chromium.org

[brianderson, 2015-11-12 04:02:59]

+isherman for owners review of histograms.xml

[Ilya Sherman, 2015-11-13 01:44:15]

LGTM

[brianderson, 2015-11-13 20:41:29]

On 2015/11/12 04:01:02, brianderson wrote:
> On 2015/11/07 04:13:15, mithro wrote:
> > Generally looks good and definitely think it should be added.
> > 
> > My only concern here is that if there are multiple swaps pending then this
> > calculation will be wrong?
> 
> I think it's the case now that all platforms and configurations use a
> max_pending_swaps of 1 in the cc::Scheduler.
> 
> CrOS uses a max_pending_swaps of 2 in the cc::DisplayScheduler, but that
doesn't
> apply here.
> 
> There's a DCHECK in CompositorTimingHistory::DidSwapBuffers that will hit if
we
> ever set max_pending_swaps > 1. I'm inclined to keep it like that since we
don't
> have any plans to increase the max_pending_swaps.

I lied. Looks like Blimp sets it to 2. See:
https://codereview.chromium.org/1435133004/

> 
> > 
> >
>
https://codereview.chromium.org/1423803004/diff/1/cc/scheduler/compositor_tim...
> > File cc/scheduler/compositor_timing_history.h (right):
> > 
> >
>
https://codereview.chromium.org/1423803004/diff/1/cc/scheduler/compositor_tim...
> > cc/scheduler/compositor_timing_history.h:90: base::TimeTicks
swap_start_time_;
> > Won't this fail if we have a max_pending_swaps > 1?

[brianderson, 2015-11-18 15:49:39]

The CQ bit was checked by brianderson@chromium.org

[brianderson, 2015-11-18 15:49:40]

The patchset sent to the CQ was uploaded after l-g-t-m from
isherman@chromium.org, tdresser@chromium.org
Link to the patchset: https://codereview.chromium.org/1423803004/#ps20001
(title: "rebase")

[commit-bot: I haz the power, 2015-11-18 15:50:35]

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

[brianderson, 2015-11-18 17:21:00]

The CQ bit was unchecked by brianderson@chromium.org

[brianderson, 2015-11-18 23:20:32]

The CQ bit was checked by brianderson@chromium.org

[commit-bot: I haz the power, 2015-11-18 23:22:57]

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

[commit-bot: I haz the power, 2015-11-19 00:01:22]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-11-19 00:02:12]

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