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

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=d96e15e9c5be9d5c1f0ed6dc29adc6a43707466bd3f92ea2f7407d008aaa2655


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

chromium-rel-mac12

Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8972511559177703584

=== Auto-CCing suspected CL author dcastagna@chromium.org ===

Hi dcastagna@chromium.org, the bisect results pointed to your CL, please take a look at the
results.


=== BISECT JOB RESULTS ===
Perf regression found with culprit

Suspected Commit
  Author : Daniele Castagna
  Commit : b5168e0d1731a1e4b8e6da22bdac6d08f6ab41a2
  Date   : Thu Jul 27 17:15:53 2017
  Subject: viz: Align OutputSurface and BufferQueue in flight buffers.

Bisect Details
  Configuration: mac_10_12_perf_bisect
  Benchmark    : media.tough_video_cases_tbmv2
  Metric       : memory:chrome:all_processes:reported_by_chrome:gpu:effective_size_avg/video.html?src_tulip2.mp4_seek
  Change       : 27.50% | 25527945.3333 -> 32548778.6667

Revision             Result                       N
chromium@489905      25527945 +- 3893541          6      good
chromium@489962      26238805 +- 9.12506e-09      6      good
chromium@489990      26238805 +- 9.12506e-09      6      good
chromium@489997      26238805 +- 9.12506e-09      6      good
chromium@489999      25527945 +- 3893541          6      good
chromium@490000      26293419 +- 299130           6      good
chromium@490001      32186965 +- 1901233          6      bad       <--
chromium@490004      32538112 +- 73900.8          6      bad
chromium@490018      32548779 +- 58423.7          6      bad

To Run This Test
  src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=video.html.src.tulip2.mp4.seek media.tough_video_cases_tbmv2

More information on addressing performance regressions:
  http://g.co/ChromePerformanceRegressions

Debug information about this bisect:
  https://chromeperf.appspot.com/buildbucket_job_status/8972511559177703584


For feedback, file a bug with component Speed>Bisection

Is it possible not calling buffer_queue_->SwapBuffers in GpuSurfacelessBrowserCompositorOutputSurface made such a big difference on mac?

There should be a dip in media.tough_video_cases_tbmv2 when  crrev.com/2829543003 landed if that's the case though.

It has been a while since I've poked around in here, but IIRC we avoid allocating a backbuffer on Mac if we are using the CoreAnimation renderer. It may be that this is triggering allocation of a backbuffer. But that's just a guess.

DS is working on crrev.com/c/590155 that might fix this regression.

Should we not have reverted the change and then work on a fix?

The change that introduced the regression fixed a pretty bad bug where we were drawing on the front buffer sometimes.

https://chromium-review.googlesource.com/c/590155 should be ready to land as soon as Dongseong fixes the BufferQueue tests.

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

commit 28c6ec2bd253f0a6409bd31f7ae3abb01eb71063
Author: Dongseong Hwang <dongseong.hwang@intel.com>
Date: Thu Aug 17 17:20:52 2017

viz: optimize empty swap.

Do nothing for empty swap as much as possible.
This CL improves
1. BufferQueue::SwapBuffers() doesn't copy redundantly
2. Only BufferQueue::BindFramebuffer() makes |current_surface_| advance, which
makes it easy to understand |current_surface_| lifecycle.
3. Make empty swap use the same buffer. It may help the underline display
hardware reuse cache effectively. (e.g. Framebuffer compression [1])

[1] p145 https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-kbl-vol12-display.pdf

TEST=XDG_RUNTIME_DIR=/var/run/chrome ./wayland_rects_client --use-drm --size=512x512 --fullscreen
viz_unittests BufferQueueTest.CheckEmptySwap

Bug:  743277 ,  750919 
Change-Id: I73c9380f61c21bfea390584f1cc493e615681548
Reviewed-on: https://chromium-review.googlesource.com/590155
Commit-Queue: Dongseong Hwang <dongseong.hwang@intel.com>
Reviewed-by: David Reveman <reveman@chromium.org>
Reviewed-by: Dongseong Hwang <dongseong.hwang@intel.com>
Cr-Commit-Position: refs/heads/master@{#495205}
[modify] https://crrev.com/28c6ec2bd253f0a6409bd31f7ae3abb01eb71063/components/viz/service/display_embedder/buffer_queue.cc
[modify] https://crrev.com/28c6ec2bd253f0a6409bd31f7ae3abb01eb71063/components/viz/service/display_embedder/buffer_queue.h
[modify] https://crrev.com/28c6ec2bd253f0a6409bd31f7ae3abb01eb71063/components/viz/service/display_embedder/buffer_queue_unittest.cc

 Issue 750772  has been merged into this issue.

 Issue 750772  has been merged into this issue.

sandersd@, dcastagna@ could we check if it's fixed? It looks we can kick developer.gserviceaccount.com somehow..

Clicking on the first link at the top of this bug you get a list of all assigned regressions.

https://chromeperf.appspot.com/group_report?bug_id=750919

There you can browse and click through some of them to see of they have recovered (I would recommend to sort by "Abs Delta" and focus on the larger regressions first).

It does seem that some of the regressions _do_ recover around r495205, which is great! For example:

https://chromeperf.appspot.com/report?sid=78d96bc306155bc4a1c2e67be5255821c59481993eaa2509cf40df4b711f0ee2&start_rev=487021&end_rev=497350

So I would recommend to click around a few more of those and, if you're confident the regression has been fixed in all places, mark this as fixed.

If you find new regressions that appear to be unrelated (on other metrics, or did not recover after your fix) you can select them and click "triage" to create a new bug and follow up on them if needed.

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

commit 686bdb21cf78f55928bd812f6a6b67d037f5b48a
Author: Daniele Castagna <dcastagna@chromium.org>
Date: Thu Aug 31 20:02:48 2017

viz: optimize empty swap.

Do nothing for empty swap as much as possible.
This CL improves
1. BufferQueue::SwapBuffers() doesn't copy redundantly
2. Only BufferQueue::BindFramebuffer() makes |current_surface_| advance, which
makes it easy to understand |current_surface_| lifecycle.
3. Make empty swap use the same buffer. It may help the underline display
hardware reuse cache effectively. (e.g. Framebuffer compression [1])

[1] p145 https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-kbl-vol12-display.pdf

TEST=XDG_RUNTIME_DIR=/var/run/chrome ./wayland_rects_client --use-drm --size=512x512 --fullscreen
viz_unittests BufferQueueTest.CheckEmptySwap

TBR=dongseong.hwang@intel.com

(cherry picked from commit 28c6ec2bd253f0a6409bd31f7ae3abb01eb71063)

Bug:  743277 ,  750919 
Change-Id: I73c9380f61c21bfea390584f1cc493e615681548
Reviewed-on: https://chromium-review.googlesource.com/590155
Commit-Queue: Dongseong Hwang <dongseong.hwang@intel.com>
Reviewed-by: David Reveman <reveman@chromium.org>
Reviewed-by: Dongseong Hwang <dongseong.hwang@intel.com>
Cr-Original-Commit-Position: refs/heads/master@{#495205}
Reviewed-on: https://chromium-review.googlesource.com/646657
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#1042}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/686bdb21cf78f55928bd812f6a6b67d037f5b48a/components/viz/service/display_embedder/buffer_queue.cc
[modify] https://crrev.com/686bdb21cf78f55928bd812f6a6b67d037f5b48a/components/viz/service/display_embedder/buffer_queue.h
[modify] https://crrev.com/686bdb21cf78f55928bd812f6a6b67d037f5b48a/components/viz/service/display_embedder/buffer_queue_unittest.cc