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

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

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


=== BISECT JOB RESULTS ===
Test failure found with culprit

Suspected Commit
  Author : khushalsagar
  Commit : de93e30d4aa72f1c1a5c88e2aafba0021697b351
  Date   : Fri Jun 23 23:03:46 2017
  Subject: cc: Move pre-decoding for checkerable images to image worker.

Bisect Details
  Configuration: android_one_perf_bisect
  Benchmark    : image_decoding.image_decoding_measurement
  Metric       : ImageDecoding_avg/image_decoding.html?gif

Revision             Exit Code      N
chromium@481985      0 +- N/A       2      good
chromium@482050      0 +- N/A       2      good
chromium@482058      0 +- N/A       2      good
chromium@482060      0 +- N/A       2      good
chromium@482061      0 +- N/A       2      good
chromium@482062      1 +- N/A       2      bad       <--
chromium@482066      1 +- N/A       2      bad
chromium@482082      1 +- N/A       2      bad
chromium@482114      1 +- N/A       2      bad

To Run This Test
  src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=image.decoding.html.gif image_decoding.image_decoding_measurement

Debug Info
  https://chromeperf.appspot.com/buildbucket_job_status/8975705938809402224

Is this bisect wrong?
  https://chromeperf.appspot.com/bad_bisect?try_job_id=6421282557526016


| 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 Speed>Bisection.  Thank you!

I'm not able to repro the failure locally on an android one device.

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

=== BISECT JOB RESULTS ===
Test failure found with culprit

Suspected Commit
  Author : khushalsagar
  Commit : de93e30d4aa72f1c1a5c88e2aafba0021697b351
  Date   : Fri Jun 23 23:03:46 2017
  Subject: cc: Move pre-decoding for checkerable images to image worker.

Bisect Details
  Configuration: android_one_perf_bisect
  Benchmark    : image_decoding.image_decoding_measurement
  Metric       : benchmark_duration/benchmark_duration

Revision             Exit Code      N
chromium@481984      0 +- N/A       2      good
chromium@482050      0 +- N/A       2      good
chromium@482059      0 +- N/A       2      good
chromium@482061      0 +- N/A       2      good
chromium@482062      1 +- N/A       2      bad       <--
chromium@482063      1 +- N/A       2      bad
chromium@482067      1 +- N/A       2      bad
chromium@482083      1 +- N/A       2      bad
chromium@482115      1 +- N/A       2      bad

To Run This Test
  src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=image.decoding.html.gif image_decoding.image_decoding_measurement

Debug Info
  https://chromeperf.appspot.com/buildbucket_job_status/8975691076775712096

Is this bisect wrong?
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5873843542949888


| 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 Speed>Bisection.  Thank you!

https://codereview.chromium.org/2960933002/ in review.

 Issue 737386  has been merged into this issue.

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

commit 83e99d0e9ce163a5b78e67d17768ed9034721ef4
Author: khushalsagar <khushalsagar@chromium.org>
Date: Thu Jun 29 01:59:07 2017

tools/benchmarks: Fix failures in image decoding measurements.

The test page uses requestAnimationFrames as a signal to rely on an
image decode completing after it has been painted by blink, which is
precisely what checker-imaging would avoid, i.e., blocking pushing
frames from blink on an image decode.

Since these tests solely want to measure the decode performance,
disable the feature for them.

BUG= 736793 
R=piman@chromium.org,nednguyen@google.com
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

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

[modify] https://crrev.com/83e99d0e9ce163a5b78e67d17768ed9034721ef4/cc/base/switches.cc
[modify] https://crrev.com/83e99d0e9ce163a5b78e67d17768ed9034721ef4/cc/base/switches.h
[modify] https://crrev.com/83e99d0e9ce163a5b78e67d17768ed9034721ef4/content/browser/gpu/compositor_util.cc
[modify] https://crrev.com/83e99d0e9ce163a5b78e67d17768ed9034721ef4/tools/perf/benchmarks/image_decoding.py

Issue 737333 has been merged into this issue.

Issue 739164 has been merged into this issue.

Please note that some memory regressions (up to 6-8 MiB) were also attributed to this change.

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

Could you please investigate those?

Its definitely not a regression. The change 482062 should only have an effect with checker-imaging turned on, which is what the bots run with.

I went back to revision 482061 and ran this benchmark with checker-imaging turned on and off, and with it turned on there is a ~5-7M improvement in cc:effective_size from discardable_memory for images, which is surprising because the feature is not expected to have an impact on memory. After the change in 482062, the value is consistent between checker-imaging turned on/off. Since the bots run with the flag on, it comes up as a regression.

My theory was that may be we are getting to decode less because before this change, the same image could be scheduled to decode on 2 threads, but that didn't look like the case with minor debugging I tried. I'm going to look a bit more to verify what the actual reason for this increase is. But its not a regression from what the default case is.

I've seen multiple cases of phantom memory regressions from changes in this area (example: 774088), mostly coming from timing issues since it affects the scheduling of decodes in the renderer. Since there isn't a regression from the default path of sync decoding, closing this as Fixed.