New issue
Advanced search Search tips

Issue 629316 link

Starred by 2 users

Issue metadata

Status: WontFix
Owner:
Closed: Aug 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

10.1% regression in blink_perf.css at 405530:405568

Project Member Reported by m...@chromium.org, Jul 18 2016

Issue description

See the link to graphs below.
 

Comment 1 by m...@chromium.org, Jul 18 2016

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

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


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

android-nexus5

Comment 4 by m...@chromium.org, Jul 19 2016

Re-running bisect...
Project Member

Comment 5 by 42576172...@developer.gserviceaccount.com, Jul 20 2016

Cc: piman@chromium.org
Owner: piman@chromium.org

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

Hi piman@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 : Only create the GPUTracer outputer when actually tracing
Author  : piman
Commit description:
  
Creating the outputter needs to create and join a thread, which is quite costly.
This is to speed up the GPU fuzzer, which recreates a decoder (hence a
GPUTracer) for every test case.

BUG=None
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2151673003
Cr-Commit-Position: refs/heads/master@{#405561}
Commit  : 1146887683dd7092beedd535e14168cad580161d
Date    : Thu Jul 14 20:49:44 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N   Good?
chromium@405529  425.67   4.98532  12  good
chromium@405549  428.608  6.47877  8   good
chromium@405559  428.049  7.03247  18  good
chromium@405560  430.444  5.17492  5   good
chromium@405561  423.111  4.82729  18  bad    <--
chromium@405562  421.802  4.98065  18  bad
chromium@405564  424.278  5.53621  18  bad
chromium@405568  420.59   4.24311  12  bad

Bisect job ran on: android_nexus5_perf_bisect
Bug ID: 629316

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests blink_perf.css
Test Metric: CSSPropertySetterGetterMethods/CSSPropertySetterGetterMethods
Relative Change: 1.32%
Score: 95.0

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


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

| 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 6 by piman@chromium.org, Jul 20 2016

Owner: ----
Status: Available (was: Assigned)
I think the bisect is incorrect:
- the change has nothing to do with CSS, and actually changes code in another process (GPU process), and that shouldn't be used anyway (it's only used when enabling a disabled-by-default trace category)
- the perf change is within 1 stddev compared and way below the 10% regression
Project Member

Comment 8 by 42576172...@developer.gserviceaccount.com, Jul 21 2016

Cc: mstarzinger@chromium.org
Owner: mstarzinger@chromium.org

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

Hi mstarzinger@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 : [turbofan] Remove fallback to TurboFan when Crankshaft bails out.
Author  : mstarzinger
Commit description:
  
This removes the fallback path in question. Now the {AstNumbering} phase
is the only phase deciding whether Crankshaft is supposed to be disabled
or not. This in turn simplifies reasoning about the paths through the
compilation pipeline. We can decide early whether we want Ignition to
kick in depending on whether Crankshaft is enabled or not.

R=mvstanton@chromium.org,rmcilroy@chromium.org

Review-Url: https://codereview.chromium.org/2146573004
Cr-Commit-Position: refs/heads/master@{#37763}
Commit  : 8bad9474492c96906c57ee625db0e08080455719
Date    : Thu Jul 14 13:05:54 2016


===== TESTED REVISIONS =====
Revision                       Mean     Std Dev  N  Good?
chromium@405508                425.334  4.24085  5  good
chromium@405559                431.762  6.93355  5  good
chromium@405566                419.854  4.59721  5  good
chromium@405569                418.04   6.57014  5  good
chromium@405570                419.044  5.53265  5  good
chromium@405570,v8@d93fd41aaa  418.894  4.07642  5  good
chromium@405570,v8@8bad947449  391.243  1.70403  5  bad    <--
chromium@405570,v8@e7111cfff7  376.74   5.44839  5  bad
chromium@405570,v8@d0d99bee2b  383.899  7.71833  5  bad
chromium@405570,v8@fee5858391  389.249  3.35102  5  bad
chromium@405571                387.913  4.48885  5  bad
chromium@405572                387.72   5.24799  5  bad
chromium@405584                378.299  6.19332  5  bad
chromium@405609                398.925  2.34313  5  bad
chromium@405709                379.315  3.93187  5  bad

Bisect job ran on: android_nexus5_perf_bisect
Bug ID: 629316

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests blink_perf.css
Test Metric: CSSPropertySetterGetterMethods/CSSPropertySetterGetterMethods
Relative Change: 10.82%
Score: 99.9

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


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

| 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!
Friendly perf-sheriff ping, could your patch have caused this regression?
The patch doesn't seem strongly related, but the bisect looks pretty clean.

mstarzinger, can you take a look?
Cc: bmeu...@chromium.org danno@chromium.org
This could be related to my patch. We removed a fallback to TurboFan when Crankshaft bails out. This was needed to simplify the compilation pipeline sufficiently in order to setup the new Ignition staging pipeline. Maintaining this patch would complicate things a lot.

Also not that scrolling back on the perf graphs shows a corresponding bump when the oringinal optimization was landed in: https://crrev.com/4af7757fdf5199224f19a75ab060c03e86ee3fc5

My CL is essentially a "revert" of that optimization. I am very much voting to WontFix this.
Perf sheriff ping: reminder to follow up on possible performance issues
Status: WontFix (was: Available)

Sign in to add a comment