All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=627172

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


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

chromium-rel-mac10

=== 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 : Paint first frame faster, don't crash with no frames during EOS.
Author  : dalecurtis
Commit description:
  
Since we're no longer using the sink to paint the first frame, it's
easier to start painting the first frame as soon as we know it's
"a sure thing." Which is when we (1) have at least two frames
queued, or (2) the first frame has a timestamp >= start timestamp,
or (3) know that no more frames are coming.

As part of this, fixes a nullptr crash caused by deciding if EOS
should be triggered before removing frames for underflow, which
leaves painting with no frames.

Additionally adds a unique identifier to VideoFrames to resolve
incorrect caching issues that the above changes exposed.

BUG=614099
TEST=new unittest
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Review-Url: https://codereview.chromium.org/2007463005
Cr-Commit-Position: refs/heads/master@{#397468}
Commit  : 046faf603abbd7886bd01bc99078d716145d3680
Date    : Thu Jun 02 18:16:30 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev   N  Good?
chromium@397450  94.4219  0.560114  8  good
chromium@397465  94.6288  1.24834   8  good
chromium@397467  94.271   0.580381  5  good
chromium@397468  100.055  0.967632  5  bad    <--
chromium@397469  99.4719  1.14012   8  bad
chromium@397473  102.507  8.64138   5  bad
chromium@397480  98.057   1.28643   5  bad

Bisect job ran on: mac_10_10_perf_bisect
Bug ID: 627172

Test Command: src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --also-run-disabled-tests media.tough_video_cases_extra
Test Metric: seek/video.html?src_garden2_10s.ogv
Relative Change: 4.22%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/mac_10_10_perf_bisect/builds/2196
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9007408106091235056


Not what you expected? We'll investigate and get back to you!
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5828440108826624

| 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!

Hmm, I wonder if it's because it posts the size change / metadata callbacks ahead of the buffering state change; so the seek completion is delayed while metadata is handled.

+chcunningham if he has any opinions on what order size/opacity should be delivered relative to the "have metadata" -> "have enough" transition.

===== BISECT JOB RESULTS =====
Status: completed


===== SUSPECTED CL(s) =====
Subject : Paint first frame faster, don't crash with no frames during EOS.
Author  : dalecurtis
Commit description:
  
Since we're no longer using the sink to paint the first frame, it's
easier to start painting the first frame as soon as we know it's
"a sure thing." Which is when we (1) have at least two frames
queued, or (2) the first frame has a timestamp >= start timestamp,
or (3) know that no more frames are coming.

As part of this, fixes a nullptr crash caused by deciding if EOS
should be triggered before removing frames for underflow, which
leaves painting with no frames.

Additionally adds a unique identifier to VideoFrames to resolve
incorrect caching issues that the above changes exposed.

BUG=614099
TEST=new unittest
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Review-Url: https://codereview.chromium.org/2007463005
Cr-Commit-Position: refs/heads/master@{#397468}
Commit  : 046faf603abbd7886bd01bc99078d716145d3680
Date    : Thu Jun 02 18:16:30 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev   N  Good?
chromium@397450  94.093   0.723728  5  good
chromium@397465  94.127   0.589816  5  good
chromium@397467  94.089   0.595907  5  good
chromium@397468  100.596  2.65495   5  bad    <--
chromium@397469  97.524   1.40441   5  bad
chromium@397473  101.927  7.35605   5  bad
chromium@397480  98.541   0.641769  5  bad

Bisect job ran on: mac_10_10_perf_bisect
Bug ID: 627172

Test Command: src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --also-run-disabled-tests media.tough_video_cases_extra
Test Metric: seek/video.html?src_garden2_10s.ogv
Relative Change: 4.73%
Score: 99.5

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/mac_10_10_perf_bisect/builds/2197
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9007406660206611440


Not what you expected? We'll investigate and get back to you!
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5297788309471232

| 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!

Moving this nonessential bug to the next milestone.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

I think the opacity/size change callbacks don't seem to do a lot of blocking work - they just post to the main thread and return. I'm much less familiar with the work done by PaintSingleFrame - perhaps its more likely the cause of the delay?

What made you want to paint before reporting HAVE_ENOUGH? My hunch is that reporting HAVE_ENOUGH is extremely cheap, while painting maybe not so much. 

Also, it seems the code you moved up is only executed when the sink is not yet started? So I'm surprised that it would have a significant impact on seeking in general.

Yeah, I'm surprised this has any effect whatsoever, but I'll test the perf locally to compare the ordering of callbacks.

Hi dalecurtis@, any updates on this?

No, haven't gotten around to it yet.

Perf sheriff ping: reminder to follow up on possible performance issues

Perf sheriff ping

dalecurtis@, what's the next step here? Have you been able to test this locally yet? Just making sure this regression doesn't fall through the cracks. Thanks!

Been OOO the last few weeks, have some ideas that I had started before leaving -- will see about getting something in this week.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dee963030706f989b4fb7de973a916c02d2a2083

commit dee963030706f989b4fb7de973a916c02d2a2083
Author: dalecurtis <dalecurtis@chromium.org>
Date: Tue Oct 25 01:31:09 2016

Fix perf and paint issues with VRI::PaintSingleFrame.

This prevents repeated rendering of the first frame and ensures the
algorithm knows about the frame being painted so that future calls
to OnLastFrameDropped() don't muck with statistics incorrectly.

Results seem slightly mixed, but maybe the official perf bot will
provide stronger evidence. It's a good change regardless.

BUG= 627172 ,  656231 
TEST=perf tests.

Review-Url: https://codereview.chromium.org/2437623004
Cr-Commit-Position: refs/heads/master@{#427217}

[modify] https://crrev.com/dee963030706f989b4fb7de973a916c02d2a2083/media/filters/video_renderer_algorithm.cc
[modify] https://crrev.com/dee963030706f989b4fb7de973a916c02d2a2083/media/filters/video_renderer_algorithm.h
[modify] https://crrev.com/dee963030706f989b4fb7de973a916c02d2a2083/media/renderers/video_renderer_impl.cc
[modify] https://crrev.com/dee963030706f989b4fb7de973a916c02d2a2083/media/renderers/video_renderer_impl.h

Hmm, latest CL didn't show any changes. This CL fixed cases where we wouldn't display the right frame, so I suspect that we were previously skipping some frames that ended up with a duplicate pointer allocation. I think this is just a natural consequence of actually displaying those frames -- so WontFix.