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

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


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

android-webview-nexus5X
android-webview-nexus6

📍 Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/12b5072d840000

📍 Couldn't reproduce a difference.
https://pinpoint-dot-chromeperf.appspot.com/job/12b5072d840000

📍 Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/118a392d840000

No-repro bisect was due to lack of capacity on N6 webview. Kicked off N5X bisect.

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

Add benchmark to DirectRenderer::DrawFrame trace by rjkroege@chromium.org
https://chromium.googlesource.com/chromium/src/+/7ff394ed7b91cd57d2533872b27d7ee990badd38

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

The infrastructure does definitely point at my change. But the increase in the number of posted tasks seems like a non-issue. My CL does not regress any of the timing metrics. In particular: while tasks_per_frame_other may have increased, the thread_other_cpu_time_per_frame and thread_browser_cpu_time_per_frame remained constant.

Re-opening... I think we should understand what's going on here. rjkroege@ CL should not change tasks_per_frame. If it does, the metric is probably wrong.

I suspect that the trace slices that are added in the CL somehow end up as top level slices and so the benchmark gets confused and thinks there are new tasks.

Also, it looks like it happens only on Android.

+sunnyps@ +vmiura@ who may have some ideas.

I'm looking at traces from "ChromiumPerf/android-webview-nexus5X/thread_times.tough_compositor_cases / tasks_per_frame_other", and I'm seeing a drop in the number of frame stats produced.

BenchmarkInstrumentation::DisplayRenderingStats [360 -> 288], without a drop in the number of tasks on Chrome_InProcRendererThread [504 -> 506].  Hence the number of "other" tasks per frame increased. *

That is utterly weird that enabling a trace could cause the rendering stats to change.  What is the thread "1404" that these traces are issued on?  Link to trace with your change : https://00e9e64bac859a962f8f046ea2e54af398448173f0ff07ded8-apidata.googleusercontent.com/download/storage/v1/b/chrome-telemetry-output/o/http___jsbin_com_beqojupo_1_quiet_JS_FULL_SCREEN_INVALIDATION_2018-02-09_23-08-59_4966.html?qk=AD5uMEtPTi378MKqfnbdPj0b9XfiIamFpfhz-Ea4U87DtABBYQCC3s8UlBmGeI4L8Kr1gFPkEb3JzpL92xl5j17i8Pa_1JSulpMUluYQwvk7PaYW8iD3pCGoq2u7IWWFyAM19nnuA9nyUQBcNfsSe8Klhubh2jbvg-io_NVZCbph-Sv_F2orU3GSW78c6GXe28Rpty4IeAb1YEQ2RoEPFVsWsP4vT4G5lR5OnQhUrq2a6ka4Du0iMa1E7jMeb990X0EfTNRoXYdqKizXLYOKjG-8yPzcu9gHWJ36Rgc0lm7NnMTcN2ocAh-ditPKIZdK6LfNV9MrDvOzyjPQBSCk_JY24HhywaFuQPFP0vL5-8qtNNKzfvXyzKNAKld-9OMcslkvbnJTk27R1uhsHdrWqU8Kzdn_f51NEvmNRKEVQ9tK47R1YihZAGLJB-mV9-JxrDFw-0C0GFen0k1hFbT8KnTuTe8PkqyCB_LZ_DHcek0fuU-14V60nv9k7P_HFOjoiImmOkM8A-4j8jqfYWM_QOuRiS5fD2rcxO8XsuKFUjjwK2euCSjfsH97W9Ci4WhuO_S_bmdK1OIAqj-v_xnsIMjDaYumP6Zz_M2M5kQBLBQyBiPYiITVyBOZImumQSKt52ZzsE4K_EkQ4D8CIvrZlR9_9F9RYW2PizD-_5LgHpMt828loiNU1jfP2Xn0rGrhb-VQrY_05RVRjB-BRZsZCIN4I1pHTm9E_fyfYQPQaLcXC0TL_c7NlbpYQ7llKB6HUc5I44KY1uDAqh_JQ0SNHlW7BGD-u7VKJupp2Tk8RtRRib7YMaVOlvO5hhYqhPMqfs1ihWtqdIjmuhNY-_yD-baxnIzrTD5uNw

* The reason the "other" category is up, is Chrome_InProcRendererThread isn't matching any of our thread name patterns.  It should be classified as a "renderer_main" thread.

I suspect we're seeing thermal throttling.  There is a slowdown in the middle of the JS_FULL_SCREEN_INVALIDATION test, where display goes from 60fps to 30fps.  This can be seen both and after rjk's change, but with rjk's change it happens sooner and so the overall frame rate gets worse.  No idea why :-\.

What I don't get though is why we're going straight from from 60fps to 30fps display rate.  The renderer and compositor seem to still be ticking at 60fps, so something is doing frame skipping.

+bo & brian in case they know what could cause frame skipping in WebView's scheduling.

N5X Before: https://00e9e64bac9c6c447f32945e789e647cc9854e1f1ee5b58f82-apidata.googleusercontent.com/download/storage/v1/b/chrome-telemetry-output/o/http___jsbin_com_beqojupo_1_quiet_JS_FULL_SCREEN_INVALIDATION_2018-02-09_12-42-39_27295.html?qk=AD5uMEs0lgpIjbpgiiZqDOQPngwWxf4fdBGc9MpxKH-SCP4l3pb4NbfwhkFEGiOoGe2OxB2zkPGfQ73d4xe7I_iy_gWfRaxSegNprO65ghOHeJ02W1d_dXkb2BdHLHY2iXjJ-rwmI_hXCuhm9B01vU80E2aUxNW28v2VYD3G2eXQgQBcRKJgBZ4clIKkXk2tBxMTZVr988aXwpxnxqsV4cOXGoBAVin4rI_yZ8d0CVIuJ-nIbtzKeCQ_fCayk3sTByAtlinbn1NTiaRIVumyUvUM2m8P0mo2raJ2SMJjApLcmYExDv7DEEnfKL8B6MaFgIOYhBluEIGp-3podvGybzfzwo0i7mpUoV2FUpikxgRK84LX_rKF_875Ffj-2bwwPvJDUT4tYd4kJ-AU2soOftsPtxrxgsRoFHwCOpEapt0_hed_suyXmy2mWE-vSrG1qvbhyaA3_t_X2DqaguWFS7nSHBHnUPlFcge6HWZLe0L05td_LFd-PFk_doL3c3_iF-bo2Ai2oLOp97VQ5rzoKK2vV4AxJxOlCPuwhfi9c_9IufbwuaPs8mz6eCz7kRQN4M2WwTaJ3_TC9nS_HzCm7iLszD8mxCJrL7fWUzVHeBnE12XwQdG4jr_b1xamI8c5is-EeiLWOg5BHj3_whnjdqSm7G_l1st_7jCLwAZUPYXYtrNOe-WnG5vehfsZNgxRg3AjR965UPfivrcQ_FcvUW1ZjuF2tEzkwvUs9dmYvmAyLlp5d7dDzi2VQkqwlcjsJsT1gujDMK_wWBR8pGyk4Pd4Z6MhlQaAQyLKXKEGx8apcR5BD1hVNMMYHptM1jq4p14pUTPBDVuY-yXIfZ8SkScR_wcyY95H-NchNHybdl0Svmm2d0ugFF0

N5X After: https://00e9e64bac859a962f8f046ea2e54af398448173f0ff07ded8-apidata.googleusercontent.com/download/storage/v1/b/chrome-telemetry-output/o/http___jsbin_com_beqojupo_1_quiet_JS_FULL_SCREEN_INVALIDATION_2018-02-09_23-08-59_4966.html?qk=AD5uMEtPTi378MKqfnbdPj0b9XfiIamFpfhz-Ea4U87DtABBYQCC3s8UlBmGeI4L8Kr1gFPkEb3JzpL92xl5j17i8Pa_1JSulpMUluYQwvk7PaYW8iD3pCGoq2u7IWWFyAM19nnuA9nyUQBcNfsSe8Klhubh2jbvg-io_NVZCbph-Sv_F2orU3GSW78c6GXe28Rpty4IeAb1YEQ2RoEPFVsWsP4vT4G5lR5OnQhUrq2a6ka4Du0iMa1E7jMeb990X0EfTNRoXYdqKizXLYOKjG-8yPzcu9gHWJ36Rgc0lm7NnMTcN2ocAh-ditPKIZdK6LfNV9MrDvOzyjPQBSCk_JY24HhywaFuQPFP0vL5-8qtNNKzfvXyzKNAKld-9OMcslkvbnJTk27R1uhsHdrWqU8Kzdn_f51NEvmNRKEVQ9tK47R1YihZAGLJB-mV9-JxrDFw-0C0GFen0k1hFbT8KnTuTe8PkqyCB_LZ_DHcek0fuU-14V60nv9k7P_HFOjoiImmOkM8A-4j8jqfYWM_QOuRiS5fD2rcxO8XsuKFUjjwK2euCSjfsH97W9Ci4WhuO_S_bmdK1OIAqj-v_xnsIMjDaYumP6Zz_M2M5kQBLBQyBiPYiITVyBOZImumQSKt52ZzsE4K_EkQ4D8CIvrZlR9_9F9RYW2PizD-_5LgHpMt828loiNU1jfP2Xn0rGrhb-VQrY_05RVRjB-BRZsZCIN4I1pHTm9E_fyfYQPQaLcXC0TL_c7NlbpYQ7llKB6HUc5I44KY1uDAqh_JQ0SNHlW7BGD-u7VKJupp2Tk8RtRRib7YMaVOlvO5hhYqhPMqfs1ihWtqdIjmuhNY-_yD-baxnIzrTD5uNw

so... do I need to be reverting my CL? I shudder to think that my change would have non-trivial performance impact.

> so... do I need to be reverting my CL? I shudder to think that my change would have non-trivial performance impact.

It's possible that your change was the straw that broke the camel's back, or that pinpoint didn't quite identify the right change here due to thermals variability.

Either way I don't think it makes sense to revert your change.

I would like some follow up on what's happening with our scheduling here... frame skipping seems unexpected.

err, those gigantic links in #11 don't work for me. I assume they are traces. I don't see links to traces from tasks_per_frame_other graph either, how do I navigate to those traces exactly?

actually, just looking at the change, maybe this does make sense..

tasks_per_frame_other, I assume "other" means tasks on none of the important threads. The CL adds a trace to the benchmark category, which means that event will now be included in the trace on perf bots. So maybe it's just more things happening to record the additional trace event? Guestimating here..

📍 Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/13f39093840000

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

📍 Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/1498194b840000

The change in frames drawn for JS_FULL_SCREEN_INVALIDATION might be a distraction.

Comparing the before/after traces on N5X of CC_POSTER_CIRCLE, the number of frames drawn (BenchmarkInstrumentation::DisplayRenderingStats) is the same, but the number of tasks per frame goes up almost exactly by 1. See:
https://chromeperf.appspot.com/report?sid=5acb05996af6df6649781591f7857674f1ed9e89655bea30be48f58ab0373d17&start_rev=530776&end_rev=537038

It does look like it detects the new trace as a task that didn't exist before.

The DirectRenderer::DrawFrame trace shows up on the same thread as the BenchmarkInstrumentation::DisplayRenderingStats. The thread doesn't have a name, but I'm assuming it is WebView's drawing thread. Since it's a WebView thread and we don't control the message pump there, the trace isn't under an existing task like "MessageLoop::RunTask" where I'm assuming it wouldn't have been counted as an extra task.

So it seem like the "other" category is correct.

I've started another bisect for the thermal throttling in JS_FULL_SCREEN_INVALIDATION, just in case that's a separate issue.

Otherwise, I'm not sure what we can do to avoid new traces on the WebView thread from being falsely detected as a new task. I don't think it would be right to filter out traces from the WebView thread either - so I'm inclined to close as WontFix.

If #19 is the reason (I think it is), and there is no immidiate fix, shouldn't we "disable" (or make it to not generate alerts) task_per_frame_other for webview because it does not mean what it is supposed to mean and so any regression/improvement is meaningless?

Bo & Brian are right.  The frame rate is changing which may account for some of this change, but just the inclusion of the "DirectRenderer::DrawFrame" explains the increase in the number of tasks counted per frame by 1.

The reason we're seeing this now is we must not have had a top-level trace on this thread, which I assume is the Android UI thread.

#20: I don't think it makes sense to disable it completely, we can just ignore this alert now that we understand it, and add a top-level trace to the WebView draw functor to make sure it's always counted in thread times & task counts.

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

Add benchmark to DirectRenderer::DrawFrame trace by rjkroege@chromium.org
https://chromium.googlesource.com/chromium/src/+/7ff394ed7b91cd57d2533872b27d7ee990badd38

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

Sorry for the noise! Switching this back to WontFix. There's an open bug on bisect not to change the status of WontFix bugs.

Thanks.  The spurious metric change should be fixed by https://chromium-review.googlesource.com/c/chromium/src/+/923392.

Attaching one of the traces for Bo to check.  This is the "N5X After" trace.

Deleted: http___jsbin_com_beqojupo_1_quiet_JS_FULL_SCREEN_INVALIDATION_2018-02-09_23-08-59_4966.html 6.5 MB

http___jsbin_com_beqojupo_1_quiet_JS_FULL_SCREEN_INVALIDATION_2018-02-09_23-08-59_4966.html 6.5 MB View Download

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

Add benchmark to DirectRenderer::DrawFrame trace by rjkroege@chromium.org
https://chromium.googlesource.com/chromium/src/+/7ff394ed7b91cd57d2533872b27d7ee990badd38

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