1220699 - Add telemetry probe to measure vsync latency in the refresh driver

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Mason Chang [Inactive] [:mchang]]

See https://bugzilla.mozilla.org/show_bug.cgi?id=1144946#c7

Comment 1

[Mason Chang [Inactive] [:mchang]]

Attachment: Add telemetry probe for vsync latency in refresh driver

Comment 2

[Mason Chang [Inactive] [:mchang]]

Attachment: Add telemetry probe to measure vsync latency in the refresh driver

Comment 3

[Avi Halachmi (:avih)]

Comment on attachment 8682077 [details] [diff] [review]
Add telemetry probe to measure vsync latency in the refresh driver

Review of attachment 8682077 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with the comment addressed.

We well discuss later what other related metrics are required with e10s+APZ.

::: layout/base/nsRefreshDriver.cpp
@@ +362,5 @@
>        if (XRE_IsParentProcess()) {
>          MonitorAutoLock lock(mRefreshTickLock);
> +        #ifndef ANDROID  /* bug 1142079 */
> +          TimeDuration vsyncLatency = TimeStamp::Now() - aVsyncTimestamp;
> +          Telemetry::Accumulate(Telemetry::FX_REFRESH_DRIVER_FRAME_DELAY_MS,

Change this to FX_REFRESH_DRIVER_CHROME_FRAME_DELAY_MS

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d2b705bd76e1

Comment 5

[Vladan Djeric (:vladan)]

Btw, this will need to be uplifted to Aurora and Beta

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d2b705bd76e1

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/d2b705bd76e1

Comment 8

[Mason Chang [Inactive] [:mchang]]

Comment on attachment 8682077 [details] [diff] [review]
Add telemetry probe to measure vsync latency in the refresh driver

Approval Request Comment
[Feature/regressing bug #]: Bug 1144946.
[User impact if declined]: None, this is a telemetry probe that currently exists in beta, but was accidentally deleted / unused.
[Describe test coverage new/current, TreeHerder]: Mochitest, manual testing.
[Risks and why]: Low, this is just a telemetry probe.
[String/UUID change made/needed]: None

Comment 9

[Mason Chang [Inactive] [:mchang]]

Comment on attachment 8682077 [details] [diff] [review]
Add telemetry probe to measure vsync latency in the refresh driver

Actually, this patch won't work since it needs to be rebased.

Comment 10

[Mason Chang [Inactive] [:mchang]]

Attachment: Add telemetry probe to measure sync latency in chrome refresh driver

See comment 8. Patch rebased for aurora/beta.

Comment 11

[Carsten Book [:Tomcat]]

removing the b2g 2.5 flag since this commit has been reverted due to an incorrect merge, sorry for the confusion

Comment 12

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8683209 [details] [diff] [review]
Add telemetry probe to measure sync latency in chrome refresh driver

Telemetry change, should be safe.
Should be in 43 beta 2.

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/pushloghtml?changeset=e96670656f90

Comment 14

[Carsten Book [:Tomcat]]

(In reply to Carsten Book [:Tomcat] from comment #13)
> https://hg.mozilla.org/releases/mozilla-aurora/
> pushloghtml?changeset=e96670656f90

ups https://hg.mozilla.org/releases/mozilla-aurora/rev/44ce9fe80d5d is the correct one

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/d681ec6fe6b2

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/44ce9fe80d5d