New issue
Advanced search Search tips

Issue 729236 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

11.1%-16.2% regression in blink_perf.layout at 475990:476045

Project Member Reported by m...@chromium.org, Jun 2 2017

Issue description

See the link to graphs below.
 
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 : d10fc6ad638e7cd58576fd378ee4fcf1481c0ed3
  Date   : Wed May 31 21:12:30 2017
  Subject: Reland of [css-align] Don't resolve 'auto' values for computed style.

Bisect Details
  Configuration: android_webview_nexus6_aosp_perf_bisect
  Benchmark    : blink_perf.layout
  Metric       : fixed-grid-lots-of-stretched-data/fixed-grid-lots-of-stretched-data
  Change       : 11.91% | 187.239499171 -> 164.94310761

Revision             Result                  N
chromium@476000      187.239 +- 3.43166      6      good
chromium@476023      181.54 +- 6.64998       6      good
chromium@476024      164.63 +- 3.30547       6      bad       <--
chromium@476025      162.699 +- 7.16296      6      bad
chromium@476026      162.516 +- 5.10148      6      bad
chromium@476029      164.834 +- 7.06093      6      bad
chromium@476034      168.06 +- 4.04716       6      bad
chromium@476045      164.943 +- 3.66244      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/8977855837873964064

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


| 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)
yes, I'll take a look. 
Components: Blink>Layout>Grid

Comment 6 by m...@chromium.org, Jun 5 2017

Cc: -miu@google.com
Friendly perf sheriff ping, any update on this?
Oh yes, patch is pending on review:

https://chromium-review.googlesource.com/c/524043/
Status: Started (was: Assigned)
Cc: svil...@igalia.com
Project Member

Comment 11 by bugdroid1@chromium.org, Jun 12 2017

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

commit 9ac1f4cef1549b900b83c5e989066b19991f2459
Author: Javier Fernandez <jfernandez@igalia.com>
Date: Mon Jun 12 19:43:36 2017

[css-grid] Control only style changes that imply shrinking or stretching

We are monitoring grid container's style change to detect cases where
some grid items may be shrunk or stretched. In those cases we are
marking the item for layout, which implies a considerable impact on
performance.

Current code assumes, wrongly, that if grid container's old style had
'stretch' as value for its align-items property it may affect some of
its children's Self Alignment properties. This assumption led to an
unnecessary overhead of evaluating every child's align-self property
to detect stretching or shrinking behaviors.

It's obvious that if grid container's Default Alignment change didn't
imply stretching or shrinking, we don't need to evaluate its children
at all.


Bug:  729236 
Change-Id: I0b21f09b1d1c16e06374c2d20d150fd2829711f0
Reviewed-on: https://chromium-review.googlesource.com/524043
Commit-Queue: Javier Fernandez <jfernandez@igalia.com>
Reviewed-by: Sergio Villar <svillar@igalia.com>
Cr-Commit-Position: refs/heads/master@{#478722}
[modify] https://crrev.com/9ac1f4cef1549b900b83c5e989066b19991f2459/third_party/WebKit/Source/core/layout/LayoutGrid.cpp
[modify] https://crrev.com/9ac1f4cef1549b900b83c5e989066b19991f2459/third_party/WebKit/Source/core/layout/LayoutGrid.h

Status: Fixed (was: Started)
This issue should be FIXED now.
Cc: m...@chromium.org
 Issue 729760  has been merged into this issue.

Sign in to add a comment