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

Issue 628115 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

6.6% regression in speedometer at 405124:405136

Project Member Reported by nikolaos@chromium.org, Jul 14 2016

Issue description

See the link to graphs below.
 
All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=628115

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


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

android-one
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Jul 14 2016


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


===== TESTED REVISIONS =====
Revision                       Mean     Std Dev  N  Good?
chromium@405123                12457.7  105.236  5  good
chromium@405123,v8@3861e51322  12414.2  112.26   5  good
chromium@405123,v8@fd420203ec  12396.0  75.6506  5  good
chromium@405124                13238.6  140.5    5  bad
chromium@405125                13227.2  117.459  5  bad
chromium@405127                13250.9  75.7515  5  bad
chromium@405130                13102.8  104.402  5  bad
chromium@405136                13301.4  126.596  5  bad

Bisect job ran on: android_one_perf_bisect
Bug ID: 628115

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests speedometer
Test Metric: React-TodoMVC/React-TodoMVC
Relative Change: 6.77%
Score: 0

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_one_perf_bisect/builds/1409
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9007175627516046128


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

| 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!
Please retrigger relevant bisection jobs because http://crbug.com/628214 got fixed.
Project Member

Comment 6 by 42576172...@developer.gserviceaccount.com, Jul 15 2016

Cc: mythria@chromium.org
Owner: mythria@chromium.org

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

Hi mythria@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 : [Interpreter] Collect type feedback for calls in the bytecode handler
Author  : mythria
Commit description:
  
Collect type feedback in the call bytecode handler. The current
implementation only collects feedback for JS function objects. The other
objects and Array functions do not collect any feedback. They will be
marked Megamorphic.

BUG= v8:4280 ,  v8:4780 
LOG=N

Review-Url: https://codereview.chromium.org/2122183002
Cr-Commit-Position: refs/heads/master@{#37700}
Commit  : fd420203ecb5abe427bda14aa896d90919d998fb
Date    : Wed Jul 13 08:00:23 2016


===== TESTED REVISIONS =====
Revision                       Mean     Std Dev  N  Good?
chromium@405123                12628.5  65.3652  5  good
chromium@405123,v8@3861e51322  12580.9  130.079  5  good
chromium@405123,v8@fd420203ec  13418.5  292.579  5  bad    <--
chromium@405124                13449.5  195.863  5  bad
chromium@405125                13475.4  248.511  5  bad
chromium@405127                13597.8  96.9174  5  bad
chromium@405130                13572.2  79.6067  5  bad
chromium@405136                13486.1  228.404  5  bad

Bisect job ran on: android_one_perf_bisect
Bug ID: 628115

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests speedometer
Test Metric: React-TodoMVC/React-TodoMVC
Relative Change: 6.79%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_one_perf_bisect/builds/1415
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9007088613514254944


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

| 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!
mythria, it looks like your change caused a regression. Please take a look!
The suspect cl adds more checks to the call bytecode handler to collect type feedback. So it causes regressions in benchmarks that have calls in tight loops. This feature is required for enabling OSR which is required to improve few of the benchamrks (see crbug/628122), though this benchmark might not benefit from OSR. 

I will look at this in future (in few weeks) to see if the call bytecode handler can be simplified further, so we can reduce regressions. I don't think we can avoid the regression fully, but may be reduce a little. 
Cc: seththompson@chromium.org
Any updates? +seth to weigh in on the tradeoff here
OSR is a Q3 OKR, and this regression is outweighed by the overall benefit of the interpreter. Probably something to take a look at again in the future, but this is definitely the right tradeoff.
Perf sheriff ping: reminder to follow up on possible performance issues
Perf sheriff ping
There were few improvements done to the CllIC in these cls: https://codereview.chromium.org/2342533002/, https://codereview.chromium.org/2325083003/. And the graphs also show improvement. There are no further improvements planned currently. 
Status: Fixed (was: Assigned)

Sign in to add a comment