Change input latency result to depend on start of GPU swap

Change input latency result to depend on start of GPU swap

Use INPUT_EVENT_GPU_SWAP_BUFFER_COMPONENT instead of
INPUT_EVENT_LATENCY_TERMINATED_FRAME_SWAP_COMPONENT as the
final component to calculate input latency.

This will more accurately reflect latency regressions in the
code we have direct control of in Chrome.

Also make PassThroughImageTransportSurface::PostSubBuffer
add a INPUT_EVENT_GPU_SWAP_BUFFER_COMPONENT, which it
was missing before.

Followup patches will snap
INPUT_EVENT_LATENCY_TERMINATED_FRAME_SWAP_COMPONENT
to the next vsync to reflect the real display time.

The real display time can then be used for better
smoothness metrics and also to track latency regressions
at the OS/driver level for Ozone and potentially other
platforms where we can extract swap info.

BUG=360844
Committed: https://crrev.com/62a1ccd2bfd61886dcdb3fed4e020fd77771699f
Cr-Commit-Position: refs/heads/master@{#318812}

[brianderson, 2015-02-12 01:07:00]

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

[brianderson, 2015-02-12 01:07:01]

Yufeng, what do you think of this?

I'm not sure of all the platform differences between the following two
compenents:
INPUT_EVENT_GPU_SWAP_BUFFER_COMPONENT
INPUT_EVENT_LATENCY_TERMINATED_FRAME_SWAP_COMPONENT

However, it seems like INPUT_EVENT_LATENCY_TERMINATED_FRAME_SWAP_COMPONENT can
sometimes return immediately after gpu swap and sometimes have a delay depending
on the platform.

I think it would be better to track INPUT_EVENT_GPU_SWAP_BUFFER_COMPONENT, which
will better reflect small latency regressions in Chrome. Then repurpose
INPUT_EVENT_LATENCY_TERMINATED_FRAME_SWAP_COMPONENT so it always snaps to the
next vsync and use it for better smoothness metrics.

[Yufeng Shen (Slow to review), 2015-02-12 20:27:28]

On 2015/02/12 01:07:01, brianderson wrote:
> Yufeng, what do you think of this?
> 
> I'm not sure of all the platform differences between the following two
> compenents:
> INPUT_EVENT_GPU_SWAP_BUFFER_COMPONENT
> INPUT_EVENT_LATENCY_TERMINATED_FRAME_SWAP_COMPONENT
> 
> However, it seems like INPUT_EVENT_LATENCY_TERMINATED_FRAME_SWAP_COMPONENT can
> sometimes return immediately after gpu swap and sometimes have a delay
depending
> on the platform.
> 
> I think it would be better to track INPUT_EVENT_GPU_SWAP_BUFFER_COMPONENT,
which
> will better reflect small latency regressions in Chrome. Then repurpose
> INPUT_EVENT_LATENCY_TERMINATED_FRAME_SWAP_COMPONENT so it always snaps to the
> next vsync and use it for better smoothness metrics.

Sounds good to me. jbauman@ once had the same suggestion of using the before gpu
swap timestamp instead of the after to make the metrics less noisy.

We probably also want to change the UMA metrics to be consistent with our
telemetry metrics
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/re...

It has value that our UMA data won't be skewed overtime by newly introduced
devices which
have various gpu swap costs.
And for tracking the gpu swap cost, I happen to have a CL
https://codereview.chromium.org/921473003/diff/20001/content/browser/renderer...
that adds a new UMA metric  Event.Latency.ScrollUpdateGpuSwap.

[brianderson, 2015-02-12 21:36:38]

ptal

[Yufeng Shen (Slow to review), 2015-02-12 21:57:50]

lgtm

https://codereview.chromium.org/918883002/diff/40001/content/browser/renderer...
File content/browser/renderer_host/render_widget_host_latency_tracker.cc
(right):

https://codereview.chromium.org/918883002/diff/40001/content/browser/renderer...
content/browser/renderer_host/render_widget_host_latency_tracker.cc:332:
LatencyInfo::LatencyComponent rwh_component;
nit: this can be removed, and just pass nullptr to the following FindLatency().

[brianderson, 2015-02-12 22:25:22]

New patchsets have been uploaded after l-g-t-m from miletus@chromium.org

[brianderson, 2015-02-12 22:26:40]

brianderson@chromium.org changed reviewers:
+ sievers@chromium.org, skyostil@chromium.org

[brianderson, 2015-02-12 22:26:40]

skyostil@chromium.org: Please review changes in tools

sievers@chromium.org: Please review changes in content

[Sami, 2015-02-13 14:58:20]

lgtm. Once we have richer trace event data from SurfaceFlinger we can use that
to cross check the next-vsync-snapping assumption here.

[brianderson, 2015-02-13 21:48:04]

brianderson@chromium.org changed reviewers:
- sievers@chromium.org

[brianderson, 2015-02-13 21:50:47]

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

[brianderson, 2015-02-13 21:50:48]

+ccameron for owners review of files in content.

[ccameron, 2015-02-13 21:57:28]

OWNER stamp, in that the change does what the description says it does.

[ccameron, 2015-02-13 21:57:46]

On 2015/02/13 21:57:28, ccameron wrote:
> OWNER stamp, in that the change does what the description says it does.

(OWNER stamp lgtm)

[brianderson, 2015-02-13 22:13:05]

The CQ bit was checked by brianderson@chromium.org

[commit-bot: I haz the power, 2015-02-13 22:13:48]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/918883002/60001

[commit-bot: I haz the power, 2015-02-13 23:12:57]

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

[commit-bot: I haz the power, 2015-02-13 23:12:59]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[brianderson, 2015-02-27 22:31:23]

The CQ bit was checked by brianderson@chromium.org

[commit-bot: I haz the power, 2015-02-27 22:33:08]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/918883002/60001

[commit-bot: I haz the power, 2015-02-27 23:08:09]

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

[commit-bot: I haz the power, 2015-02-27 23:08:10]

Try jobs failed on following builders:
  android_aosp on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)

[brianderson, 2015-02-27 23:42:56]

Yufeng, I rebased this on top of a recent patch of yours. As a sanity check, can
you take another look?

[Yufeng Shen (Slow to review), 2015-03-02 23:54:27]

On 2015/02/27 23:42:56, brianderson wrote:
> Yufeng, I rebased this on top of a recent patch of yours. As a sanity check,
can
> you take another look?

still lgtm.

[brianderson, 2015-03-02 23:54:49]

The CQ bit was checked by brianderson@chromium.org

[brianderson, 2015-03-02 23:54:49]

The patchset sent to the CQ was uploaded after l-g-t-m from
skyostil@chromium.org, ccameron@chromium.org
Link to the patchset: https://codereview.chromium.org/918883002/#ps70001 (title:
"rebase")

[commit-bot: I haz the power, 2015-03-02 23:56:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/918883002/70001

[commit-bot: I haz the power, 2015-03-03 01:33:23]

Committed patchset #5 (id:70001)

[commit-bot: I haz the power, 2015-03-03 01:34:19]

Patchset 5 (id:??) landed as
https://crrev.com/62a1ccd2bfd61886dcdb3fed4e020fd77771699f
Cr-Commit-Position: refs/heads/master@{#318812}