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

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


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

android-galaxy-s5

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

Hi enne@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 : Optimize render passes with single quads
Author  : enne
Commit description:
  
Many effects (masks, replicas, filters) generate render passes for
simplicity in the code.  However, in the cases where the pass contains a
single quad with a texture, that resulting texture could just be used as
the input texture instead of the render pass itself.

The complication is mostly that render passes and tile textures are
flipped relative to each other (oops) and so some knowledge of this has
to leak into drawing render passes.

This is done by detecting such render passes inside of DirectRenderer,
storing the TileQuad that would have been drawn, skipping allocating the
pass and rendering it, and then calling a slightly modified version of
DrawRenderPassQuad with the TileQuad's resource.  The check for which
render passes can be supported is conservative to start.

This optimization usually will not be supported on mac because skia
does not support textures with texture rectangle targets as input.

BUG= 254639 , 606672 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/ff3dc65b0f7845184458fb25b3d566fa079cd232
Review-Url: https://codereview.chromium.org/1960543002
Cr-Original-Commit-Position: refs/heads/master@{#399060}
Cr-Commit-Position: refs/heads/master@{#399532}
Commit  : 4fd93e539bd8fdc3a42d1ba5f5bfe9acd9bbd949
Date    : Mon Jun 13 21:01:17 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev   N  Good?
chromium@399492  89.5657  5.0752    5  good
chromium@399513  85.6631  2.0769    5  good
chromium@399524  82.4362  0.620803  5  good
chromium@399529  81.7603  1.80432   5  good
chromium@399531  82.7468  0.26612   5  good
chromium@399532  73.1696  1.14578   5  bad    <--
chromium@399534  73.959   0.161235  5  bad

Bisect job ran on: android_s5_perf_bisect
Bug ID: 620324

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests blink_perf.bindings
Test Metric: append-child/append-child
Relative Change: 17.42%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_s5_perf_bisect/builds/719
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9009779673975493760


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

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

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


===== SUSPECTED CL(s) =====
Subject : Optimize render passes with single quads
Author  : enne
Commit description:
  
Many effects (masks, replicas, filters) generate render passes for
simplicity in the code.  However, in the cases where the pass contains a
single quad with a texture, that resulting texture could just be used as
the input texture instead of the render pass itself.

The complication is mostly that render passes and tile textures are
flipped relative to each other (oops) and so some knowledge of this has
to leak into drawing render passes.

This is done by detecting such render passes inside of DirectRenderer,
storing the TileQuad that would have been drawn, skipping allocating the
pass and rendering it, and then calling a slightly modified version of
DrawRenderPassQuad with the TileQuad's resource.  The check for which
render passes can be supported is conservative to start.

This optimization usually will not be supported on mac because skia
does not support textures with texture rectangle targets as input.

BUG= 254639 , 606672 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/ff3dc65b0f7845184458fb25b3d566fa079cd232
Review-Url: https://codereview.chromium.org/1960543002
Cr-Original-Commit-Position: refs/heads/master@{#399060}
Cr-Commit-Position: refs/heads/master@{#399532}
Commit  : 4fd93e539bd8fdc3a42d1ba5f5bfe9acd9bbd949
Date    : Mon Jun 13 21:01:17 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev   N  Good?
chromium@399492  97.847   0.192515  5  good
chromium@399513  86.7396  1.93224   8  good
chromium@399524  81.7196  3.02428   8  good
chromium@399529  83.1357  1.93957   5  good
chromium@399531  82.8779  0.548403  5  good
chromium@399532  67.187   4.02803   5  bad    <--
chromium@399534  73.4158  1.42102   5  bad

Bisect job ran on: android_s5_perf_bisect
Bug ID: 620324

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests blink_perf.bindings
Test Metric: append-child/append-child
Relative Change: 24.97%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_s5_perf_bisect/builds/720
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9009768301013339680


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

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

This doesn't really make any sense, as this is affecting the way part of the graphics stack in the compositor works and this is just a blink test testing dromaeo append/remove child stuff that doesn't exercise the optimization I added in my patch (verified via CHECK(false)).

It's also reland of a previous patch that landed on June 9th in r399060 and was then reverted in r399308, and so if this was truly a regression, I would have expected to see a similar dip across that time period.

I am also not able to reproduce this on my N4 locally with content shell.

git log 128c298a0f..e3d85c9 third_party/WebKit (which includes the data points before and after) shows very little going in WebKit that could affect this, and so I'm a bit lost as to where to hand this off to next.

I'm unassigning myself and CC-ing the current perf sheriff for opinions.

peria@ any ideas?

Looking at the performance dashboard, galaxy-s5 seems to be the only platform where this regression happened.
I suspect that this issue depends on the environment of the device, but I'll try if it actually happens on Galaxy S5.

Peria, any updates? You can use the perf try bots to run on S5 if you don't have one: https://www.chromium.org/developers/telemetry/performance-try-bots

peria, ping on this. Have you been able to investigate?

Hi, I'm sorry to leave this so long.
What I now know is that reverting the suspected CL does not fix the regression.
I hope someone takes this over.

Moving this nonessential bug to the next milestone.

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

Kicked off another bisect here.
https://chromeperf.appspot.com/buildbucket_job_status/9007412352885470416

peria@, can you recommend a better owner?

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


===== SUSPECTED CL(s) =====
Subject : Optimize render passes with single quads
Author  : enne
Commit description:
  
Many effects (masks, replicas, filters) generate render passes for
simplicity in the code.  However, in the cases where the pass contains a
single quad with a texture, that resulting texture could just be used as
the input texture instead of the render pass itself.

The complication is mostly that render passes and tile textures are
flipped relative to each other (oops) and so some knowledge of this has
to leak into drawing render passes.

This is done by detecting such render passes inside of DirectRenderer,
storing the TileQuad that would have been drawn, skipping allocating the
pass and rendering it, and then calling a slightly modified version of
DrawRenderPassQuad with the TileQuad's resource.  The check for which
render passes can be supported is conservative to start.

This optimization usually will not be supported on mac because skia
does not support textures with texture rectangle targets as input.

BUG= 254639 , 606672 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/ff3dc65b0f7845184458fb25b3d566fa079cd232
Review-Url: https://codereview.chromium.org/1960543002
Cr-Original-Commit-Position: refs/heads/master@{#399060}
Cr-Commit-Position: refs/heads/master@{#399532}
Commit  : 4fd93e539bd8fdc3a42d1ba5f5bfe9acd9bbd949
Date    : Mon Jun 13 21:01:17 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev   N  Good?
chromium@399492  96.5202  3.24051   5  good
chromium@399513  87.4175  0.335309  5  good
chromium@399524  82.6372  0.931507  5  good
chromium@399529  82.8537  0.716968  5  good
chromium@399531  82.3451  1.88772   5  good
chromium@399532  73.6804  0.861309  5  bad    <--
chromium@399534  74.1136  0.531709  5  bad

Bisect job ran on: android_s5_perf_bisect
Bug ID: 620324

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests blink_perf.bindings
Test Metric: append-child/append-child
Relative Change: 23.21%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_s5_perf_bisect/builds/784
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9007412352885470416


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

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

Dave, what should we do in a case like this? Bisect repeatedly points to a change but other factors make it impossible to be this one.

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

I think this metric is very sensitive to changes in Chrome overall. The value changes frequently, but it's hard to see that with the metric's noise level. This "regression" is a combination of a series of small regressions that happened in rapid succession.

On the graph we're looking for a change from ~95 → ~65. The bisect results have more iterations per revision, so we get a clearer picture of what's going on. I get the impression that enne's CL is only responsible for 82 → 73. Looks like there were additional regressions in the following ranges:

- r399492 → r399513 :: 97 → 86 runs/s
- r399513 → r399524 :: 86 → 82 runs/s
- r399532           :: 82 → 73 runs/s
- r399535 → r399595 :: 73 → ~65 runs/s (graph shows an additional regression after the bisection range)

Did this only regress on the S5? This benchmark is important, but mostly for sanity and comparison reasons. How fast does it run in Safari vs Chrome on a macbook? Does it work for a "reasonable" number of elements on the given hardware. The runs per second make it hard to understand that, also this benchmark is literally checking how fast we can call the appendChild bindings method, not the code under it because it's doing:

https://cs.chromium.org/chromium/src/third_party/WebKit/PerformanceTests/Bindings/append-child.html?q=append-child&sq=package:chromium&l=18

var div = document.createElement("div");
var childDiv = document.createElement("div");

for (var i = 0; i < 50000; i++)
    div.appendChild(childDiv);

but appendChild has this check:

if (newChild == m_lastChild) // nothing to do
    return newChild;

so this benchmark is actually doing the updating work of appendChild once, and then it's hitting that branch 49999 times and aborting early. So it's really just checking how fast we can cross the bindings calling a method and checking the types of the input argument.

Yeah, this only regressed on S5, other android bots are fine: https://chromeperf.appspot.com/report?sid=f2fbff0072896197b50b5f7eb14fe917863671d10186f373a442ad9e374c22a4&rev=399534

Perf sheriff ping

Since the alert only appeared on one bot (the S5), and the graph looks fairly noisy (https://chromeperf.appspot.com/group_report?bug_id=620324), I think that this is likely to have just been noise, so I'll close this one.