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

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


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

chromium-rel-win7-gpu-nvidia

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

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


===== TESTED REVISIONS =====
Revision         Mean     Std Dev    N  Good?
chromium@409178  16.6368  0.103125   5  good
chromium@409185  16.5859  0.0957724  5  good
chromium@409188  16.5762  0.0666626  5  good
chromium@409190  16.5556  0.0354802  5  good
chromium@409191  17.3162  0.467605   5  bad

Bisect job ran on: winx64nvidia_perf_bisect
Bug ID: 633842

Test Command: .\src\out\Release_x64\performance_browser_tests.exe --test-launcher-print-test-stdio=always --enable-gpu
Test Metric: TabCapturePerformance_novsync/Capture
Relative Change: 4.08%
Score: 0

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/winx64nvidia_perf_bisect/builds/1776
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9005374916453808080


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

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

https://codereview.chromium.org/2199293002 This is the CL of 409191 that the bisect suspected.
This is skia roll and range is https://chromium.googlesource.com/skia.git/+log/d6dd44140d8d..320a40d77339

Let me assign reed@ who submit the CL in the range.

I don't see a Skia smoking gun in a Linux profile (the test seems to mostly draw paths to a Canvas2d - https://cs.chromium.org/chromium/src/chrome/test/data/extensions/api_test/tab_capture/performance.js):

+    1.66%  CompositorTileW  [.] AAConvexPathBatch::onPrepareDraws
+    0.94%  CompositorTileW  [.] SkPoint::normalize
+    0.57%  performance_bro  [.] SkPathRef::Editor::Editor
     0.31%  CompositorTileW  [.] SkPath::SkPath
     0.28%  performance_bro  [.] Convexicator::directionChange
     0.27%  performance_bro  [.] SkPath::SkPath
     0.27%  CompositorTileW  [.] GrPathUtils::QuadUVMatrix::set
     0.26%  CompositorTileW  [.] GrPathUtils::QuadUVMatrix::apply<6, 24ul, 8ul>
     0.24%  performance_bro  [.] Convexicator::addPt
     0.23%  CompositorTileW  [.] subdivide
     0.22%  CompositorTileW  [.] SkPath::Iter::consumeDegenerateSegments
     0.20%  CompositorTileW  [.] GrDrawBatch::renderTargetUniqueID
     0.19%  CompositorTileW  [.] SkConic::chopIntoQuadsPOW2
     0.19%  CompositorTileW  [.] SkGpuDevice::drawPath
     0.18%  performance_bro  [.] SkPath::Iter::consumeDegenerateSegments
     0.17%  CompositorTileW  [.] GrDrawTarget::recordBatch
     0.17%  performance_bro  [.] SkPath::internalGetConvexity
     0.16%  CompositorTileW  [.] SkConic::computeQuadPOW2
     0.16%  CompositorTileW  [.] SkCanvas::onDrawPath
     0.16%  CompositorTileW  [.] SkPath::internalGetConvexity
     0.15%  CompositorTileW  [.] update_degenerate_test

With a printf-instrumented build I do see some MakeBitmapShader calls, but they all take immutable bitmaps - so we're not expecting an impact.

OTOH the linked perf alerts (https://chromeperf.appspot.com/group_report?bug_id=633842) intersect exactly/suspiciously on r409191.  Still looking.

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

Perf sheriff ping

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

=== Auto-CCing suspected CL author reed@google.com ===

Hi reed@google.com, 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 : Always return ImageShader, even from SkShader::MakeBitmapShader
Author  : reed
Commit description:
  
Lessons learned

1. ImageShader (correctly) always compresses (typically via PNG) during serialization. This has the surprise results of
- if the image was marked opaque, but has some non-opaque pixels (i.e. bug in blitter or caller), then compressing may "fix" those pixels, making the deserialized version draw differently. bug filed.
- 565 compressess/decompresses to 8888 (at least on Mac), which draws differently (esp. under some filters). bug filed.

2. BitmapShader did not enforce a copy for mutable bitmaps, but ImageShader does (since it creates an Image). Thus the former would see subsequent changes to the pixels after shader creation, while the latter does not, hence the change to the BlitRow test to avoid this modify-after-create pattern. I sure hope this prev. behavior was a bug/undefined-behavior, since this CL changes that.

BUG=skia:5595
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2195893002

Review-Url: https://codereview.chromium.org/2195893002
Commit  : 320a40d7733979703bdf675c31108255e011e34e
Date    : Tue Aug 02 13:12:06 2016


===== TESTED REVISIONS =====
Revision                         Mean     Std Dev   N   Good?
chromium@409178                  22.4024  1.08354   12  good
chromium@409185                  21.4069  0.140371  5   good
chromium@409188                  21.3719  0.252038  5   good
chromium@409190                  21.7544  0.272174  5   good
chromium@409190,skia@320a40d773  23.4858  0.184642  5   bad    <--
chromium@409191                  24.2672  1.24341   8   bad

Bisect job ran on: winx64nvidia_perf_bisect
Bug ID: 633842

Test Command: .\src\out\Release_x64\performance_browser_tests.exe --test-launcher-print-test-stdio=always --enable-gpu
Test Metric: TabCapturePerformance_webrtc/CaptureSucceeded
Relative Change: 5.29%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/winx64nvidia_perf_bisect/builds/1929
Job details: https://chromeperf.appspot.com/buildbucket_job_status/8998327579577292656


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

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

The CL fixes a bug where, if the caller gave a mutable bitmap to create a shader, we now copy the pixels to enforce the shaders-always-draw-the-same contract.

In general blink/chrome already know to mark their bitmaps as immutable before drawing them. Does this benchmark hit this bug?

cc miu who owns these tests: can you answer reed's question in #11?

reed: Unfortunately, I don't know the answer to your question. These perf tests are browser-level tests that could regress in response to a number of things. The JavaScript is rendering into Canvas2D's, which the render process would then be uploading to the GPU and compositing. Then, the tab capture side of things would be asking the compositor to perform GPU read-back.

So, it's possible there are multiple places where mutable bitmaps might be accidentally used. How about instrumenting when the "mutable bitmap --> copy bitmap" code path is reached, to identify the calling code that is doing the wrong thing? From your code change, it looks like you would do this by adding the following "printf debugging" near this LOC (https://cs.chromium.org/chromium/src/third_party/skia/src/image/SkImage_Raster.cpp?rcl=1477077889&l=347):

  LOG(ERROR) << "MUTABLE bitmap passed to SkMakeImageFromRasterBitmap. Stack trace: \n"
             << base::debug::StackTrace().ToString();

(requires #include "base/debug/stack_trace.h")

reed@, have you had a chance to look into the suggestion in #13?

No update in 8 months, and graph is back down below regression levels. Marking WontFix.