New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 755229 link

Starred by 8 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug-Regression



Sign in to add a comment

48.8%-6075.5% regression in smoothness.tough_canvas_cases at 493104:493399

Project Member Reported by fmea...@chromium.org, Aug 14 2017

Issue description

See the link to graphs below.
 
Project Member

Comment 1 by 42576172...@developer.gserviceaccount.com, Aug 14 2017

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

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=d54243e1721550eff658c62241949aa7bb898fc4cc97148411ac27b79bc37cbd


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

chromium-rel-mac-retina
chromium-rel-mac11
chromium-rel-mac11-air
chromium-rel-mac11-pro
chromium-rel-mac12
chromium-rel-mac12-mini-8gb
chromium-rel-win10
chromium-rel-win7-dual
chromium-rel-win7-gpu-ati
chromium-rel-win7-gpu-intel
chromium-rel-win7-gpu-nvidia
chromium-rel-win7-x64-dual
chromium-rel-win8-dual
win-high-dpi
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Aug 14 2017

Cc: khushals...@chromium.org
Owner: khushals...@chromium.org
Status: Assigned (was: Untriaged)

=== 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 ===
Perf regression found with culprit

Suspected Commit
  Author : Khushal
  Commit : 1b8abc019a5b476d503bff7405120c44036799eb
  Date   : Thu Aug 10 05:16:17 2017
  Subject: cc/blink: Replace SkImageGenerator with PaintImageGenerator.

Bisect Details
  Configuration: win_perf_bisect
  Benchmark    : smoothness.tough_canvas_cases
  Metric       : frame_times/http___ie.microsoft.com_testdrive_Performance_FishIETank_Default.html
  Change       : 5477.44% | 17.6963674132 -> 987.0046

Revision             Result                   N
chromium@493209      17.6964 +- 0.51308       6      good
chromium@493243      18.1027 +- 0.747053      6      good
chromium@493248      17.5832 +- 0.396274      6      good
chromium@493250      17.7243 +- 0.839507      6      good
chromium@493251      1013.77 +- 266.899       6      bad       <--
chromium@493252      950.428 +- 210.94        6      bad
chromium@493260      1086.91 +- 365.243       6      bad
chromium@493277      963.718 +- 486.223       6      bad
chromium@493349      987.005 +- 182.44        6      bad

To Run This Test
  src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=http...ie.microsoft.com.testdrive.Performance.FishIETank.Default.html smoothness.tough_canvas_cases

More information on addressing performance regressions:
  http://g.co/ChromePerformanceRegressions

Debug information about this bisect:
  https://chromeperf.appspot.com/buildbucket_job_status/8971261467290599136


For feedback, file a bug with component Speed>Bisection
Project Member

Comment 4 by 42576172...@developer.gserviceaccount.com, Aug 14 2017

 Issue 755230  has been merged into this issue.
Debugging. Looking at the trace, we have cases where the main frame is taking ~130ms now. May be there is some case where we are accidentally doing the decode on the main thread.
Project Member

Comment 6 by 42576172...@developer.gserviceaccount.com, Aug 14 2017

 Issue 755298  has been merged into this issue.
Project Member

Comment 7 by 42576172...@developer.gserviceaccount.com, Aug 15 2017

 Issue 755312  has been merged into this issue.
Project Member

Comment 8 by 42576172...@developer.gserviceaccount.com, Aug 15 2017

 Issue 755292  has been merged into this issue.
Project Member

Comment 9 by 42576172...@developer.gserviceaccount.com, Aug 15 2017

Cc: charliea@chromium.org ellenpli@google.com
 Issue 755585  has been merged into this issue.
Cc: chrishtr@chromium.org vmp...@chromium.org
The change above removed caching of SkImages inside BitmapImage. Since skia's caching is reference based, it discards the decode once there are no references to an SkImage. For canvas cases, where the decoding is done on the main thread and relies on skia's caching, this is causing us to re-decode every frame.

Uploading a fix.
Labels: -Pri-2 Pri-1
Project Member

Comment 12 by 42576172...@developer.gserviceaccount.com, Aug 15 2017

 Issue 755587  has been merged into this issue.
Project Member

Comment 13 by 42576172...@developer.gserviceaccount.com, Aug 15 2017

Cc: ericrk@chromium.org
 Issue 755647  has been merged into this issue.
Project Member

Comment 15 by 42576172...@developer.gserviceaccount.com, Aug 15 2017

 Issue 755648  has been merged into this issue.
Project Member

Comment 16 by 42576172...@developer.gserviceaccount.com, Aug 15 2017

 Issue 755292  has been merged into this issue.
Project Member

Comment 17 by 42576172...@developer.gserviceaccount.com, Aug 15 2017

 Issue 755649  has been merged into this issue.
Project Member

Comment 18 by bugdroid1@chromium.org, Aug 16 2017

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

commit 78cf6d75aa98c913dfcf79455ba70318438613bf
Author: Khushal <khushalsagar@chromium.org>
Date: Wed Aug 16 03:57:39 2017

blink: Cache PaintImages in BitmapImage.

The caching of SkImages in BitmapImage was removed in [1]. This caching
is necessary for cases where images are rasterized/decoded on the main
thread (2d canvas). Since skia's decode caching is reference based, it
discards decodes once the SkImage is destroyed. This implies re-using
of the same uniqueID for an image will still cause multiple decodes.

This changes restores the previous behaviour by caching PaintImages
in BitmapImage and adds tests for it in BitmapImageTest.

[1]: https://chromium-review.googlesource.com/c/597322

Bug:  735741 ,  755229 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I76707822136568b743fc604b2bccaabd4e97d04c
Reviewed-on: https://chromium-review.googlesource.com/607568
Commit-Queue: Khushal <khushalsagar@chromium.org>
Reviewed-by: Vladimir Levin <vmpstr@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494690}
[modify] https://crrev.com/78cf6d75aa98c913dfcf79455ba70318438613bf/third_party/WebKit/Source/core/svg/graphics/SVGImage.cpp
[modify] https://crrev.com/78cf6d75aa98c913dfcf79455ba70318438613bf/third_party/WebKit/Source/core/svg/graphics/SVGImage.h
[modify] https://crrev.com/78cf6d75aa98c913dfcf79455ba70318438613bf/third_party/WebKit/Source/core/svg/graphics/SVGImageForContainer.cpp
[modify] https://crrev.com/78cf6d75aa98c913dfcf79455ba70318438613bf/third_party/WebKit/Source/core/svg/graphics/SVGImageForContainer.h
[modify] https://crrev.com/78cf6d75aa98c913dfcf79455ba70318438613bf/third_party/WebKit/Source/platform/DragImageTest.cpp
[modify] https://crrev.com/78cf6d75aa98c913dfcf79455ba70318438613bf/third_party/WebKit/Source/platform/graphics/AcceleratedStaticBitmapImage.cpp
[modify] https://crrev.com/78cf6d75aa98c913dfcf79455ba70318438613bf/third_party/WebKit/Source/platform/graphics/AcceleratedStaticBitmapImage.h
[modify] https://crrev.com/78cf6d75aa98c913dfcf79455ba70318438613bf/third_party/WebKit/Source/platform/graphics/BitmapImage.cpp
[modify] https://crrev.com/78cf6d75aa98c913dfcf79455ba70318438613bf/third_party/WebKit/Source/platform/graphics/BitmapImage.h
[modify] https://crrev.com/78cf6d75aa98c913dfcf79455ba70318438613bf/third_party/WebKit/Source/platform/graphics/BitmapImageTest.cpp
[modify] https://crrev.com/78cf6d75aa98c913dfcf79455ba70318438613bf/third_party/WebKit/Source/platform/graphics/DeferredImageDecoderTest.cpp
[modify] https://crrev.com/78cf6d75aa98c913dfcf79455ba70318438613bf/third_party/WebKit/Source/platform/graphics/GeneratedImage.cpp
[modify] https://crrev.com/78cf6d75aa98c913dfcf79455ba70318438613bf/third_party/WebKit/Source/platform/graphics/GeneratedImage.h
[modify] https://crrev.com/78cf6d75aa98c913dfcf79455ba70318438613bf/third_party/WebKit/Source/platform/graphics/Image.cpp
[modify] https://crrev.com/78cf6d75aa98c913dfcf79455ba70318438613bf/third_party/WebKit/Source/platform/graphics/Image.h
[modify] https://crrev.com/78cf6d75aa98c913dfcf79455ba70318438613bf/third_party/WebKit/Source/platform/graphics/ImageLayerChromiumTest.cpp
[modify] https://crrev.com/78cf6d75aa98c913dfcf79455ba70318438613bf/third_party/WebKit/Source/platform/graphics/PlaceholderImage.cpp
[modify] https://crrev.com/78cf6d75aa98c913dfcf79455ba70318438613bf/third_party/WebKit/Source/platform/graphics/PlaceholderImage.h
[modify] https://crrev.com/78cf6d75aa98c913dfcf79455ba70318438613bf/third_party/WebKit/Source/platform/graphics/StaticBitmapImage.cpp
[modify] https://crrev.com/78cf6d75aa98c913dfcf79455ba70318438613bf/third_party/WebKit/Source/platform/graphics/StaticBitmapImage.h
[modify] https://crrev.com/78cf6d75aa98c913dfcf79455ba70318438613bf/third_party/WebKit/Source/platform/graphics/UnacceleratedStaticBitmapImage.cpp
[modify] https://crrev.com/78cf6d75aa98c913dfcf79455ba70318438613bf/third_party/WebKit/Source/platform/graphics/UnacceleratedStaticBitmapImage.h

Project Member

Comment 20 by 42576172...@developer.gserviceaccount.com, Aug 16 2017


=== BISECT JOB RESULTS ===
Perf regression found with culprit

Suspected Commit
  Author : Khushal
  Commit : 1b8abc019a5b476d503bff7405120c44036799eb
  Date   : Thu Aug 10 05:16:17 2017
  Subject: cc/blink: Replace SkImageGenerator with PaintImageGenerator.

Bisect Details
  Configuration: win_perf_bisect
  Benchmark    : smoothness.tough_canvas_cases
  Metric       : frame_times/http___ie.microsoft.com_testdrive_Performance_FishIETank_Default.html
  Change       : 3329.50% | 28.4336452054 -> 975.13205

Revision             Result                   N
chromium@493209      28.4336 +- 0.699549      6      good
chromium@493243      28.7096 +- 1.2006        5      good
chromium@493248      28.7541 +- 1.22793       6      good
chromium@493250      28.8479 +- 3.24021       6      good
chromium@493251      1046.1 +- 223.792        6      bad       <--
chromium@493252      1007.84 +- 504.687       6      bad
chromium@493260      1028.04 +- 255.819       6      bad
chromium@493277      954.07 +- 283.541        6      bad
chromium@493349      975.132 +- 280.674       6      bad

To Run This Test
  src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=http...ie.microsoft.com.testdrive.Performance.FishIETank.Default.html smoothness.tough_canvas_cases

More information on addressing performance regressions:
  http://g.co/ChromePerformanceRegressions

Debug information about this bisect:
  https://chromeperf.appspot.com/buildbucket_job_status/8971126547584767232


For feedback, file a bug with component Speed>Bisection
Project Member

Comment 22 by 42576172...@developer.gserviceaccount.com, Aug 16 2017

Cc: zakerinasab@chromium.org
Owner: zakerinasab@chromium.org

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

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


=== BISECT JOB RESULTS ===
Perf regression found with culprit

Suspected Commit
  Author : Reza.Zakerinasab
  Commit : 037613aa74af94a82dce77e1e246f1f5364c0cfa
  Date   : Mon Aug 14 16:26:25 2017
  Subject: Respect color correct rendering mode in canvas 2D

Bisect Details
  Configuration: mac_10_11_perf_bisect
  Benchmark    : smoothness.tough_canvas_cases
  Metric       : frame_times/http___ie.microsoft.com_testdrive_Performance_AsteroidBelt_Default.html
  Change       : 86.89% | 127.326679487 -> 16.6864398629

Revision             Result                    N
chromium@494039      127.327 +- 0.99312        6      good
chromium@494060      127.369 +- 1.4409         6      good
chromium@494066      127.303 +- 0.592416       6      good
chromium@494067      16.6987 +- 0.025565       6      bad       <--
chromium@494068      16.6846 +- 0.0299154      6      bad
chromium@494069      16.6953 +- 0.0631155      6      bad
chromium@494071      16.6903 +- 0.0209305      6      bad
chromium@494081      16.704 +- 0.0571255       6      bad
chromium@494122      16.6864 +- 0.0224319      6      bad

To Run This Test
  src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=http...ie.microsoft.com.testdrive.Performance.AsteroidBelt.Default.html smoothness.tough_canvas_cases

More information on addressing performance regressions:
  http://g.co/ChromePerformanceRegressions

Debug information about this bisect:
  https://chromeperf.appspot.com/buildbucket_job_status/8971069824853670400


For feedback, file a bug with component Speed>Bisection
Project Member

Comment 23 by 42576172...@developer.gserviceaccount.com, Aug 16 2017

 Issue 756167  has been merged into this issue.
Project Member

Comment 26 by 42576172...@developer.gserviceaccount.com, Aug 17 2017

Owner: khushals...@chromium.org

=== 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 ===
Perf regression found with culprit

Suspected Commit
  Author : Khushal
  Commit : 78cf6d75aa98c913dfcf79455ba70318438613bf
  Date   : Wed Aug 16 03:57:39 2017
  Subject: blink: Cache PaintImages in BitmapImage.

Bisect Details
  Configuration: android_nexus5X_perf_bisect
  Benchmark    : system_health.memory_mobile
  Metric       : memory:chrome:all_processes:reported_by_os:system_memory:proportional_resident_size_avg/browse_media/browse_media_flickr_infinite_scroll
  Change       : 5.92% | 220848486.667 -> 207775761.333

Revision             Result                     N
chromium@494674      220848487 +- 1589517       6      good
chromium@494684      227572071 +- 10439026      6      good
chromium@494689      230622567 +- 1328288       6      good
chromium@494690      210877287 +- 1312232       6      bad       <--
chromium@494691      210449767 +- 1525050       6      bad
chromium@494693      208925372 +- 1122440       6      bad
chromium@494711      207775761 +- 1083163       6      bad

Please refer to the following doc on diagnosing memory regressions:
  https://chromium.googlesource.com/chromium/src/+/master/docs/memory-infra/memory_benchmarks.md

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=browse.media.flickr.infinite.scroll system_health.memory_mobile

More information on addressing performance regressions:
  http://g.co/ChromePerformanceRegressions

Debug information about this bisect:
  https://chromeperf.appspot.com/buildbucket_job_status/8971036976208268496


For feedback, file a bug with component Speed>Bisection
There were the following regressions that were duped into this bug:

1) cpu_time_percentage_avg/load_games
2) system_health.memory_mobile/load_games
3) smoothness.tough_canvas_cases.frame_times/AsteroidBelt
4) system_health.memory_desktop/browse_media_imgr
5) system_health.memory_mobile/load_games
6) system_health.memory_mobile/browse_media_flickr

It looks like 494067 has caused an effect with recovering some of the canvas regressions for memory and smoothness. I'm not sure how exactly, may be something pertaining to how color correct rendering is affecting caching of decodes in skia?

The memory graphs for browse_media cases have recovered with 494690 for sure. And all the other smoothness benchmarks that were not affected by 494067.

There were a few other regressions that were duped to this bug with a regression range clearly after the suspected change here and I think are related to 494067. I've filed a bug and started a bisect for them ( issue 756572 ).
Status: Fixed (was: Assigned)
Project Member

Comment 29 by 42576172...@developer.gserviceaccount.com, Aug 18 2017

Cc: khushalsagar@google.com
 Issue 756572  has been merged into this issue.
Project Member

Comment 30 by 42576172...@developer.gserviceaccount.com, Aug 18 2017

Cc: chiniforooshan@chromium.org
 Issue 756871  has been merged into this issue.
Project Member

Comment 31 by 42576172...@developer.gserviceaccount.com, Aug 25 2017

Cc: kraynov@chromium.org
 Issue 758884  has been merged into this issue.
Project Member

Comment 32 by 42576172...@developer.gserviceaccount.com, Aug 29 2017

Cc: pmeenan@chromium.org
 Issue 760138  has been merged into this issue.
Project Member

Comment 33 by 42576172...@developer.gserviceaccount.com, Aug 30 2017

Cc: u...@chromium.org
 Issue 760521  has been merged into this issue.
 Issue 755586  has been merged into this issue.

Sign in to add a comment