New issue
Advanced search Search tips

Issue 910635 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

Add a UpdateTime Ratio metric for ScrollingCoordinator

Project Member Reported by schenney@chromium.org, Nov 30

Issue description

ScrollingCordinator takes up a non-trivial amount of Blink MainFrame update time. So track how much.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 1

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8a9326ce19965c7a52554ae2f8a2d1c990f1f724

commit 8a9326ce19965c7a52554ae2f8a2d1c990f1f724
Author: Stephen Chenney <schenney@chromium.org>
Date: Sat Dec 01 00:54:19 2018

Add UKM for ScrollingCoordinator.

ScrollingCoordinator accounts for some of the time in MainFrame
update time. Record how much.

As a side effect, move the macro for local frame UKM scoped recording
into the aggregator class so that it can be shared across classes.
This is to avoid relocating the UMA metric recording site in a way that
would skew the data toward zero, a lot.

R=pdr@chromium.org,rkaplow@chromium.org
BUG= 910635 

Change-Id: Id527efb7f30cc69de8ee45e27c616621059ef544
Reviewed-on: https://chromium-review.googlesource.com/c/1356888
Commit-Queue: Stephen Chenney <schenney@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612880}
[modify] https://crrev.com/8a9326ce19965c7a52554ae2f8a2d1c990f1f724/third_party/blink/renderer/core/frame/local_frame_ukm_aggregator.h
[modify] https://crrev.com/8a9326ce19965c7a52554ae2f8a2d1c990f1f724/third_party/blink/renderer/core/frame/local_frame_ukm_aggregator_test.cc
[modify] https://crrev.com/8a9326ce19965c7a52554ae2f8a2d1c990f1f724/third_party/blink/renderer/core/frame/local_frame_view.cc
[modify] https://crrev.com/8a9326ce19965c7a52554ae2f8a2d1c990f1f724/third_party/blink/renderer/core/frame/local_frame_view.h
[modify] https://crrev.com/8a9326ce19965c7a52554ae2f8a2d1c990f1f724/third_party/blink/renderer/core/page/scrolling/scrolling_coordinator.cc
[modify] https://crrev.com/8a9326ce19965c7a52554ae2f8a2d1c990f1f724/tools/metrics/ukm/ukm.xml

Labels: Merge-Request-72
Status: Started (was: Assigned)
This patch to gather important metrics missed the M-72 branch by 400 commits. We would very much like to get it in so that we get Stable metrics as soon as possible.
Project Member

Comment 3 by sheriffbot@chromium.org, Dec 4

Labels: -Merge-Request-72 Hotlist-Merge-Approved Merge-Approved-72
Your change meets the bar and is auto-approved for M72. Please go ahead and merge the CL to branch 3626 manually. Please contact milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Pls merge your change to M72 branch 3626 ASAP so we can pick it up for tomororw's dev release. Thank you.
Project Member

Comment 5 by bugdroid1@chromium.org, Dec 4

Labels: -merge-approved-72 merge-merged-3626
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3439e5a044d2e2b5534d3810a9fa650c84dc0abf

commit 3439e5a044d2e2b5534d3810a9fa650c84dc0abf
Author: Stephen Chenney <schenney@chromium.org>
Date: Tue Dec 04 17:01:10 2018

Add UKM for ScrollingCoordinator.

M-72 Merge

ScrollingCoordinator accounts for some of the time in MainFrame
update time. Record how much.

As a side effect, move the macro for local frame UKM scoped recording
into the aggregator class so that it can be shared across classes.
This is to avoid relocating the UMA metric recording site in a way that
would skew the data toward zero, a lot.

R=pdr@chromium.org, rkaplow@chromium.org
TBR=schenney@chromium.org
BUG= 910635 

(cherry picked from commit 8a9326ce19965c7a52554ae2f8a2d1c990f1f724)

Change-Id: Id527efb7f30cc69de8ee45e27c616621059ef544
Reviewed-on: https://chromium-review.googlesource.com/c/1356888
Commit-Queue: Stephen Chenney <schenney@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#612880}
Reviewed-on: https://chromium-review.googlesource.com/c/1361332
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#30}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/3439e5a044d2e2b5534d3810a9fa650c84dc0abf/third_party/blink/renderer/core/frame/local_frame_ukm_aggregator.h
[modify] https://crrev.com/3439e5a044d2e2b5534d3810a9fa650c84dc0abf/third_party/blink/renderer/core/frame/local_frame_ukm_aggregator_test.cc
[modify] https://crrev.com/3439e5a044d2e2b5534d3810a9fa650c84dc0abf/third_party/blink/renderer/core/frame/local_frame_view.cc
[modify] https://crrev.com/3439e5a044d2e2b5534d3810a9fa650c84dc0abf/third_party/blink/renderer/core/frame/local_frame_view.h
[modify] https://crrev.com/3439e5a044d2e2b5534d3810a9fa650c84dc0abf/third_party/blink/renderer/core/page/scrolling/scrolling_coordinator.cc
[modify] https://crrev.com/3439e5a044d2e2b5534d3810a9fa650c84dc0abf/tools/metrics/ukm/ukm.xml

Status: Fixed (was: Started)
Labels: Merge-Merged-72-3626
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/3439e5a044d2e2b5534d3810a9fa650c84dc0abf

Commit: 3439e5a044d2e2b5534d3810a9fa650c84dc0abf
Author: schenney@chromium.org
Commiter: schenney@chromium.org
Date: 2018-12-04 17:01:10 +0000 UTC

Add UKM for ScrollingCoordinator.

M-72 Merge

ScrollingCoordinator accounts for some of the time in MainFrame
update time. Record how much.

As a side effect, move the macro for local frame UKM scoped recording
into the aggregator class so that it can be shared across classes.
This is to avoid relocating the UMA metric recording site in a way that
would skew the data toward zero, a lot.

R=pdr@chromium.org, rkaplow@chromium.org
TBR=schenney@chromium.org
BUG= 910635 

(cherry picked from commit 8a9326ce19965c7a52554ae2f8a2d1c990f1f724)

Change-Id: Id527efb7f30cc69de8ee45e27c616621059ef544
Reviewed-on: https://chromium-review.googlesource.com/c/1356888
Commit-Queue: Stephen Chenney <schenney@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#612880}
Reviewed-on: https://chromium-review.googlesource.com/c/1361332
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#30}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}

Sign in to add a comment