All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=890871

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


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

Android Nexus5 Perf
Android Nexus6 WebView Perf
android-nexus5x-perf

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

📍 Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/16c54fcce40000

📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/16c54fcce40000

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

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

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

tdresser@, what do we do when adding metrics reduces performance? The most likely cause is the additional UMA counters that are incremented on every frame update for more metrics.

Is there any way to know if UMA and/or UKM is off? Is our only choice to reduce the number of metrics?

One option, I suppose, is to only report an UMA sample every 30s, like we do for the UKM. Apart from fewer samples, is there any downside to that?

Sampling is reasonable.

I don't have time to dig in right now, but we may also be able to optimize how we're recording the UMA. Will look tomorrow.

Hmmm, there's a fair bit going on here, and it's likely someone on the UMA team will have better advice on how to optimize this.

It's possible that migrating over to STATIC_HISTOGRAM_POINTER_GROUP will see non-trivial wins, but I'm not sure. https://cs.chromium.org/chromium/src/base/metrics/histogram_macros.h?rcl=a4ca4b0f7dd11a99ce4f7c3e7d2d2ea71ba7a4e3&l=345

I recommend seeing if someone on the UMA team can provide some advice on how to speed this up. Maybe +isherman@?

I guess the other thing you could do is try just eliminating the actual call to record the UMA, and see what impact it has on the benchmark results.

That would help narrow down exactly where the slowdown is coming from.

I have a patch in process to reduce the newly-added UMA overhead by a factor of about 30. If that moves the needle then we know we're on the right track.

It seems optimizing the UMA code is something that would benefit everyone, but that would just set the baseline lower and this patch would probably still result in a perf regression.

For UMA metrics, we recommend using the UMA_HISTOGRAM* macros for performance-sensitive code. These macros cause each metric update to be a single atomic int op, which should be quite performant. However, they require using runtime-constant names, which can mean adding explicit branching (essentially, unrolling) to your code.

For UKM, we believe that if you're hitting performance bottlenecks in recording the metrics, you will also be using more memory than we support for UKM metrics, and we'll ultimately end up discarding your data. So, we'd recommend either sampling (which sounds like is the plan here), or recording pre-aggregated statistics to UKM.

--

> Is there any way to know if UMA and/or UKM is off? Is our only choice to reduce the number of metrics?

This is intentionally discouraged, because we don't want to create a performance incentive for opting out of metrics reporting. (We think that users should decide whether or not they opt into metrics reporting based on their comfort level with sharing that data, and not on any other tangential factors.)

--

+Steve as a UKM expert, in case there are further UKM questions

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

Thanks very much for the feedback in comment #8. We'll see how the patch affects things and move to the UMA_HISTOGRAM* calls if we need more improvement.

Our UKM data is already locally aggregating but I'm thinking of moving to sampling because I think it can be better structured to avoid sampling bias.

This one is still not improved. I'll look for additional ways to save.

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