New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 596468 link

Starred by 0 users

Issue metadata

Status: WontFix
Owner:
Last visit > 30 days ago
Closed: May 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

15.4% regression in thread_times.simple_mobile_sites at 382134:382165

Project Member Reported by alexclarke@chromium.org, Mar 21 2016

Issue description

Looks like a significant regression.
 
All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=596468

Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg4LunpgoM


Bot(s) for this bug's original alert(s):

android-nexus6
I note it's only seems to affect the flickr page.
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Mar 21 2016

Cc: dalecur...@chromium.org
Owner: dalecur...@chromium.org

=== 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!
Cc: -dalecur...@chromium.org w...@chromium.org alexclarke@chromium.org liber...@chromium.org
Components: Internals>Media
Labels: Proj-Spitzer
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.
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.
Cc: -w...@chromium.org dalecur...@chromium.org
Owner: w...@chromium.org
Same as  issue 596575 .

Comment 7 by w...@chromium.org, 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.

Comment 8 by w...@chromium.org, Mar 21 2016

BeginFrame callbacks for video rendering, I mean.
Is it actually playing a video?

Comment 10 by w...@chromium.org, 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.
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.
Cc: vmi...@chromium.org
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.
Project Member

Comment 13 by 42576172...@developer.gserviceaccount.com, 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!
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.

Comment 15 by w...@chromium.org, 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.
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.

Comment 17 by w...@chromium.org, 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.
Cc: nyerramilli@chromium.org mustaq@chromium.org
 Issue 597275  has been merged into this issue.
Status: WontFix (was: Assigned)
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