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

Issue 649988 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

18.6% regression in blink_perf.dom at 420028:420032

Project Member Reported by benjhayden@chromium.org, Sep 25 2016

Issue description

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

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


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

chromium-rel-win8-dual
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Sep 25 2016

Cc: cbiesin...@chromium.org
Owner: cbiesin...@chromium.org

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

Hi cbiesinger@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 : [css-flexbox] Have to call updateBlockChildDirtyBitsBeforeLayout before layoutIfNeeded
Author  : cbiesinger
Commit description:
  
This makes sure that we relayout children when necessary if they use a
percentage height.

BUG= 527039 
R=eae@chromium.org,dgrogan@chromium.org

Review-Url: https://codereview.chromium.org/2343413004
Cr-Commit-Position: refs/heads/master@{#420032}
Commit  : 443873aad1beed753a2370a31079d51459e3c3a0
Date    : Wed Sep 21 12:55:16 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N  Good?
chromium@420027  4081.41  22.8609  5  good
chromium@420030  4040.27  11.8685  5  good
chromium@420031  3798.85  9.46153  5  good
chromium@420032  3340.54  8.01416  5  bad    <--

Bisect job ran on: win_8_perf_bisect
Bug ID: 649988

Test Command: src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --also-run-disabled-tests blink_perf.dom
Test Metric: select-multiple-add/select-multiple-add
Relative Change: 18.15%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/win_8_perf_bisect/builds/2205
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9000579531245007952


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

| 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!
Cc: jasontiller@chromium.org
cbiesinger, the bisect indicates that your CL caused this regression. Can you take a look please?
Sooo, that patch was necessary for correctness. I'll have to see if I can optimize this, though...
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 12 2016

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

commit c03addfdbc3e63d0df45e83f14b6c8d12572ef4b
Author: cbiesinger <cbiesinger@chromium.org>
Date: Wed Oct 12 01:21:26 2016

[css-flexbox] Don't force layout for overflow: auto anymore

Now that we include the scrollbar in the preferred width due to these changes:
https://chromium.googlesource.com/chromium/src/+/c9b3e99e2a71f7bcc4d9652916347782ff616e0d
https://chromium.googlesource.com/chromium/src/+/906252430fe0108cf163f1fd9c2e96aa283f9cb5%5E%21/

we no longer need to force layout to include the scrollbar in the flex item
size as the above code makes this work correctly. This should help a bit with
performance, too.

(The listed bug is somewhat optimistic; it may help but I haven't really
investigated yet)

R=eae@chromium.org,dgrogan@chromium.org
TEST=n/a, no behavior change
BUG= 649988 

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

[modify] https://crrev.com/c03addfdbc3e63d0df45e83f14b6c8d12572ef4b/third_party/WebKit/Source/core/layout/LayoutFlexibleBox.cpp
[modify] https://crrev.com/c03addfdbc3e63d0df45e83f14b6c8d12572ef4b/third_party/WebKit/Source/core/layout/LayoutFlexibleBox.h

Project Member

Comment 8 by 42576172...@developer.gserviceaccount.com, Oct 12 2016


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


===== SUSPECTED CL(s) =====
Subject : [css-flexbox] Have to call updateBlockChildDirtyBitsBeforeLayout before layoutIfNeeded
Author  : cbiesinger
Commit description:
  
This makes sure that we relayout children when necessary if they use a
percentage height.

BUG= 527039 
R=eae@chromium.org,dgrogan@chromium.org

Review-Url: https://codereview.chromium.org/2343413004
Cr-Commit-Position: refs/heads/master@{#420032}
Commit  : 443873aad1beed753a2370a31079d51459e3c3a0
Date    : Wed Sep 21 12:55:16 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N  Good?
chromium@420027  4043.72  86.7932  5  good
chromium@420030  4048.46  6.51394  5  good
chromium@420031  3786.3   8.81022  5  good
chromium@420032  3344.21  12.1918  5  bad    <--

Bisect job ran on: win_8_perf_bisect
Bug ID: 649988

Test Command: src/tools/perf/run_benchmark -v --browser=release --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests blink_perf.dom
Test Metric: select-multiple-add/select-multiple-add
Relative Change: 17.30%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/win_8_perf_bisect/builds/2241
Job details: https://chromeperf.appspot.com/buildbucket_job_status/8998987314150919184


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

| 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?
Components: Blink>Layout

Comment 11 by e...@chromium.org, Mar 7 2017

Status: Fixed (was: Assigned)
Fix landed as r424632 and tests have since recovered.

Sign in to add a comment