New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 756101 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Jan 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

7.6%-11.5% regression in thread_times.key_idle_power_cases at 494001:494064

Project Member Reported by ellenpli@google.com, Aug 16 2017

Issue description

See the link to graphs below.
 
Project Member

Comment 1 by 42576172...@developer.gserviceaccount.com, Aug 16 2017

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

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


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

android-nexus5
android-nexus5X
android-nexus7v2
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Aug 16 2017

Cc: jkrcal@chromium.org
Owner: jkrcal@chromium.org
Status: Assigned (was: Untriaged)

=== Auto-CCing suspected CL author jkrcal@chromium.org ===

Hi jkrcal@chromium.org, the bisect results pointed to your CL, please take a look at the
results.


=== BISECT JOB RESULTS ===
Perf regression found with culprit

Suspected Commit
  Author : Jan Krcal
  Commit : 961edb38b6dc0099f7921e5ae612447852316898
  Date   : Mon Aug 14 12:01:42 2017
  Subject: [Favicons] Automatic eviction of on-demand favicons in bot config

Bisect Details
  Configuration: android_nexus5X_perf_bisect
  Benchmark    : thread_times.key_idle_power_cases
  Metric       : tasks_per_second_total_all/set-timeout.html (Long Idle)
  Change       : 5.56% | 3.16374447272 -> 3.33968652696

Revision             Result                    N
chromium@494029      3.16374 +- 0.0592908      6      good
chromium@494034      3.14402 +- 0.0611027      6      good
chromium@494035      3.15132 +- 0.128727       6      good
chromium@494036      3.42374 +- 0.150521       6      bad       <--
chromium@494038      3.37681 +- 0.119044       6      bad
chromium@494046      3.37505 +- 0.123617       6      bad
chromium@494063      3.33969 +- 0.0526236      6      bad

To Run This Test
  src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=set.timeout.html..Long.Idle. thread_times.key_idle_power_cases

More information on addressing performance regressions:
  http://g.co/ChromePerformanceRegressions

Debug information about this bisect:
  https://chromeperf.appspot.com/buildbucket_job_status/8971081320069425504


For feedback, file a bug with component Speed>Bisection

Comment 4 by jkrcal@chromium.org, Sep 18 2017

This is a code guarded by a feature flag. In its form it won't ship to Beta.
I am working on a CL to make the execution quicker.

I am missing the context what is the scenario my CL is slowing down. Is there some place I can learn more about the particular test thread_times.key_idle_power_cases?
The docs for the benchmark (a bit outdated) are here: http://www.chromium.org/developers/design-documents/rendering-benchmarks

You can use the "To Run This Test" command from the bisect to run locally. Looks like the regression is on Android only. More data about performance regression debugging here: http://g.co/ChromePerformanceRegressions

Note that features that are expected to launch are the ones that should be turned on in the testing file.

Comment 6 by jkrcal@chromium.org, Sep 18 2017

Thanks for the links!

We planned to launch but this regression (among other reasons) caused a delay. We are targeting M63, instead. I hope to improve the situation, in the meantime.
jkrcal: Ping on the status here?
Cc: sullivan@chromium.org skyos...@chromium.org
sullivan: I am sorry for the delay, it slipped through my radar, my bad :(

As promised in #6, I did changes in the implementation. I did not get to measuring perf impact, though. 
Now I created a test CL that disables the feature and measured the thread_times.key_idle_power_cases benchmark:
  https://chromium-review.googlesource.com/c/chromium/src/+/852532
The numbers look similar, tasks_per_second_total_all.set-timeout.html (Long Idle) benchmark shows again a regressions of +7.3%.

A bit of context on my culprit CL:
 - it adds a procedure (required by the privacy team) to do in the _first_ cycle of clearing history after startup, 
 - AFAIK, clearing history happens on Android also when backgrounded,
 - thus it makes sense it has impact on the 90s Long Idle test.

What is the meaning of this number? Does it mean the processor runs 7.3% more CPU time within there 90s of being idle? 
Can I extract the impact of this added procedure in absolute numbers (how many ms does it take)?
What is the state of the bot for this test, does it have any history / favicons?

I am not sure I can make the procedure perform better, I can definitely change the timing when it is run. Do you have any suggestions?
Sami, can you answer the questions in #8?
Friendly ping!
Status: WontFix (was: Assigned)
Sorry for the delay. I don't think we need to worry about this particular regression since it sounds like you're only adding one new task that runs some time after browser startup. This test is really meant to guard against severe problems where some subsystem starts staying running code perpetually, but unfortunately it's working more as a change detector because of the way alerting works (Is there a way to increase the threshold?)

And yes, tasks_per_second_total_all is simply counting how many tasks per second we observed during the 90s waiting period. The issue isn't as much the number of milliseconds used but the number of wakeups the CPU has to do.
Thanks for the update!

Sign in to add a comment