Issue metadata
Sign in to add a comment
|
100% improvement in thread_times.simple_mobile_sites at 417088:417296 |
||||||||||||||||||||
Issue descriptionLooks like this metric is broken. Maybe this was on purpose, maybe not. Lets see what the bisect bot finds.
,
Oct 18 2016
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8998466164505756720
,
Oct 19 2016
=== Auto-CCing suspected CL author danakj@chromium.org === Hi danakj@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 : cc: Merge the DrawLayer()/DrawFrame() and SwapBuffers() methods. Author : danakj Commit description: This deletes the SwapBuffers() method in LayerTreeHostImpl and in DelegatingRenderer, merging them into DrawLayers() and DrawFrame() in their respective classes. Next up is to collapse DelegatingRenderer into LayerTreeHostImpl and remove all "renderer" and "draw and swap" terminology in LayerTreeHostImpl, Scheduler, Proxies and friends. R=enne BUG= 606056 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel Review-Url: https://codereview.chromium.org/2317953002 Cr-Commit-Position: refs/heads/master@{#417098} Commit : aecfcfba7f2622ab3fa1691e939f76ca30a74f2d Date : Wed Sep 07 22:35:57 2016 ===== TESTED REVISIONS ===== Revision Mean Std Dev N Good? chromium@417087 1.94236 0.0400282 5 good chromium@417094 1.97155 0.030902 5 good chromium@417096 1.96774 0.0229711 5 good chromium@417097 1.9952 0.025431 5 good chromium@417098 0.0 0.0 5 bad <-- chromium@417101 0.0 0.0 5 bad chromium@417114 0.0 0.0 5 bad chromium@417140 0.0 0.0 5 bad chromium@417192 0.0 0.0 5 bad chromium@417296 0.0 0.0 5 bad Bisect job ran on: android_nexus7_perf_bisect Bug ID: 656947 Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests thread_times.simple_mobile_sites Test Metric: thread_renderer_compositor_cpu_time_per_frame/thread_renderer_compositor_cpu_time_per_frame Relative Change: 100.00% Score: 99.9 Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus7_perf_bisect/builds/3402 Job details: https://chromeperf.appspot.com/buildbucket_job_status/8998466164505756720 Not what you expected? We'll investigate and get back to you! https://chromeperf.appspot.com/bad_bisect?try_job_id=5241658847789056 | 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!
,
Oct 19 2016
Hey Dana It's possible your patch broke the thread_times telemetry benchmark. Would you mind taking a look?
,
Oct 20 2016
Hm so I deleted the "DelegatingRenderer::SwapBuffers" trace event but I don't see that referred to anywhere in the codebase so probably not that..
,
Oct 20 2016
Putting this in LTHI gives me non-0 results for thread_times.tough_scrolling_cases:
TRACE_EVENT0("benchmark", "DelegatingRenderer::SwapBuffers");
,
Oct 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7607802c76a4a0b826d22807213e981025ec734b commit 7607802c76a4a0b826d22807213e981025ec734b Author: danakj <danakj@chromium.org> Date: Sat Oct 22 02:21:05 2016 cc: Add back a benchmark category trace for counting renderer frames When deleting DelegatingRenderer::SwapBuffers I failed to notice that the TRACE_EVENT in there had the benchmark category. This category is used by perf tests, and without that event they had no way to count the number of renderer frames produced. This adds benchmark to the DrawLayers trace event in LayerTreeHostImpl instead and points the perf tests at that. Also removes the benchmark category from the GLRenderer and SoftwareRenderer classes as they are never used in the renderer and are not used for perf tests (tools/ no longer refers to the name "SwapBuffers" for perf tests). R=enne@chromium.org TBR=dtu BUG= 656947 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel Review-Url: https://chromiumcodereview.appspot.com/2442453003 Cr-Commit-Position: refs/heads/master@{#426952} [modify] https://crrev.com/7607802c76a4a0b826d22807213e981025ec734b/cc/output/gl_renderer.cc [modify] https://crrev.com/7607802c76a4a0b826d22807213e981025ec734b/cc/output/software_renderer.cc [modify] https://crrev.com/7607802c76a4a0b826d22807213e981025ec734b/cc/trees/layer_tree_host_impl.cc [modify] https://crrev.com/7607802c76a4a0b826d22807213e981025ec734b/tools/perf/metrics/timeline.py
,
Oct 25 2016
It worked thanks! Note I've assigned a "regression" to this bug (it's not a regression), the bisect bot will come and find your patch. Feel free to ignore that. I'm going to mark this as fixed. |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by alexclarke@chromium.org
, Oct 18 2016