New issue
Advanced search Search tips

Issue 605080 link

Starred by 3 users
Status: Fixed
Owner:
jbroman@chromium.org
Closed: Apr 2016
Cc:
jbroman@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression

Blocking:
issue 607208



Sign in to add a comment

1.6%-3.2% regression in smoothness.tough_canvas_cases at 388236:388294

Project Member Reported by rmcilroy@chromium.org, Apr 20 2016 
See the link to graphs below.

Comment 1 by rmcilroy@chromium.org, Apr 20 2016 

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

Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICghLeMtAkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICghLOKtwoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgxLn_ugoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgxO3zvwoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgxKLpowkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgxK2nswsM


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

chromium-rel-mac-retina
chromium-rel-mac10
chromium-rel-win7-dual
chromium-rel-win7-gpu-ati
chromium-rel-win7-gpu-intel
chromium-rel-win7-gpu-nvidia
Project Member

Comment 2 by 42576172...@developer.gserviceaccount.com, Apr 21 2016

Cc: jbroman@chromium.org
Owner: jbroman@chromium.org

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

Hi jbroman@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 : WTF: Implement explicit RefPtr::operator bool.
Author  : jbroman
Commit description:
  
This is the modern alternative to the "safe bool" idiom. Some changes at
call sites are required, primarily because returning a boolean does not
cause an explicit conversion.

Review URL: https://codereview.chromium.org/1891473002

Cr-Commit-Position: refs/heads/master@{#388297}
Commit  : faf124b93463720518cae3ab3eb1388f1e3b418a
Date    : Tue Apr 19 20:25:14 2016


===== TESTED REVISIONS =====
Revision                Mean Value  Std. Dev.   Num Values  Good?
chromium@388216         1640.5146   9.288723    5           good
chromium@388260         1646.7272   4.199363    5           good
chromium@388282         1660.0178   7.598163    5           good
chromium@388293         1633.461    5.332462    5           good
chromium@388296         1630.593    3.228541    5           good
chromium@388297         1880.9914   7.63881     5           bad         <-
chromium@388298         1880.9144   6.70872     5           bad
chromium@388303         1892.0344   4.371841    5           bad

Bisect job ran on: android_nexus5_perf_bisect
Bug ID: 605080

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests blink_perf.dom
Test Metric: select-long-word/select-long-word
Relative Change: 15.33%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus5_perf_bisect/builds/3610
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9014760957246155744


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

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

Comment 3 by 42576172...@developer.gserviceaccount.com, Apr 21 2016 


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


===== SUSPECTED CL(s) =====
Subject : WTF: Implement explicit RefPtr::operator bool.
Author  : jbroman
Commit description:
  
This is the modern alternative to the "safe bool" idiom. Some changes at
call sites are required, primarily because returning a boolean does not
cause an explicit conversion.

Review URL: https://codereview.chromium.org/1891473002

Cr-Commit-Position: refs/heads/master@{#388297}
Commit  : faf124b93463720518cae3ab3eb1388f1e3b418a
Date    : Tue Apr 19 20:25:14 2016


===== TESTED REVISIONS =====
Revision                Mean Value  Std. Dev.   Num Values  Good?
chromium@388216         1641.6864   13.736169   5           good
chromium@388260         1634.931    8.12462     5           good
chromium@388282         1656.239    6.685915    5           good
chromium@388293         1629.0928   5.609623    5           good
chromium@388296         1632.2122   9.298318    5           good
chromium@388297         1882.2006   4.105413    5           bad         <-
chromium@388298         1882.083    5.824478    5           bad
chromium@388303         1888.396    9.556796    5           bad

Bisect job ran on: android_nexus5_perf_bisect
Bug ID: 605080

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests blink_perf.dom
Test Metric: select-long-word/select-long-word
Relative Change: 15.03%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus5_perf_bisect/builds/3611
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9014748466196618832


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

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

Comment 4 by jbroman@chromium.org, Apr 21 2016 

I'm a little mystified as to how this change had this effect, but the duplicate bisect agreed. :/

I'll look closer tomorrow.

Comment 5 by jbroman@chromium.org, Apr 22 2016 

I'm not sure what's up here. I'm going to speculatively revert, and see how the perf bots respond.

Comment 6 by danakj@chromium.org, Apr 22 2016 

Weird! If it's true maybe reland the changes outside of refptr separately, in pieces, and see if they do anything.
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 22 2016 

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

commit 1f735b48f731259f08b4fd7d46da0bfba5cb0911
Author: jbroman <jbroman@chromium.org>
Date: Fri Apr 22 18:59:15 2016

Revert of WTF: Implement explicit RefPtr::operator bool. (patchset #2 id:20001 of https://codereview.chromium.org/1891473002/ )

Reason for revert:
Possible cause of performance regressions, somehow?
https://bugs.chromium.org/p/chromium/issues/detail?id=605080

Original issue's description:
> WTF: Implement explicit RefPtr::operator bool.
>
> This is the modern alternative to the "safe bool" idiom. Some changes at
> call sites are required, primarily because returning a boolean does not
> cause an explicit conversion.

TBR=thakis@chromium.org,yutak@chromium.org,sigbjornf@opera.com
BUG= 605080 
# Not skipping CQ checks because original CL landed more than 1 days ago.

Review URL: https://codereview.chromium.org/1912203002

Cr-Commit-Position: refs/heads/master@{#389192}

[modify] https://crrev.com/1f735b48f731259f08b4fd7d46da0bfba5cb0911/third_party/WebKit/Source/core/css/LocalFontFaceSource.cpp
[modify] https://crrev.com/1f735b48f731259f08b4fd7d46da0bfba5cb0911/third_party/WebKit/Source/core/fetch/SubstituteData.h
[modify] https://crrev.com/1f735b48f731259f08b4fd7d46da0bfba5cb0911/third_party/WebKit/Source/core/inspector/NetworkResourcesData.h
[modify] https://crrev.com/1f735b48f731259f08b4fd7d46da0bfba5cb0911/third_party/WebKit/Source/core/layout/LayoutCounter.cpp
[modify] https://crrev.com/1f735b48f731259f08b4fd7d46da0bfba5cb0911/third_party/WebKit/Source/core/style/ComputedStyle.h
[modify] https://crrev.com/1f735b48f731259f08b4fd7d46da0bfba5cb0911/third_party/WebKit/Source/core/xmlhttprequest/XMLHttpRequest.cpp
[modify] https://crrev.com/1f735b48f731259f08b4fd7d46da0bfba5cb0911/third_party/WebKit/Source/platform/PODRedBlackTree.h
[modify] https://crrev.com/1f735b48f731259f08b4fd7d46da0bfba5cb0911/third_party/WebKit/Source/platform/fonts/FontDataForRangeSet.h
[modify] https://crrev.com/1f735b48f731259f08b4fd7d46da0bfba5cb0911/third_party/WebKit/Source/platform/fonts/SimpleFontData.h
[modify] https://crrev.com/1f735b48f731259f08b4fd7d46da0bfba5cb0911/third_party/WebKit/Source/platform/fonts/shaping/CachingWordShapeIterator.h
[modify] https://crrev.com/1f735b48f731259f08b4fd7d46da0bfba5cb0911/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp
[modify] https://crrev.com/1f735b48f731259f08b4fd7d46da0bfba5cb0911/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.h
[modify] https://crrev.com/1f735b48f731259f08b4fd7d46da0bfba5cb0911/third_party/WebKit/Source/platform/graphics/paint/DrawingDisplayItem.cpp
[modify] https://crrev.com/1f735b48f731259f08b4fd7d46da0bfba5cb0911/third_party/WebKit/Source/web/WebViewImpl.h
[modify] https://crrev.com/1f735b48f731259f08b4fd7d46da0bfba5cb0911/third_party/WebKit/Source/wtf/PassRefPtr.h
[modify] https://crrev.com/1f735b48f731259f08b4fd7d46da0bfba5cb0911/third_party/WebKit/Source/wtf/RefPtr.h
[modify] https://crrev.com/1f735b48f731259f08b4fd7d46da0bfba5cb0911/third_party/WebKit/Source/wtf/typed_arrays/ArrayBufferBuilder.h
[modify] https://crrev.com/1f735b48f731259f08b4fd7d46da0bfba5cb0911/third_party/WebKit/Source/wtf/typed_arrays/ArrayBufferView.h

Comment 9 by jbroman@chromium.org, Apr 27 2016

Blocking: 607208

Comment 10 by jbroman@chromium.org, Apr 27 2016

Status: Fixed (was: Assigned)
Investigation moved to  bug 607208 . Closing this.
►

Sign in to add a comment