Issue metadata
Sign in to add a comment
|
7.6%-11.5% regression in thread_times.key_idle_power_cases at 494001:494064 |
||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Aug 16 2017
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8971081320069425504
,
Aug 16 2017
=== 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
,
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?
,
Sep 18 2017
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.
,
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.
,
Jan 5 2018
jkrcal: Ping on the status here?
,
Jan 8 2018
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?
,
Jan 8 2018
Sami, can you answer the questions in #8?
,
Jan 17 2018
Friendly ping!
,
Jan 17 2018
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.
,
Jan 17 2018
Thanks for the update! |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by 42576172...@developer.gserviceaccount.com
, Aug 16 2017