Issue metadata
Sign in to add a comment
|
14.8%-16.9% regression in blink_perf.layout at 415632:415693 |
||||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Sep 1 2016
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/9002691519892175760
,
Sep 1 2016
=== Auto-CCing suspected CL author svillar@igalia.com === Hi svillar@igalia.com, 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 : [css-grid] Implement fit-content track size Author : svillar Commit description: This CL implements the new <fit-content> track size which is defined as follows: "Represents the formula min(max-content, max(auto, argument)), which is calculated similar to auto (i.e. minmax(auto, max-content)), except that the track size is clamped at argument if it is greater than the auto minimum." From the parsing POV fit-content was implemented as a new type of function which only takes one argument. That forced us to refactor some code because minmax() was the only allowed function for <track-size>s so far. The implementation key is a new attribute in GridTrack called growthLimitCap which is precisely the attribute of fit-content(). Some parts of the track sizing algorithm were adapted to this change like for example the sorting of tracks by growth potential (we need to consider the caps). BUG= 618972 Review-Url: https://codereview.chromium.org/2287113004 Cr-Commit-Position: refs/heads/master@{#415676} Commit : 1993c05386140afe56921048784bf3ca449f4b63 Date : Wed Aug 31 17:43:04 2016 ===== TESTED REVISIONS ===== Revision Mean Std Dev N Good? chromium@415642 749.684 3.25797 5 good chromium@415660 747.97 3.57644 5 good chromium@415669 753.54 2.84553 5 good chromium@415674 753.499 5.09173 5 good chromium@415675 751.241 5.47193 5 good chromium@415676 627.738 0.990206 5 bad <-- chromium@415678 622.63 9.92542 5 bad Bisect job ran on: winx64nvidia_perf_bisect Bug ID: 643359 Test Command: src/tools/perf/run_benchmark -v --browser=release_x64 --output-format=chartjson --upload-results --also-run-disabled-tests blink_perf.layout Test Metric: auto-grid-lots-of-data/auto-grid-lots-of-data Relative Change: 16.95% Score: 99.9 Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/winx64nvidia_perf_bisect/builds/1850 Job details: https://chromeperf.appspot.com/buildbucket_job_status/9002691519892175760 Not what you expected? We'll investigate and get back to you! https://chromeperf.appspot.com/bad_bisect?try_job_id=6665547184340992 | 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!
,
Sep 2 2016
Working on this
,
Sep 5 2016
So after bisecting a bit the changes it turns out that the main issue is the condition I've added to maxTrackBreadth(). That is causing most of the issues because it's called a lot. I've to think in a new way to represent the different track sizes without having to add extra conditions.
,
Sep 7 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8b54987d15a92a54796a357daf6e021f0ff9a2b6 commit 8b54987d15a92a54796a357daf6e021f0ff9a2b6 Author: svillar <svillar@igalia.com> Date: Wed Sep 07 18:25:40 2016 [css-grid] Fix performance regression in grid layout Some of the changes added in crrev.com/415676 made layout of grid containers a bit slower. That was because it modified some methods in GridTrackSize.h which is a hot spot. That class is basically a cache of some values and it's critical to keep it really fast. The aforementioned CL was adding an if clause to a getter method, and that was causing the slowdown. BUG= 643359 Review-Url: https://codereview.chromium.org/2318993002 Cr-Commit-Position: refs/heads/master@{#417001} [modify] https://crrev.com/8b54987d15a92a54796a357daf6e021f0ff9a2b6/third_party/WebKit/Source/core/css/ComputedStyleCSSValueMapping.cpp [modify] https://crrev.com/8b54987d15a92a54796a357daf6e021f0ff9a2b6/third_party/WebKit/Source/core/layout/LayoutGrid.cpp [modify] https://crrev.com/8b54987d15a92a54796a357daf6e021f0ff9a2b6/third_party/WebKit/Source/core/style/GridTrackSize.h
,
Sep 7 2016
,
Sep 14 2016
Is it better now?
,
Sep 14 2016
Yes, all metrics have recovered from the regressions, in most cases to a better level than pre-Aug-21 level. Thanks svillar@. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by mustaq@chromium.org
, Sep 1 2016