Issue metadata
Sign in to add a comment
|
25.7%-50.5% regression in smoothness.key_mobile_sites_smooth at 387206:387253 |
||||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Apr 16 2016
=== Auto-CCing suspected CL author enne@chromium.org === Hi enne@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 : Hook up ui::Compositor to Display's BeginFrameSource Author : enne Commit description: This hooks up the first SurfaceFactoryClient to the real BeginFrameSource owned by the OnScreenDisplayClient. This is done by adding an OutputSurfaceClient::SetBeginFrameSource method. As the SurfaceDisplayOutputSurface is also a SurfaceFactoryClient, it can hand the real begin frame source directly to ui::Compositor's scheduler. This allows the removal of some of the browser vsync plumbing, but not all of it. Once the renderer compositors have been hooked up to use this path as well, then this and all of the VSyncObserver code can be ripped out. The BeginFrameSource is created by the BrowserCompositorOutputSurface which updates it based on vsync information that it receives. This BeginFrameSource is passed to the Display (via OutputSurfaceClient), which informs the SurfaceManager that the compositor using that Display should be driven by that BeginFrameSource. The SurfaceManager then informs the SurfaceDisplayOutputSurface about the BeginFrameSource, which passes it into the single thread proxy's cc::Scheduler for that ui::Compositor's instance. Plumbing! The path from SurfaceManager to SurfaceDisplayOutputSurface was added in https://codereview.chromium.org/1673783004, but the rest of the plumbing is new to this patch. CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel Review URL: https://codereview.chromium.org/1821863002 Cr-Commit-Position: refs/heads/master@{#387228} Commit : 19c108580b99c469ed192066310e3162bf8c196e Date : Thu Apr 14 03:37:18 2016 ===== TESTED REVISIONS ===== Revision Mean Value Std. Dev. Num Values Good? chromium@387214 15.4908 0.777584 5 good chromium@387227 15.453 1.549306 5 good chromium@387228 24.1972 1.108553 5 bad <- chromium@387229 23.2974 1.418229 5 bad chromium@387231 23.6124 1.8817 5 bad chromium@387234 23.4864 0.364056 5 bad chromium@387240 23.0484 0.625192 5 bad Bisect job ran on: android_nexus7_perf_bisect Bug ID: 604007 Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --also-run-disabled-tests scheduler.tough_scheduling_cases Test Metric: mean_main_thread_scroll_latency/mean_main_thread_scroll_latency Relative Change: 48.79% Score: 99.9 Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus7_perf_bisect/builds/2913 Job details: https://chromeperf.appspot.com/buildbucket_job_status/9015284077926644080 Not what you expected? We'll investigate and get back to you! https://chromeperf.appspot.com/bad_bisect?try_job_id=604007 | 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!
,
Apr 18 2016
enne, can you take a look at this? The bisect looks solid.
,
Apr 18 2016
Yup, am planning on it. :)
,
Apr 18 2016
,
Apr 18 2016
Looks like this should be addressed by https://codereview.chromium.org/1879833002/.
,
Apr 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6414a4526b77dd087581d8a9c1cec50affc66333 commit 6414a4526b77dd087581d8a9c1cec50affc66333 Author: sievers <sievers@chromium.org> Date: Fri Apr 22 21:33:20 2016 Android: Browser-side scheduling latency tweaks - Emit an immediate 'missed' BeginFrame when an BFObserver is added instead of waiting for next vsync - Remove unneeded requestRender() (=SetNeedsAnimate()) call The former helps both the browser compositor and display scheduler (now that it uses the same BeginFrameSource). Regarding the browser compositor: When the omnibox is hiding (or selection handles are showing) we need to browser composite with the correct scroll offset for a given renderer frame. Without this patch we run: ScrollBegin VSync SwapCompositorFrame VSync BrowserComposite Now we do: ScrollBegin VSync SwapCF MissedBF BrowserComposite VSync In the next interval, this works because if nothing changed yet, the scheduler will defer the BeginMainFrame until there is another change in the browser compositor tree (MODE_LATE). Calling SetNeedsAnimate()) from CompositorViewHolder when the top control offset changes is unnecessary, since this is a renderer-driven animation (causes an invalidation in the browser compositor tree), so will already cause the necessary UpdateLayerTreeHost(). Requesting SetNeedsAnimate() causes an extra BeginMainFrame() (SingleThreadProxy+scheduler are not particularly good at tracking these between needs_commit and needs_animate) and puts us into high latency mode otherwise. Fileed bug crbug.com/602489 to make this more robust in SingleThreadProxy and scheduler. TBR=dtrainor@chromium.org BUG= 591725 , 602489 , 602485 , 604007 Review URL: https://codereview.chromium.org/1879833002 Cr-Commit-Position: refs/heads/master@{#389248} [modify] https://crrev.com/6414a4526b77dd087581d8a9c1cec50affc66333/chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorViewHolder.java [modify] https://crrev.com/6414a4526b77dd087581d8a9c1cec50affc66333/content/browser/renderer_host/compositor_impl_android.cc
,
May 10 2016
Not all graphs have recovered, e.g., ChromiumPerf/android-nexus6/thread_times.tough_scrolling_cases / thread_total_all_cpu_time_per_frame
,
Jun 1 2016
Moving this nonessential bug to the next milestone. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 8 2016
Trying a bisect on ChromiumPerf/android-nexus5X/thread_times.tough_scrolling_cases to see if it's the same root cause.
,
Jul 8 2016
=== Auto-CCing suspected CL author enne@chromium.org === Hi enne@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 : Hook up ui::Compositor to Display's BeginFrameSource Author : enne Commit description: This hooks up the first SurfaceFactoryClient to the real BeginFrameSource owned by the OnScreenDisplayClient. This is done by adding an OutputSurfaceClient::SetBeginFrameSource method. As the SurfaceDisplayOutputSurface is also a SurfaceFactoryClient, it can hand the real begin frame source directly to ui::Compositor's scheduler. This allows the removal of some of the browser vsync plumbing, but not all of it. Once the renderer compositors have been hooked up to use this path as well, then this and all of the VSyncObserver code can be ripped out. The BeginFrameSource is created by the BrowserCompositorOutputSurface which updates it based on vsync information that it receives. This BeginFrameSource is passed to the Display (via OutputSurfaceClient), which informs the SurfaceManager that the compositor using that Display should be driven by that BeginFrameSource. The SurfaceManager then informs the SurfaceDisplayOutputSurface about the BeginFrameSource, which passes it into the single thread proxy's cc::Scheduler for that ui::Compositor's instance. Plumbing! The path from SurfaceManager to SurfaceDisplayOutputSurface was added in https://codereview.chromium.org/1673783004, but the rest of the plumbing is new to this patch. CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel Review URL: https://codereview.chromium.org/1821863002 Cr-Commit-Position: refs/heads/master@{#387228} Commit : 19c108580b99c469ed192066310e3162bf8c196e Date : Thu Apr 14 03:37:18 2016 ===== TESTED REVISIONS ===== Revision Mean Std Dev N Good? chromium@387100 34.9485 0.195519 5 good chromium@387175 35.0593 0.214291 5 good chromium@387213 35.1264 0.315226 5 good chromium@387223 34.8132 0.317377 8 good chromium@387226 35.0077 0.242574 8 good chromium@387227 34.9177 0.307028 5 good chromium@387228 35.8307 0.294107 5 bad <-- chromium@387232 36.1411 0.720547 5 bad chromium@387250 36.0933 0.519026 5 bad Bisect job ran on: android_nexus5X_perf_bisect Bug ID: 604007 Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests thread_times.tough_scrolling_cases Test Metric: thread_total_all_cpu_time_per_frame/thread_total_all_cpu_time_per_frame Relative Change: 3.28% Score: 99.8 Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus5X_perf_bisect/builds/315 Job details: https://chromeperf.appspot.com/buildbucket_job_status/9007707675812718640 Not what you expected? We'll investigate and get back to you! https://chromeperf.appspot.com/bad_bisect?try_job_id=5848655983017984 | 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!
,
Jul 13 2016
This issue has been moved once and is lower than Pri-1. Removing the milestone. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 5 2016
@enne, there are a few graphs that still haven't recovered. Do you think you could take another look at this? Thanks!
,
Aug 18 2016
Perf sheriff ping: reminder to follow up on possible performance issues
,
Oct 5 2016
,
Oct 6 2016
https://chromeperf.appspot.com/report?sid=61aed11e4b64655e83b7c97940f03e3f8066b852b6b8abe419635cd351f0b54f&start_rev=386450&end_rev=423363 This graph eventually regressed a bit more and recovered somewhat, then "improved" to 0, meaning that it stopped working on Sep 9 and nobody noticed (separate bug). enne, do you have something specific in mind that you still want to do for this bug? If not, this looks to me like it's as Fixed as it's going to get.
,
Oct 6 2016
Yeah, from my end, I think there's nothing else to be done here. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by rsch...@chromium.org
, Apr 15 2016