New issue
Advanced search Search tips

Issue 891003 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

4.4%-56.4% regression in rendering.mobile/thread_total_all_cpu_time_per_frame at 594754:594939

Project Member Reported by tdres...@chromium.org, Oct 1

Issue description

See the link to graphs below.
 
All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=891003

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=6912a2f82c786da92209ed097fc968ad8c0257ba9ba9affd98c1d9256e5782e9


Bot(s) for this bug's original alert(s):

Android Nexus5 Perf
android-nexus5x-perf

rendering.mobile - Benchmark documentation link:
  https://bit.ly/rendering-benchmarks
Cc: schenney@chromium.org
Owner: schenney@chromium.org
Status: Assigned (was: Untriaged)
📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/12ff91dce40000

Add UMA and UKM for forced layouts by schenney@chromium.org
https://chromium.googlesource.com/chromium/src/+/fe7f8c3cc9889e4a1fd18742691d621982b2c171
4.542 → 7.089 (+2.547)

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions

Benchmark documentation link:
  https://bit.ly/rendering-benchmarks
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 5

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

commit b942c118f1ba5aab7e3cfd3b962aa277cd851346
Author: Stephen Chenney <schenney@chromium.org>
Date: Fri Oct 05 14:44:11 2018

Record MainFrame ratio UMA only every 30 seconds

Mobile performance regressed when UMA and UKM metrics for the
Rendering Core team were added to BeginMainFrame. We hypothesize
that the primary cost is recording UMA on every frame for the
new metrics, so this patch switches to recording UMA only every 30
seconds, or on frame destruction, when the UKM metrics are recorded.

The plan is to monitor performance graphs to determine if this has the
desired effect.

For the ForcedLayout metric we may need to send the primary UMA metric
only once per frame, not once per forced layout as is done now. That is
a follow on patch.

R=vmpstr@chromium.org
BUG=890871,891003,891341

Change-Id: I2f4ec8bb9301cece98aef52463d671e4bebcd91e
Reviewed-on: https://chromium-review.googlesource.com/c/1260443
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: vmpstr <vmpstr@chromium.org>
Commit-Queue: Stephen Chenney <schenney@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597113}
[modify] https://crrev.com/b942c118f1ba5aab7e3cfd3b962aa277cd851346/third_party/blink/renderer/core/frame/local_frame_ukm_aggregator.cc
[modify] https://crrev.com/b942c118f1ba5aab7e3cfd3b962aa277cd851346/third_party/blink/renderer/core/frame/local_frame_ukm_aggregator.h
[modify] https://crrev.com/b942c118f1ba5aab7e3cfd3b962aa277cd851346/tools/metrics/histograms/histograms.xml

Still elevated.
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 17

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

commit 2ecdfc5d281dfca76778a1218aca5215aaf91793
Author: Stephen Chenney <schenney@chromium.org>
Date: Wed Oct 17 19:27:32 2018

Revert "Record MainFrame ratio UMA only every 30 seconds"

This reverts commit b942c118f1ba5aab7e3cfd3b962aa277cd851346.

Reason for revert: The patch caused all of our ratio metrics to disappear on the UMA dashboard. The metrics are being sent, but I suspect the sample count is too low for reporting. Also, it didn't fix the perf regression. 

Original change's description:
> Record MainFrame ratio UMA only every 30 seconds
> 
> Mobile performance regressed when UMA and UKM metrics for the
> Rendering Core team were added to BeginMainFrame. We hypothesize
> that the primary cost is recording UMA on every frame for the
> new metrics, so this patch switches to recording UMA only every 30
> seconds, or on frame destruction, when the UKM metrics are recorded.
> 
> The plan is to monitor performance graphs to determine if this has the
> desired effect.
> 
> For the ForcedLayout metric we may need to send the primary UMA metric
> only once per frame, not once per forced layout as is done now. That is
> a follow on patch.
> 
> R=​vmpstr@chromium.org
> BUG=890871,891003,891341
> 
> Change-Id: I2f4ec8bb9301cece98aef52463d671e4bebcd91e
> Reviewed-on: https://chromium-review.googlesource.com/c/1260443
> Reviewed-by: Steven Holte <holte@chromium.org>
> Reviewed-by: vmpstr <vmpstr@chromium.org>
> Commit-Queue: Stephen Chenney <schenney@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#597113}

TBR=vmpstr@chromium.org,rkaplow@chromium.org,holte@chromium.org,schenney@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 890871, 891003, 891341
Change-Id: I4668b91a1cc7080d65ba722a2168652cd5b89416
Reviewed-on: https://chromium-review.googlesource.com/c/1286735
Commit-Queue: Stephen Chenney <schenney@chromium.org>
Reviewed-by: Stephen Chenney <schenney@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600516}
[modify] https://crrev.com/2ecdfc5d281dfca76778a1218aca5215aaf91793/third_party/blink/renderer/core/frame/local_frame_ukm_aggregator.cc
[modify] https://crrev.com/2ecdfc5d281dfca76778a1218aca5215aaf91793/third_party/blink/renderer/core/frame/local_frame_ukm_aggregator.h
[modify] https://crrev.com/2ecdfc5d281dfca76778a1218aca5215aaf91793/tools/metrics/histograms/histograms.xml

Sign in to add a comment