New issue
Advanced search Search tips

Issue 624241 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2016
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug-Regression



Sign in to add a comment

12.3%-28.6% regression in blink_perf.css.FocusUpdate at 402362:402400

Project Member Reported by sullivan@chromium.org, Jun 29 2016

Issue description

See the link to graphs below.
 
Project Member

Comment 2 by sheriffbot@chromium.org, Jul 1 2016

Labels: -M-53 M-54 MovedFrom-53
Moving this nonessential bug to the next milestone.

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

Comment 4 by 42576172...@developer.gserviceaccount.com, Jul 11 2016


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


===== TESTED REVISIONS =====
Revision         Mean     Std Dev   N  Good?
chromium@402361  78.7436  1.56486   5  good
chromium@402374  75.8626  0.603658  5  good
chromium@402380  75.9758  0.651678  5  good
chromium@402383  76.6082  1.19783   5  good
chromium@402386  65.4559  0.441659  5  bad

Bisect job ran on: android_nexus7_perf_bisect
Bug ID: 624241

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: FocusUpdate/FocusUpdate
Relative Change: 16.87%
Score: 0

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus7_perf_bisect/builds/3064
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9007419399763560496


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

| 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!
Owner: wangxianzhu@chromium.org
Based on that bisect the range is 402384-402386, which makes it look like it;'s almost definitely b2fce5b3734ef52319ffe52f8c054a496d4e3fa4

wangxianzhu, can you please investigate?
The regression happened on Android only. Seems related to cache efficiency affected by changed of order of LayoutObject fields. Will try different orders.
Project Member

Comment 9 by 42576172...@developer.gserviceaccount.com, Jul 22 2016


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


===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N  Good?
chromium@402361  77.2464  1.52973  5  good
chromium@402374  75.7514  1.14558  5  good
chromium@402380  77.0751  1.77686  5  good
chromium@402383  76.5964  1.85584  5  good
chromium@402386  65.7528  1.04381  5  bad

Bisect job ran on: android_nexus7_perf_bisect
Bug ID: 624241

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: FocusUpdate/FocusUpdate
Relative Change: 14.88%
Score: 0

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus7_perf_bisect/builds/3089
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9006407463046571792


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

| 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 10 by 42576172...@developer.gserviceaccount.com, Jul 22 2016


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


===== TESTED REVISIONS =====
Revision         Mean     Std Dev   N  Good?
chromium@402379  118.696  4.39423   5  good
chromium@402382  118.718  2.52513   5  good
chromium@402383  119.181  0.443931  5  good
chromium@402386  100.551  3.21019   5  bad
chromium@402390  97.6081  3.2891    5  bad
chromium@402400  101.889  3.81724   5  bad

Bisect job ran on: android_nexus6_perf_bisect
Bug ID: 624241

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: FocusUpdate/FocusUpdate
Relative Change: 14.16%
Score: 0

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus6_perf_bisect/builds/2328
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9006407469635607312


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

| 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, any update on this?
Experimenting different code combinations on try bots. May determine to accept the regression of fix it in a week. 
Ping wangxianzhu@, it has been a week. Do you have a fix?
Labels: -Pri-2 Pri-3
Still looking at experimental results.
Experiment CLs:
https://codereview.chromium.org/2218763002/
https://codereview.chromium.org/2216873004/
https://codereview.chromium.org/2219523004/
etc.

Took more time because the results were not stable so had to repeat time-consuming try jobs to see the trend.

Lowered priority because I think the current performance of the benchmark is acceptable.
Project Member

Comment 16 by bugdroid1@chromium.org, Aug 6 2016

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

commit 6b109b4d67f5a3e18947b192f661f34f54e5f922
Author: wangxianzhu <wangxianzhu@chromium.org>
Date: Sat Aug 06 00:32:29 2016

Restore order of LayoutObject fields before crrev.com/402384

In crrev.com/402384 we changed the order of LayoutObject fields
to avoid memory gaps on 64-bit systems (which actually didn't work
because it just moves the gap to after the fields). It caused
performance regression of blink_perf.css.FocusUpdate on some
Android devices, perhaps because of changed cache behavior.

Restore the original LayoutObject field order.

BUG= 624241 

Review-Url: https://codereview.chromium.org/2220693002
Cr-Commit-Position: refs/heads/master@{#410229}

[modify] https://crrev.com/6b109b4d67f5a3e18947b192f661f34f54e5f922/third_party/WebKit/Source/core/layout/LayoutObject.cpp
[modify] https://crrev.com/6b109b4d67f5a3e18947b192f661f34f54e5f922/third_party/WebKit/Source/core/layout/LayoutObject.h

Status: Fixed (was: Assigned)
Summary: 12.3%-28.6% regression in blink_perf.css.FocusUpdate at 402362:402400 (was: 12.3%-28.6% regression in blink_perf.css at 402362:402400)
Performance recovered by about half after the CL.

I'm not going to put more effort on this because blink_perf.css.FocusUpdate was the only benchmark that was affected and the current performance is acceptable.

Sign in to add a comment