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

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


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

chromium-rel-mac10

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

Hi sunnyps@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 : cc: Add mailbox support to ResourceProvider write locks.
Author  : sunnyps
Commit description:
  
This adds support for mailboxes to ScopedWriteLockGL. Using the mailbox
requires using ScopedTextureProvider/ScopedSkSurfaceProvider which
ensures that the texture id for the mailbox is destroyed after use on
the worker context.

This CL also includes the following cleanup:
1. ResourceProvider locks don't keep resource pointers around.
2. ScopedSamplerGL does not inherit from ScopedReadLockGL.
3. GpuRasterizer is folded back into GpuRasterBufferProvider.
4. TileTaskManager does not own RasterBufferProvider.

BUG= 525259 
R=piman@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/5fa5dbdf25bbec21b84f752d3f0642cd184467e2
Review-Url: https://codereview.chromium.org/1951193002
Cr-Original-Commit-Position: refs/heads/master@{#398204}
Cr-Commit-Position: refs/heads/master@{#399983}
Commit  : 3b0f0b8d3db0a9f66864d5b7da87c82f49e74a29
Date    : Wed Jun 15 19:09:57 2016


===== TESTED REVISIONS =====
Revision         Mean    Std Dev  N  Good?
chromium@399966  3426.0  233.433  5  good
chromium@399979  3532.8  28.2436  5  good
chromium@399982  3546.8  22.0839  5  good
chromium@399983  5091.4  14.8762  5  bad    <--
chromium@399984  4915.2  249.26   5  bad
chromium@399985  5097.0  15.1658  5  bad
chromium@399991  5075.6  21.1731  5  bad

Bisect job ran on: mac_10_10_perf_bisect
Bug ID: 621198

Test Command: src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --also-run-disabled-tests power.trivial_scroll
Test Metric: idle_wakeups_total/idle_wakeups_total
Relative Change: 48.15%
Score: 99.9

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


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

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

piman@, vmiura@ Can you think of a reason why adding an extra insert sync token (in OrderingBarrier) and extra waits (when the raster task runs on the worker context) can introduce extra wakeups?

An OrderingBarrier can cause a Flush (if the last OB was on a different context), which can cause extra IPCs/wakeups.

We had the OrderingBarrier before this CL as well - we just added an extra insert sync token there.

The OrderingBarrier may be elided I think if there were no commands since the last one.

That seems plausible. Another possibility is that we flush the compositor context unnecessarily (in CanWaitUnverifiedSyncToken) when we do WaitSyncToken on the worker context.

If it's the former I'll remove the sync token insert/wait for the non-async (default) case but it looks like we'll need a way to flush an entire chain of OrderingBarriers together.

Moving this nonessential bug to the next milestone.

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

 Issue 622222  has been merged into this issue.

 Issue 626523  has been merged into this issue.

 Issue 626517  has been merged into this issue.

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

commit 779233fed288858ffdd15762702dd91e0f14240c
Author: sunnyps <sunnyps@chromium.org>
Date: Fri Jul 08 19:28:16 2016

cc: Do not synchronize on deletion of non-internal resources.

Resources which do not belong to a particular ResourceProvider should
not require a WaitSyncToken. This is a speculative fix for the increased
idle wakeups seen on Mac non-overlay.

R=piman@chromium.org
BUG= 621198 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

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

[modify] https://crrev.com/779233fed288858ffdd15762702dd91e0f14240c/cc/resources/resource_provider.cc

Doesn't look like the above CL helped.

Actually the bot doesn't have any data newer than Jun 28 which includes the CL above. I've asked the perf sheriffs about this.

Issue 618677 has been merged into this issue.

+erikchen@, do you know what happened to the power.trivial_scroll benchmark? Was it renamed?

This became power.trivial_pages, which contains a scrolling page.

https://chromeperf.appspot.com/report?sid=8dc04fabe5168d3d2e49667cdb5c7ec76058c0d8a50f73f2e0b3c99a15cf682f

Perf sheriff ping

erikchen@, thanks for the insight. It looks like the above commit in #14 got lost in the transition between these two benchmarks. Do you have any recommendations for next steps with this bug? Should we close it?

sunnyps, erickchen: what's the status of this issue? Is there more work to be done here?

There's no perfect way to track regressions across renames/refactors.

The original bug has many regressions. Looking only at:
ChromiumPerf/chromium-rel-mac10/power.trivial_scroll / idle_wakeups_total

We see that idle wakeups jumped from ~3500 to ~5000. Looking at ChromiumPerf/chromium-rel-mac10/power.trivial_pages / idle_wakeups_total / TrivialScrollingPageSharedPageState

We see that idle wakeups drop from ~5000 to ~3500 around 07/02. Around the range 403595 - 403602. This doesn't quite match up with sunnyps@'s CL, but at least the regression appears to have recovered.