Issue metadata
Sign in to add a comment
|
5.6%-135.4% regression in thread_times.key_idle_power_cases at 383185:383235 |
||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Mar 26 2016
=== Auto-CCing suspected CL author tedchoc@chromium.org === Hi tedchoc@chromium.org, the bisect results pointed to your CL below as possibly causing a regression. Please have a look at this info and see whether your CL be related. ===== BISECT JOB RESULTS ===== Status: completed ===== SUSPECTED CL(s) ===== Subject : Enable url path fading all on Android configurations. Author : tedchoc Commit description: Previously, this was restricted to document mode, but that is no longer desired and it should be enabled everywhere. BUG=597147 Review URL: https://codereview.chromium.org/1827183002 Cr-Commit-Position: refs/heads/master@{#383200} Commit : 328129ac349c860993a94ccb1a8944e2f500cc89 Date : Fri Mar 25 00:30:31 2016 ===== TESTED REVISIONS ===== Revision Mean Value Std. Dev. Num Values Good? chromium@383186 0.233137 0.002531 5 good chromium@383199 0.230297 0.003442 5 good chromium@383200 0.538801 0.007638 5 bad <- chromium@383201 0.540028 0.00055 5 bad chromium@383202 0.538988 0.003622 5 bad chromium@383205 0.540964 0.003519 5 bad chromium@383211 0.537979 0.002503 5 bad chromium@383235 0.539608 0.003815 5 bad Bisect job ran on: android_one_perf_bisect Bug ID: 598025 Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --also-run-disabled-tests thread_times.key_idle_power_cases Test Metric: thread_total_all_cpu_time_per_second/blank.html Relative Change: 131.45% Score: 99.9 Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_one_perf_bisect/builds/1229 Job details: https://chromeperf.appspot.com/buildbucket_job_status/9017190537850110112 Not what you expected? We'll investigate and get back to you! https://chromeperf.appspot.com/bad_bisect?try_job_id=598025 | O O | Visit http://www.chromium.org/developers/speed-infra/perf-bug-faq | X | for more information addressing perf regression bugs. For feedback, | / \ | file a bug with component Tests>AutoBisect. Thank you!
,
Mar 26 2016
What is this specifically testing? This change did add a new animation for the URL on every page load, so if we are expecting things to not happen after a page has been loaded, then I definitely think this is expected (granted the 135% is frightening). FWIW, this has already been the default behavior in document mode for all users on Android L+. Although, we could also argue that maybe we shouldn't animate on Android one. Adding rolfe@ for that one. Should we just not fade out the path on android one? Hide it instantly? Or just leave it as is for now?
,
Mar 27 2016
Guessing no Svelte device runs L + document mode that can conveniently dictate how this is handled? Barring that instant fade seems the way to go. If that looks horrible, leave up the whole path, but I rather deal with an instant fade if not too jarring to keep the omnibox clear for everything else in it.
,
Mar 28 2016
Adding skyostil, owner of thread_times.key_idle_power_cases, to answer the questions in #3. The documentation for the benchmark says "Measures timeline metrics for sites that should be idle in foreground and background scenarios. The metrics are per-second rather than per-frame". Docs for the benchmark link here, but I don't see thread_times explained: http://www.chromium.org/developers/design-documents/rendering-benchmarks The largest increase is 135% in android-one/thread_times.key_idle_power_cases / thread_total_all_cpu_time_per_second / blank.html: https://chromeperf.appspot.com/report?sid=cbe882a438825237fef872a39a1b7bc4fd8d54e5f1c49a392ba683e6b407a78b&rev=383235 Re #4: I don't know anything about document mode, but the android one svelte devices on the waterfall are running L (LMY47W).
,
Mar 28 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d070c0ede9bf1faea3a66d91b8f68014c6ae1f67 commit d070c0ede9bf1faea3a66d91b8f68014c6ae1f67 Author: tedchoc <tedchoc@chromium.org> Date: Mon Mar 28 22:41:23 2016 Optimize the URL path fading to avoid unnecessary draws when empty. BUG= 598025 Review URL: https://codereview.chromium.org/1842543002 Cr-Commit-Position: refs/heads/master@{#383587} [modify] https://crrev.com/d070c0ede9bf1faea3a66d91b8f68014c6ae1f67/chrome/android/java/src/org/chromium/chrome/browser/omnibox/UrlContainer.java
,
Mar 28 2016
Just hiding the path on svelte does look horrible without the animation. I did find that we were over-zealously animating when we didn't need to (i.e. no path to fade), so hopefully that handles most of this. Will monitor and either it will resolve itself. But if this is comparing a completely blank page with a path to be faded, then I do expect a regression of some amount.
,
Apr 7 2016
gentle ping..
,
Apr 14 2016
Issue 598318 has been merged into this issue.
,
Apr 14 2016
In general, this is a won't fix. Previously, this behavior was done on document mode and we are enabled it for tabbed mode as well. This just happened to be hit by that move. I did make some improvements by not animating unnecessarily -- as it had been for the last year for document mode :-/. But as this was approved by launch review, there isn't much else to do. Launch review bug to enable this all the places: https://bugs.chromium.org/p/chromium/issues/detail?id=597147 |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by sullivan@chromium.org
, Mar 25 2016