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

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


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

chromium-rel-win7-gpu-intel
chromium-rel-win7-gpu-nvidia
chromium-rel-win7-x64-dual
chromium-rel-win8-dual

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

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

📍 Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/148b84acc40000

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

📍 Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/14cd64acc40000

Trying bisects on a few more platforms/pages.

📍 Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/14a6d702c40000

Also trying a bisect without story filter.

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

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

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

📍 Found significant differences after each of 2 commits.
https://pinpoint-dot-chromeperf.appspot.com/job/14a6d702c40000

webauthn: add test for oversized credential IDs. by agl@chromium.org
https://chromium.googlesource.com/chromium/src/+/ce1605967091027c08692ee570366d8b808a72fb

Adding fieldtrial config for the IncompatibleApplicationsWarning feature by pmonette@chromium.org
https://chromium.googlesource.com/chromium/src/+/7a86e22e588d7941bde54bdb4a8c58c23e502836

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

-agl, engedy: this is definitely not the webauthn CL above.

pmonette: There is a really clear jump in the benchmark at your CL in the bisect above. The regressions are pretty widespread (88 jumps on various pages and devices). Can you take a look? The pinpoint job above produced traces before and after your CL:

before: https://console.developers.google.com/m/cloudstorage/b/chrome-telemetry-output/o/text_hover_50000_pixels_per_second_2018-04-11_16-30-02_98246.html
after: https://console.developers.google.com/m/cloudstorage/b/chrome-telemetry-output/o/text_hover_50000_pixels_per_second_2018-04-11_19-20-58_36223.html

+benchmark owners bokan and nzolghadr in case they have thoughts here.

Thanks for adding the benchmark owners!

I have a few questions about the benchmark.
Is my intuition correct that this is most probably caused by tasks running on the UI thread? Background tasks should not influence the scrolling?
Is this test done during startup or after?

> Is my intuition correct that this is most probably caused by tasks running on the UI thread? Background tasks should not influence the scrolling?

Yes, the scroll itself occurs in the renderer process.

> Is this test done during startup or after?

After, we wait until the page is loaded then begin scrolling.

Despite owning it (the scrolling side, anyway) I don't have a deep understanding of this benchmark. That said, I believe thread_times is measuring amount of CPU time spent in each generated frame in various threads over the scroll. It looks like this regressed thread_times_all, which is a combination of some bucket of threads but I'm not sure which. In that case, doing a significant amount of work on any thread this is measuring would cause this to regress. 

One thing to note, the traces sullivan@ attached don't have much detail. You can get a much clearer picture by running the benchmark locally, with and without your patch, while capturing a trace (I think this benchmark automatically captures a trace). Use --extra-chrome-categories="*" in run_benchmarks to specify all the detail in the trace, there'll be a link to it on the results page.

 Issue 830788  has been merged into this issue.

 Issue 830656  has been merged into this issue.

This is probably caused by the expensive work that runs in the background.

My CL enables the inspection of loaded modules in the process, which requires disk IO and making calls to the Crypto API.

Unless the total time taken by the benchmark goes up, there is no reason to think that this background works is negatively impacting scrolling. The Windows scheduler should be smart enough to not prioritize the background thread while UI work is happening.

Hi Bokan,

I'd like to close this issue if we can confirm that the extra CPU work done in the background is not negatively impacting the benchmark.

This is blocking the launch of the feature to beta. Thanks!

Sorry for the delay.

One interesting point here is that the regression isn't absolute. It's turned the graphs bimodal - so either we regress the metric quite significantly or not at all. e.g.:
https://chromeperf.appspot.com/report?sid=b42a53b19a88f775f6e00cf8a3f979b832234a96b0d1ecea29b0315f0092e265&start_rev=544076&end_rev=557773
Do you know why this might be? Can we do something to make the measurement more consistent?

Another question I have is why this regresses the tough_scrolling_cases page set but not touch_compositing_cases. 

Intuitively, doing work on a separate thread seems like it should be ok to me since it looks like it's just eating spare cycles. If it were interfering with handling scroll gestures I'd expect to see regressions in other metrics which doesn't seem the case. But then that begs the question, why have this metric at all?

+chiniforooshan@ who I see is doing related work on these metrics and may have more insight. Do you know if a regression in thread_times_all without any other regression is worth worrying about?

 Issue 830680  has been merged into this issue.

> Do you know if a regression in thread_times_all without any other regression is worth worrying about?

+vmiura is a better person to ask this from.

Probably the metric exists to make sure that if we are doing more work during animation, it's intentional, not accidental (someone should look at the regression and won't fix it if it's intentional, like in this case). If the metric is causing too many false positives, we should probably get rid of it.

Closing this bug

Increased CPU time is totally expected from my feature. But the extra work is scheduled on a background thread and should not interfere with foreground work.

In addition, the extra work I am doing is a one time occurrence, that happens shortly after startup. A series of tasks is posted sequentially for each loaded DLL in the process (InspectModule() in module_info_win,h), and each tasks takes less than a second. It may seem long but keep in mind that this runs on a background task. Our metrics shows that the median amount of loaded DLLs for users in the wild is 120.

So unless we see more pref regressions that doesn't involve CPU time, this is working as intended.