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

Issue 755253 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Sep 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Linux , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

36.8%-65.3% regression in smoothness.tough_canvas_cases at 493504:493679

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=755253

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


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

android-nexus6
chromium-rel-win10
chromium-rel-win7-dual
chromium-rel-win7-x64-dual
chromium-rel-win8-dual
Project Member

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

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

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

Hi xidachen@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 : Xida Chen
  Commit : 4113351ca0024d9bcb037257db25a77d2a452563
  Date   : Fri Aug 11 00:34:17 2017
  Subject: Use Canvas2DLayerBridge for software path in HTMLCanvas

Bisect Details
  Configuration: android_nexus6_perf_bisect
  Benchmark    : smoothness.tough_canvas_cases
  Metric       : frame_times/http___ie.microsoft.com_testdrive_Performance_LetItSnow_
  Change       : 68.67% | 21.2785341125 -> 35.8898436805

Revision             Result                   N
chromium@493562      21.2785 +- 1.24894       6      good
chromium@493591      21.3799 +- 0.515728      6      good
chromium@493606      21.3512 +- 1.2988        6      good
chromium@493613      21.4265 +- 1.11076       6      good
chromium@493614      21.1648 +- 0.474759      6      good
chromium@493615      35.3373 +- 1.22739       6      bad       <--
chromium@493617      35.6705 +- 2.08375       6      bad
chromium@493620      35.341 +- 2.28967        6      bad
chromium@493678      35.8898 +- 1.23257       6      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=http...ie.microsoft.com.testdrive.Performance.LetItSnow. 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/8971259284599452096


For feedback, file a bug with component Speed>Bisection
Cc: junov@chromium.org
cc: junov@, the bisect range is very narrow (pointing directly to my CL)
Status: WontFix (was: Assigned)
Can't repro locally, I have a window 7 machine locally, and my profiling shows that the current code is 6% faster than the code without that CL.
Status: Assigned (was: WontFix)
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 31 2017

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

commit d6f5cd725ddf5ebc6ec23b3976bc367aa2b08a09
Author: Xida Chen <xidachen@chromium.org>
Date: Thu Aug 31 01:37:10 2017

Remove the status for "Canvas2dImageChromium"

This CL removes the status for the "Canvas2dImageChromium" flag in the
RuntimeEnabledFeature.json5. This feature should be enabled on Mac only.

This CL fixes a regression here:
https://chromium-review.googlesource.com/c/chromium/src/+/612850
where we deprecate the usage of USE_IOSURFACE_FOR_2D_CANVAS.
So after the above CL, the Canvas2DLayerBridge will also use the
IOSurface code path whenever the "Canvas2dImageChromium" flag is on.

Bug:  755253 
Change-Id: I55d5edd5cf8ba14b8dc46856246ad4f35113faf6
Reviewed-on: https://chromium-review.googlesource.com/643149
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: Justin Novosad <junov@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498716}
[modify] https://crrev.com/d6f5cd725ddf5ebc6ec23b3976bc367aa2b08a09/content/public/common/content_features.cc
[modify] https://crrev.com/d6f5cd725ddf5ebc6ec23b3976bc367aa2b08a09/third_party/WebKit/Source/platform/RuntimeEnabledFeatures.json5

 Issue 762127  has been merged into this issue.
Labels: -Pri-2 OS-Linux OS-Mac Pri-1
Given the other bug, put it as P1
Project Member

Comment 10 by bugdroid1@chromium.org, Sep 6 2017

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

commit 269abb4c50e78fb570477022635210dbab7c25db
Author: Justin Novosad <junov@chromium.org>
Date: Wed Sep 06 00:35:48 2017

Fix performance regression in canvas to canvas drawImage

In Canvas2DLayerBridge::NewImageSnapshot the call to
notifyContentWillChange was causing the generation ID of the SkSurface
to be bumped, thus preventing GPU texture cache hits when the canvas is
not accelerated. The call to notifyContentWillChange is necessary to fix
a bug that only affects the GPU-accelerated case. So this fix is simply
to call notifyContentWillChange only when the canvas is accelerated

BUG= 755253 

Change-Id: I04e1dd2da4c70ad470f9fdd6bb4577cff6a7cea5
Reviewed-on: https://chromium-review.googlesource.com/651347
Commit-Queue: Xida Chen <xidachen@chromium.org>
Reviewed-by: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499817}
[modify] https://crrev.com/269abb4c50e78fb570477022635210dbab7c25db/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp

Owner: junov@chromium.org
Status: Assigned (was: Fixed)
Labels: Merge-Request-62
Project Member

Comment 14 by sheriffbot@chromium.org, Sep 7 2017

Labels: -Merge-Request-62 Hotlist-Merge-Approved Merge-Approved-62
Your change meets the bar and is auto-approved for M62. Please go ahead and merge the CL to branch 3202 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

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

Comment 15 by bugdroid1@chromium.org, Sep 7 2017

Labels: -merge-approved-62 merge-merged-3202
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/525c62c932137dae642f686e44ef9bf4dbe7ad3d

commit 525c62c932137dae642f686e44ef9bf4dbe7ad3d
Author: Justin Novosad <junov@chromium.org>
Date: Thu Sep 07 14:08:22 2017

Fix performance regression in canvas to canvas drawImage

In Canvas2DLayerBridge::NewImageSnapshot the call to
notifyContentWillChange was causing the generation ID of the SkSurface
to be bumped, thus preventing GPU texture cache hits when the canvas is
not accelerated. The call to notifyContentWillChange is necessary to fix
a bug that only affects the GPU-accelerated case. So this fix is simply
to call notifyContentWillChange only when the canvas is accelerated

BUG= 755253 
TBR=junov@chromium.org

(cherry picked from commit 269abb4c50e78fb570477022635210dbab7c25db)

Change-Id: I04e1dd2da4c70ad470f9fdd6bb4577cff6a7cea5
Reviewed-on: https://chromium-review.googlesource.com/651347
Commit-Queue: Xida Chen <xidachen@chromium.org>
Reviewed-by: Xida Chen <xidachen@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#499817}
Reviewed-on: https://chromium-review.googlesource.com/655277
Reviewed-by: Justin Novosad <junov@chromium.org>
Cr-Commit-Position: refs/branch-heads/3202@{#63}
Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098}
[modify] https://crrev.com/525c62c932137dae642f686e44ef9bf4dbe7ad3d/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp

Status: Fixed (was: Assigned)

Sign in to add a comment