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

Issue 708546 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

12.7%-14.5% regression in blink_perf.layout at 461671:461683

Project Member Reported by pmeenan@chromium.org, Apr 5 2017

Issue description

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

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


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

android-webview-nexus5X
Cc: jfernan...@igalia.com
Owner: jfernan...@igalia.com

=== Auto-CCing suspected CL author jfernandez@igalia.com ===

Hi jfernandez@igalia.com, 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 : jfernandez
  Commit : 45688c2bcf5aedeff66d2d427e099d5f65251849
  Date   : Tue Apr 04 11:04:23 2017
  Subject: [css-grid] Implementation of Baseline Self-Alignment

Bisect Details
  Configuration: android_webview_arm64_aosp_perf_bisect
  Benchmark    : blink_perf.layout
  Metric       : fixed-grid-lots-of-stretched-data/fixed-grid-lots-of-stretched-data
  Change       : 23.11% | 99.868098773 -> 76.7927411327

Revision             Result                  N
chromium@461670      99.8681 +- 49.0331      6      good
chromium@461677      104.727 +- 48.1027      6      good
chromium@461680      89.9935 +- 1.59632      9      good
chromium@461681      90.5509 +- 2.14182      9      good
chromium@461682      81.1067 +- 41.1588      9      bad       <--
chromium@461683      76.7927 +- 1.25524      6      bad

To Run This Test
  src/tools/perf/run_benchmark -v --browser=android-webview --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests blink_perf.layout

Debug Info
  https://chromeperf.appspot.com/buildbucket_job_status/8983144988352572768

Is this bisect wrong?
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5025342248452096


| 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 Speed>Bisection.  Thank you!
Status: Assigned (was: Untriaged)
I'll take a look. The suspected CL is probably the cause of the regression. 
Components: Blink>Layout>Grid

Comment 6 by e...@chromium.org, May 3 2017

Any update here jfernandez?
I've been giving more priority to other issues, actually. This is affecting only the grid layout test cases and it's because a new feature was added: self-baseline alignment. Even if I could improve the results, the new operation will have some impact on performance. 

I could give more priority to this if you think is more convenient. 

Comment 8 by e...@chromium.org, May 5 2017

Status: WontFix (was: Assigned)
Makes sense. Sounds like this was an expected perf regression as we're doing more work and support more of the spec.

Thanks!
FTR, I send perf try jobs with a patch disabling all the suspected logic and the improvement is relatively small.

https://codereview.chromium.org/2866663003/

I think the regression was not caused by my patch exclusively. Additionally, it seems that the affected performance tests recovered similar levels to the ones we had before the regression. 

After all my analysis, it seems the new logic has an impact of 4-7%, which is more reasonable given the new work we have to do in order to implement the
baseline alignment for grid, even if the items are not finally baseline aligned.
Labels: Performance-Tradeoff

Sign in to add a comment