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

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


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

chromium-rel-mac-retina

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

Hi erikchen@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 : Re-enable WebGL Image Chromium.
Author  : erikchen
Commit description:
  
This CL makes DrawingBuffer use the newly added DescheduleUntilFinishedCHROMIUM,
and turns on IOSurface backed WebGL by default.

BUG= 581777 

Review-Url: https://codereview.chromium.org/2025973002
Cr-Commit-Position: refs/heads/master@{#397603}
Commit  : 801f15833c140f120a2f5baed88645cb0619ab86
Date    : Fri Jun 03 02:54:33 2016


===== TESTED REVISIONS =====
Revision         Mean    Std Dev  N  Good?
chromium@397576  9990.0  7.07107  5  good
chromium@397592  9988.0  4.47214  5  good
chromium@397600  9988.0  8.3666   5  good
chromium@397602  9980.0  10.0     5  good
chromium@397603  6654.0  23.0217  5  bad    <--
chromium@397604  6650.0  18.7083  5  bad
chromium@397607  6678.0  13.0384  5  bad

Bisect job ran on: mac_retina_perf_bisect
Bug ID: 617249

Test Command: src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --also-run-disabled-tests oortonline
Test Metric: Total/http___oortonline.gl__run
Relative Change: 33.15%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/mac_retina_perf_bisect/builds/1320
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9010848857538681312


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

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

What is the actual regression? What is being measured? The best I can guess is that some artificial score is being lowered:
https://cs-staging.chromium.org/chromium/src/tools/perf/benchmarks/oortonline.py?q=oortonline&sq=package:chromium&dr=CSs&l=26

My CL implements throttling for WebGL, which is known to reduce artificial performance metrics, but improve real performance.

I'm observing a behavior difference between the Image CHROMIUM and non-Image CHROMIUM path. Turning off Image CHROMIUM while I investigate.

Thanks for continuing to investigate this Erik. Let me know if you need any help running the benchmark or interpreting its results.

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

commit 7843491fb012d3d3a37f77355b92d999da732bca
Author: erikchen <erikchen@chromium.org>
Date: Fri Jun 10 18:17:43 2016

WebGL: Three small fixes to Image CHROMIUM logic.

1. Use the newly added method DescheduleUntilFinishedCHROMIUM() to add back
pressure.
2. Update the color mask appropriately before calling clearFramebuffers().
3. Plumb gpuMemoryBufferId through to the mailbox.

BUG= 611805 ,  617249 ,  607130 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel

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

[modify] https://crrev.com/7843491fb012d3d3a37f77355b92d999da732bca/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
[modify] https://crrev.com/7843491fb012d3d3a37f77355b92d999da732bca/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp
[modify] https://crrev.com/7843491fb012d3d3a37f77355b92d999da732bca/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.h

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

commit 29277336774a776985a04c57fc6a61c2ac3ab4af
Author: erikchen <erikchen@chromium.org>
Date: Fri Jun 10 23:38:07 2016

Add tracing for DescheduleUntilFinishedCHROMIUM.

This makes it easier to track down problems with chrome://tracing. This should
have no functional effect.

BUG= 617249 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel

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

[modify] https://crrev.com/29277336774a776985a04c57fc6a61c2ac3ab4af/gpu/command_buffer/service/gles2_cmd_decoder.cc

I've got another bug fix coming down the line:
https://codereview.chromium.org/2056413003/

But even with that, I'm seeing a ~10-15% performance regression with Chromium Image (CI) vs Non-Chromium Image (NCI). (On an Intel HD 5000 MacBook Air).

The following configuration performs well.
(0) NCI. (glFinish on compositor context implicitly waits for WebGL context). 

The following configurations all perform ~10-15% worse than NCI.
(1) CI using DescheduleUntilFinished
(2) CI using a service side glFinish on the WebGLContext every frame.
(3) NCI using a service side glFinish  on the WebGLContext every frame.

Traces show that (1), (2), and (3) all have similar performance characteristics. (Basically, ~14ms are spent in DescheduleUntilFinished/glFinish, followed by ~10ms in CommandExecutor::PutChanged) Traces show that (0) looks similar, except the glFinish in the compositor context only takes about ~10ms!  Logging shows that oortonline is shoving thousands of commands through the command buffer every frame, and processing them requires ~1-4 microseconds on average, resulting in the ~6ms of processing time per frame. 

Here is my theory about what's happening:
(1), (2), and (3) intentionally make command processing and deschedule/finish mutually exclusive, on the premise that the former is GPU bound, and is intentionally being throttled by the latter.

(0) calls finish at a different time (later in the cycle of the frame), which allows the CPU-bound command processing to happen in parallel with the GPU work.

In hindsight, the concept of descheduling frame N+1 until the work for frame N has finished was incorrect. It breaks the pipeline, which consists of heavy CPU work (command processing) followed by heavy GPU work (drawing). Instead, I'm going to try descheduling frame N+2 until the work for frame N has finished.

I spoke with piman@. In an attempt to move towards a future where mac behaves more like other platforms, he prefers a solution that gets RAF backpressure to behave correctly. This means we will want to insert a sync token, and then pass a reference all the way to the gpu process, where it can wait on the sync token to delay the swap ack.

I tried out a small modification to DescheduleUntilFinished that makes it deschedule frame N+2, rather than frame N+1. It yielded a 5-10% improvement over non-Image CHROMIUM. Interesting, the DescheduleUntilFinished wait lined up exactly with ImageTransportSurfaceOverlayMac::ClientWait. Which implies that the new implementation of ImageTransportSurfaceOverlayMac::ClientWait also waits for other contexts to finish. I removed the call to DescheduleUntilFinished, and still saw a 5-10% improvement. I tested this new binary out on WebGL aquarium with all settings maxed. FPS dropped to 40, but did not continue to drop, so throttling is working as intended.

tldr: Removing DescheduleUntilFinished fixes the problems, since the new logic in ImageTransportSurfaceOverlayMac::SwapBuffersInternal will correctly wait on the WebGL context, even in the CA compositing path.

Great analysis. Thanks for continuing to push on this Erik.

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

commit 5bcbf89d9ccff7e123c0b195e26f0901b7e65eff
Author: erikchen <erikchen@chromium.org>
Date: Tue Jun 14 00:37:39 2016

Remove use of DescheduleUntilFinished from WebGL.

It's no longer necessary, since the new implementation of
ImageTransportSurfaceOverlayMac::ClientWait correctly waits for the WebGL
context's work to finish.

BUG= 617249 

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

[modify] https://crrev.com/5bcbf89d9ccff7e123c0b195e26f0901b7e65eff/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp

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

commit 89f08d55d12c4593a91094c1c0a29ea47fdff6d8
Author: erikchen <erikchen@chromium.org>
Date: Tue Jun 14 00:45:57 2016

Reenable WebGL Image Chromium.

The feature was disabled because of two bugs. Both have since been fixed.

BUG= 617249 ,  581777 

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

[modify] https://crrev.com/89f08d55d12c4593a91094c1c0a29ea47fdff6d8/content/child/runtime_features.cc

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

commit 7843491fb012d3d3a37f77355b92d999da732bca
Author: erikchen <erikchen@chromium.org>
Date: Fri Jun 10 18:17:43 2016

WebGL: Three small fixes to Image CHROMIUM logic.

1. Use the newly added method DescheduleUntilFinishedCHROMIUM() to add back
pressure.
2. Update the color mask appropriately before calling clearFramebuffers().
3. Plumb gpuMemoryBufferId through to the mailbox.

BUG= 611805 ,  617249 ,  607130 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel

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

[modify] https://crrev.com/7843491fb012d3d3a37f77355b92d999da732bca/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
[modify] https://crrev.com/7843491fb012d3d3a37f77355b92d999da732bca/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp
[modify] https://crrev.com/7843491fb012d3d3a37f77355b92d999da732bca/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.h

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

commit 29277336774a776985a04c57fc6a61c2ac3ab4af
Author: erikchen <erikchen@chromium.org>
Date: Fri Jun 10 23:38:07 2016

Add tracing for DescheduleUntilFinishedCHROMIUM.

This makes it easier to track down problems with chrome://tracing. This should
have no functional effect.

BUG= 617249 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel

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

[modify] https://crrev.com/29277336774a776985a04c57fc6a61c2ac3ab4af/gpu/command_buffer/service/gles2_cmd_decoder.cc

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

commit 5bcbf89d9ccff7e123c0b195e26f0901b7e65eff
Author: erikchen <erikchen@chromium.org>
Date: Tue Jun 14 00:37:39 2016

Remove use of DescheduleUntilFinished from WebGL.

It's no longer necessary, since the new implementation of
ImageTransportSurfaceOverlayMac::ClientWait correctly waits for the WebGL
context's work to finish.

BUG= 617249 

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

[modify] https://crrev.com/5bcbf89d9ccff7e123c0b195e26f0901b7e65eff/third_party/WebKit/Source/platform/graphics/gpu/DrawingBuffer.cpp

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

commit 89f08d55d12c4593a91094c1c0a29ea47fdff6d8
Author: erikchen <erikchen@chromium.org>
Date: Tue Jun 14 00:45:57 2016

Reenable WebGL Image Chromium.

The feature was disabled because of two bugs. Both have since been fixed.

BUG= 617249 ,  581777 

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

[modify] https://crrev.com/89f08d55d12c4593a91094c1c0a29ea47fdff6d8/content/child/runtime_features.cc

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

commit 3fb834bc086a1b617db81795475559cc4eb6b524
Author: erikchen <erikchen@chromium.org>
Date: Wed Jun 15 01:38:19 2016

Revert of Reenable WebGL Image Chromium. (patchset #2 id:20001 of https://codereview.chromium.org/2063803002/ )

Reason for revert:
Breaks 2 conformance tests:
https://bugs.chromium.org/p/chromium/issues/detail?id=620067

https://build.chromium.org/p/chromium.gpu.fyi/builders/Mac%2010.10%20Release%20%28Intel%29/builds/13588

Original issue's description:
> Reenable WebGL Image Chromium.
>
> The feature was disabled because of two bugs. Both have since been fixed.
>
> BUG= 617249 ,  581777 
>
> Committed: https://crrev.com/89f08d55d12c4593a91094c1c0a29ea47fdff6d8
> Cr-Commit-Position: refs/heads/master@{#399625}

TBR=kbr@chromium.org,piman@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 617249 ,  581777 

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

[modify] https://crrev.com/3fb834bc086a1b617db81795475559cc4eb6b524/content/child/runtime_features.cc

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

commit f5c146230498ed8e7c5b24d2361c0551415e368d
Author: erikchen <erikchen@chromium.org>
Date: Mon Jun 20 17:29:08 2016

[Reland 1] Reenable WebGL Image Chromium.

The feature was disabled because of two bugs. Both have since been fixed.

BUG= 617249 ,  581777 
TBR=piman@chromium.org, kbr@chromium.org

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

[modify] https://crrev.com/f5c146230498ed8e7c5b24d2361c0551415e368d/content/child/runtime_features.cc

This was fixed by c#19.

Great work Erik!

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

commit 63170daf2b1bc70388d39d8b3635058b8269e3ea
Author: erikchen <erikchen@chromium.org>
Date: Thu Jun 23 02:39:23 2016

Fix a bug in the implementation of DescheduleUntilFinishedCHROMIUM.

The logic for checking whether an executor was idle was only checking if all of
the commands submitted had been processed. This always returns false for
DescheduleUntilFinishedCHROMIUM, since there are unprocessed commands.

I added the functions HasPollingWork() and PerformPollingWork() to
CommandExecutor to separate DescheduleUntilFinishedCHROMIUM work from idle work.

BUG= 617249 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel

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

[modify] https://crrev.com/63170daf2b1bc70388d39d8b3635058b8269e3ea/gpu/command_buffer/service/command_executor.cc
[modify] https://crrev.com/63170daf2b1bc70388d39d8b3635058b8269e3ea/gpu/command_buffer/service/command_executor.h
[modify] https://crrev.com/63170daf2b1bc70388d39d8b3635058b8269e3ea/gpu/command_buffer/service/gles2_cmd_decoder.cc
[modify] https://crrev.com/63170daf2b1bc70388d39d8b3635058b8269e3ea/gpu/command_buffer/service/gles2_cmd_decoder.h
[modify] https://crrev.com/63170daf2b1bc70388d39d8b3635058b8269e3ea/gpu/command_buffer/service/gles2_cmd_decoder_mock.h
[modify] https://crrev.com/63170daf2b1bc70388d39d8b3635058b8269e3ea/gpu/command_buffer/service/gles2_cmd_decoder_passthrough.cc
[modify] https://crrev.com/63170daf2b1bc70388d39d8b3635058b8269e3ea/gpu/command_buffer/service/gles2_cmd_decoder_passthrough.h
[modify] https://crrev.com/63170daf2b1bc70388d39d8b3635058b8269e3ea/gpu/ipc/service/gpu_command_buffer_stub.cc