Issue metadata
Sign in to add a comment
|
15.4% regression in thread_times.simple_mobile_sites at 382134:382165 |
||||||||||||||||||||||
Issue descriptionLooks like a significant regression.
,
Mar 21 2016
I note it's only seems to affect the flickr page.
,
Mar 21 2016
=== Auto-CCing suspected CL author dalecurtis@chromium.org === Hi dalecurtis@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 : Reland of Fix typo in Android perf variations for unified media pipeline trial. (patchset #1 id:1 of https://codereview.chromium.org/1813553002/ ) Author : dalecurtis Commit description: Reason for revert: Reverting this revert now that internal bots should be good to go per jbudorick@. Original issue's description: > Revert of Fix typo in Android perf variations for unified media pipeline trial. (patchset #1 id:1 of https://codereview.chromium.org/1808803002/ ) > > Reason for revert: > This is breaking the media tests downstream. > > Original issue's description: > > Fix typo in Android perf variations for unified media pipeline trial. > > > > BUG=533190 > > TEST=none > > > > Committed: https://crrev.com/31d22b6338ac6ac8b95e9f4c20cdccea11609be2 > > Cr-Commit-Position: refs/heads/master@{#381498} > > TBR=asvitkine@chromium.org,dalecurtis@chromium.org > # Skipping CQ checks because original CL landed less than 1 days ago. > NOPRESUBMIT=true > NOTREECHECKS=true > NOTRY=true > BUG=533190 > > Committed: https://crrev.com/194b8a29bb5bf254b7c5dc4d4b3a7cd56318542a > Cr-Commit-Position: refs/heads/master@{#381585} TBR=asvitkine@chromium.org,khushalsagar@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=533190 Review URL: https://codereview.chromium.org/1816703003 Cr-Commit-Position: refs/heads/master@{#382165} Commit : e28faac6f4198be9f5dda7b3dd1ccf85b68d3dc5 Date : Sat Mar 19 01:26:41 2016 ===== TESTED REVISIONS ===== Revision Mean Value Std. Dev. Num Values Good? chromium@382133 1.739537 0.014532 5 good chromium@382150 1.743592 0.014384 5 good chromium@382158 1.732485 0.007871 5 good chromium@382162 1.740947 0.014399 5 good chromium@382164 1.722939 0.022172 5 good chromium@382165 2.914358 0.019509 5 bad Bisect job ran on: android_nexus6_perf_bisect Bug ID: 596468 Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --also-run-disabled-tests thread_times.simple_mobile_sites Test Metric: thread_renderer_compositor_cpu_time_per_frame/https___www.flickr.com_ Relative Change: 67.54% Score: 99.9 Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus6_perf_bisect/builds/2034 Job details: https://chromeperf.appspot.com/buildbucket_job_status/9017570562680215824 | 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 label Cr-Tests-AutoBisect. Thank you!
,
Mar 21 2016
Hmm, filed this as issue 596575 -- not sure why it wasn't linked from graphs. alexclarke@, do you know how this test works and what it's measuring exactly? liberato@ is looking into running some of these locally and watk@ was going to look at the other issue.
,
Mar 21 2016
Hmm, is this just counting the number of active threads? https://code.google.com/p/chromium/codesearch#chromium/src/tools/perf/measurements/thread_times.py -- if so this is expected to go up.
,
Mar 21 2016
,
Mar 21 2016
Looks like this is measuring cpu time for each thread and dividing it by the number of frames. It makes sense that we'd see Compositor thread cpu time go up slightly for Spitzer because now it will be handling BeginFrame callbacks, but I don't know if we expect it to go up by this much. I can take a look at a chrome trace.
,
Mar 21 2016
BeginFrame callbacks for video rendering, I mean.
,
Mar 21 2016
Is it actually playing a video?
,
Mar 21 2016
That's a good question. I finally got replay.py working with https and was able to inspect the page. It has four vp8 videos in it. They don't play until you click on them though.
,
Mar 21 2016
Is this test simulated over wifi or cellular? I.e. was media-extractor previously being used to load those clips? Are they preload=auto or preload=metadata? vp8 in particular means we'll spin up 1 blocking thread per video, 2-1=1 decode threads per video, 1 media thread total, and 1 audio thread total (if there is audio). These threads will largely be idle (and shutdown after the idle period) though if the videos are never played. The blocking thread is one that we should replace with a pool. Is there any thread in particular that we're adding a lot of cost to? You mention the compositor thread above.
,
Mar 22 2016
I'm going to cc the benchmark owners who can hopefully answer your questions. One thing that's surprising is it looks like somehow several potentially unrelated alerts got bundled into this one bug. I'm going to unbundle them and see what the bisects find.
,
Mar 22 2016
===== BISECT JOB RESULTS ===== Status: completed ===== SUSPECTED CL(s) ===== Subject : Reland of Fix typo in Android perf variations for unified media pipeline trial. (patchset #1 id:1 of https://codereview.chromium.org/1813553002/ ) Author : dalecurtis Commit description: Reason for revert: Reverting this revert now that internal bots should be good to go per jbudorick@. Original issue's description: > Revert of Fix typo in Android perf variations for unified media pipeline trial. (patchset #1 id:1 of https://codereview.chromium.org/1808803002/ ) > > Reason for revert: > This is breaking the media tests downstream. > > Original issue's description: > > Fix typo in Android perf variations for unified media pipeline trial. > > > > BUG=533190 > > TEST=none > > > > Committed: https://crrev.com/31d22b6338ac6ac8b95e9f4c20cdccea11609be2 > > Cr-Commit-Position: refs/heads/master@{#381498} > > TBR=asvitkine@chromium.org,dalecurtis@chromium.org > # Skipping CQ checks because original CL landed less than 1 days ago. > NOPRESUBMIT=true > NOTREECHECKS=true > NOTRY=true > BUG=533190 > > Committed: https://crrev.com/194b8a29bb5bf254b7c5dc4d4b3a7cd56318542a > Cr-Commit-Position: refs/heads/master@{#381585} TBR=asvitkine@chromium.org,khushalsagar@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=533190 Review URL: https://codereview.chromium.org/1816703003 Cr-Commit-Position: refs/heads/master@{#382165} Commit : e28faac6f4198be9f5dda7b3dd1ccf85b68d3dc5 Date : Sat Mar 19 01:26:41 2016 ===== TESTED REVISIONS ===== Revision Mean Value Std. Dev. Num Values Good? chromium@382133 2.492676 0.009525 5 good chromium@382150 2.50918 0.0258 5 good chromium@382158 2.520279 0.027588 5 good chromium@382162 2.539152 0.023919 5 good chromium@382164 2.601459 0.181115 5 good chromium@382165 3.299085 0.163244 5 bad Bisect job ran on: android_nexus6_perf_bisect Bug ID: 596468 Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --also-run-disabled-tests thread_times.simple_mobile_sites Test Metric: thread_GPU_cpu_time_per_frame/https___www.flickr.com_ Relative Change: 32.35% Score: 99.9 Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus6_perf_bisect/builds/2040 Job details: https://chromeperf.appspot.com/buildbucket_job_status/9017497544695866624 | 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 label Cr-Tests-AutoBisect. Thank you!
,
Mar 22 2016
This is similar to the memory increases where we're moving previously uncounted operations into Chrome, so some increase isn't unexpected here. We should verify that nothing crazy is happening though before moving on.
,
Mar 22 2016
Re #11: compositor thread cpu time is the metric that regressed (see #3): Test Metric: thread_renderer_compositor_cpu_time_per_frame/https___www.flickr.com_ The new bisect in #13 is for thread_GPU_cpu_time_per_frame. The videos are preload=auto. I haven't yet verified if we use the MediaExtractor path with WMPA but I expect so. This is on an N6, which AFAIK has a h/w vp8 decoder, so it's also not too surprising that the GPU thread cpu time went up. It's not obvious whether this is a net regression or not, because we're moving cpu time from Android threads to Chrome threads. I'll see if there's a way to see total cpu time.
,
Mar 22 2016
Since the videos aren't playing the compositor thread should only get 1 frame per video and then shut off. Given load a few ticks might occur, but they should do nothing since the video frame isn't changing. If you grab a trace and it shows an initial spike to render the first frame and bail, we can mark this as WontFix. However if the trace shows that we're continuing to tick the compositor or gpu after the initial frames we have a bug. There are media trace events for VideoFrameCompositor:Start/Stop and cc trace events for VideoFrameProviderClientImpl::OnBeginFrame.
,
Mar 29 2016
I grabbed a trace of this benchmark and everything looks as expected. There doesn't seem to be a device-wide cpu time metric, so we don't know how much cpu time MediaExtractor was using. I still don't know why compositor cpu time would go up. I'll keep looking at that one.
,
Apr 15 2016
,
May 18 2016
These have recovered or are only slightly elevated now. Some increase is expected if videos are present since we are now decoding in the GPU process versus decoding in some untracked Android process. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by alexclarke@chromium.org
, Mar 21 2016