New issue
Advanced search Search tips
Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 4
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android , Windows , Chrome
Pri: 1
Type: Bug



Sign in to add a comment
link

Issue 870707: PageLoad.PaintTiming.NavigationToFirstContentfulPaint reporting many 0 and overflowed values on Android

Reported by csharrison@chromium.org, Aug 3 Project Member

Issue description

See UMA link:
https://uma.googleplex.com/p/chrome/timeline_v2/?sid=7f7335bbb4c7103c76be0f883d4b51ba

This looks to have been triggered in regression range:
https://chromium.googlesource.com/chromium/src/+log/69.0.3457.0..69.0.3462.0?pretty=fuller&n=10000

And partially mitigated in regression range:
https://chromium.googlesource.com/chromium/src/+log/70.0.3502.0..70.0.3503.0?pretty=fuller&n=10000

However, there's still more mass in the 0 bucket (~3%) than there used to be.
 

Comment 1 by schenney@chromium.org, Aug 3

Components: -Blink>Paint

Comment 2 by skobes@chromium.org, Aug 10

Cc: maxlg@chromium.org
Owner: tdres...@chromium.org
Status: Assigned (was: Untriaged)
Tim, I think you were looking at issues with Android timestamps, could this be related?

Comment 3 by maxlg@google.com, Aug 10

We have an issue that the swap time of FCP, FP etc have 0.3s difference from the screenshot timestamp. But it's not as far as being 0.

Comment 4 by tdres...@chromium.org, Aug 13

Cc: -sadrul@chromium.org tdres...@chromium.org
Owner: sadrul@chromium.org
I think https://chromium.googlesource.com/chromium/src/+/3821fdf5c54add24d02adef248a311c293ea0bd0 is probably responsible.

I thought sadrul@ already had a bug for this, but I couldn't find it...

Comment 5 by sadrul@chromium.org, Aug 13

Cc: sadrul@chromium.org
Owner: ----
Status: Available (was: Assigned)
GLFence was disabled on android (https://chromium-review.googlesource.com/c/chromium/src/+/1155472 landed on Jul 30). It doesn't look like that has made a difference in the UMA. So it is likely that there are some other issues involved.

Comment 6 by tdres...@chromium.org, Aug 14

Owner: npm@chromium.org
Status: Assigned (was: Available)

Comment 7 by npm@chromium.org, Aug 14

Cc: ccameron@chromium.org
Could either of 691239ed7c0738ac3f6dc37a1d026958e039903c or 	bd6ad56b4f186dc9db574c3d4ef3bd572ea3cd11 have caused the major fix? This would imply GLFence was probably responsible for the large dip. This still doesn't explain the persisting small dip in the graph.

Comment 9 by npm@chromium.org, Aug 14

Cc: -ccameron@chromium.org penghuang@chromium.org
Oh sorry. Maybe the fix is actually 6c51f537164d29b80ba89ca51fd94721873bd85c ? Other OS do not show dips and the implementation is pretty much shared so it seems to me like some sort of timer issue is the most likely problem.

Is it possible to view a graph with the counts of the 0 bucket?

Comment 10 by tdres...@chromium.org, Aug 15

You can view the histogram at various points in time, but you can't view a timeline of the counts in the zero bucket without writing custom queries.

Comment 11 by mattcary@chromium.org, Sep 21

I am looking at FCP in M69, and noticed that there seem to be a lot of large outliers. See [1]; M68 and M69 performance is similar if one looks at percentile (even 99%) or trimmed mean, but when looking at the arithmetic mean, there is a large increase in M69, suggesting that there are more and/or larger outliers.

Could that be related to this bug?

[1] https://uma.googleplex.com/p/chrome/timeline_v2?sid=fb5d0902f931aabbad817a41bdb27070

Comment 12 by mattcary@chromium.org, Sep 21

Cc: mattcary@chromium.org

Comment 13 by tdres...@chromium.org, Sep 25

Labels: -Pri-2 Pri-1

Comment 14 by bugdroid1@chromium.org, Sep 26

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

commit f962a47e219a605eb9648251df3d76471fe06bd9
Author: Nicolas Pena <npm@chromium.org>
Date: Wed Sep 26 16:01:46 2018

Paint timing: revert back to swap-time

This CL is a minimal change to use swap time instead of presentation
time in PageLoad.PaintTiming.NavigationToFirstContentfulPaint. This is a
tentative fix for the unexpected regressions in the metric. The change
is minimal since it may require a merge to Stable. A followup change
will record the presentation timestamp, or the delta with the swap
timestamp, in a separate metric.

Bug:  870707 
Change-Id: I1b48968d010bbd6120a6fbac59e26dc789d23394
Reviewed-on: https://chromium-review.googlesource.com/1244371
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: Nicolás Peña Moreno <npm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594331}
[modify] https://crrev.com/f962a47e219a605eb9648251df3d76471fe06bd9/content/renderer/gpu/layer_tree_view.cc

Comment 15 by npm@chromium.org, Oct 2

I'm setting up a Finch trial (no data yet on that). When this is live, half of Canary/Dev will use swap and the other half will use presentation.

Right now, Canary is using swap timestamps. Preliminary graphs suggest that FCP values are back to 'normal' after reverting to swap timestamps. Graphs I'm looking at:

https://uma.googleplex.com/p/chrome/timeline_v2/?sid=a069baf2dc8df31c70af71dbaba4b441

https://uma.googleplex.com/p/chrome/timeline_v2?sid=37763ffe35d196ecdcffcf0753ec5007

https://uma.googleplex.com/p/chrome/timeline_v2?sid=8fa1831a1a53d39ebd828498677b6093

Comment 16 by npm@chromium.org, Oct 2

Labels: Merge-Request-70
Requesting merge to M70 to recover FCP metric. The changed is considered safe because it reverts back to the behavior before M69.

Comment 17 by sheriffbot@chromium.org, Oct 2

Project Member
Labels: -Merge-Request-70 Merge-Review-70 Hotlist-Merge-Review
This bug requires manual review: We are only 13 days from stable.
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 18 by mdw@chromium.org, Oct 3

Hi there -- I would very much like to see this fix in M70. The regression on the FCP metrics is causing a bunch of problems for our performance tracking dashboards for NBU, and getting this fixed in the next milestone is quite high priority for us. Thanks!

Comment 19 by npm@chromium.org, Oct 3

Labels: OS-Chrome OS-Windows
This affects other OS as well so adding the ones we know are affected for sure. Ping on milestone owners for approval.

Comment 20 by benmason@chromium.org, Oct 4

Labels: -Hotlist-Merge-Review -Merge-Review-70 Merge-Approved-70
Fix looks safe and has been in canary since 71.0.3563.0 with no side-effects.

Approved to merge to 70, branch 3538.

Comment 21 by bugdroid1@chromium.org, Oct 4

Project Member
Labels: -merge-approved-70 merge-merged-3538
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/012527ecf5ced6845fbea843375f441a517b502a

commit 012527ecf5ced6845fbea843375f441a517b502a
Author: Nicolas Pena <npm@chromium.org>
Date: Thu Oct 04 15:13:14 2018

[M70] Paint timing: revert back to swap-time

This CL is a minimal change to use swap time instead of presentation
time in PageLoad.PaintTiming.NavigationToFirstContentfulPaint. This is a
tentative fix for the unexpected regressions in the metric. The change
is minimal since it may require a merge to Stable. A followup change
will record the presentation timestamp, or the delta with the swap
timestamp, in a separate metric.

Bug:  870707 
Change-Id: I1b48968d010bbd6120a6fbac59e26dc789d23394
Reviewed-on: https://chromium-review.googlesource.com/1244371
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: Nicolás Peña Moreno <npm@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#594331}(cherry picked from commit f962a47e219a605eb9648251df3d76471fe06bd9)
Reviewed-on: https://chromium-review.googlesource.com/c/1262035
Reviewed-by: Nicolás Peña Moreno <npm@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#850}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/012527ecf5ced6845fbea843375f441a517b502a/content/renderer/gpu/layer_tree_view.cc

Comment 22 by cr-audit...@appspot.gserviceaccount.com, Oct 4

Project Member
Labels: Merge-Merged-70-3538
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/012527ecf5ced6845fbea843375f441a517b502a

Commit: 012527ecf5ced6845fbea843375f441a517b502a
Author: npm@chromium.org
Commiter: npm@chromium.org
Date: 2018-10-04 15:13:14 +0000 UTC

[M70] Paint timing: revert back to swap-time

This CL is a minimal change to use swap time instead of presentation
time in PageLoad.PaintTiming.NavigationToFirstContentfulPaint. This is a
tentative fix for the unexpected regressions in the metric. The change
is minimal since it may require a merge to Stable. A followup change
will record the presentation timestamp, or the delta with the swap
timestamp, in a separate metric.

Bug:  870707 
Change-Id: I1b48968d010bbd6120a6fbac59e26dc789d23394
Reviewed-on: https://chromium-review.googlesource.com/1244371
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: Nicolás Peña Moreno <npm@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#594331}(cherry picked from commit f962a47e219a605eb9648251df3d76471fe06bd9)
Reviewed-on: https://chromium-review.googlesource.com/c/1262035
Reviewed-by: Nicolás Peña Moreno <npm@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#850}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}

Comment 23 by npm@chromium.org, Oct 4

Status: Fixed (was: Assigned)
Summary: PageLoad.PaintTiming.NavigationToFirstContentfulPaint reporting many 0 and overflowed values on Android (was: PageLoad.PaintTiming.NavigationToFirstContentfulPaint reporting many 0 values on Android)

Sign in to add a comment