All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=675534

Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgj5_s7AgM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgz8fbswoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgz4yZkAsM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgz5PHpQoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgz9v25ggM


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

android-nexus5
android-nexus5X
android-nexus6
android-nexus7v2
android-nexus9

Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8992862555179790096

=== PERF REGRESSION ===


=== Auto-CCing suspected CL author cathiechen@tencent.com ===

Hi cathiechen@tencent.com, the bisect results pointed to your CL, please take a look at the
results.


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


===== SUSPECTED CL(s) =====
Subject : Fix the inconsistent problem while the content of textNodes is changed.
Author  : cathiechen
Commit description:
  
If the text content of one node who share fingerPrint with others changed,
the multipiler of this node might be different from others.
This's the reason of this inconsistent problem.

The solution:
     1. Make superclusters across layouts
     2. Mark superclusters if needed
         2.1 new roots are added to superclusters
         2.2 old roots add some new contents
     3. Recalculate the multiplier of the marked superclusters during the first beginLayout().

BUG= 612119 

Review-Url: https://codereview.chromium.org/2299213003
Cr-Commit-Position: refs/heads/master@{#439093}
Commit  : bf9901feb0c46cefbd5e7af40fe252a6a92d52cb
Date    : Fri Dec 16 12:38:04 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev  N   Good?
chromium@439092  4.97009  1.72991  30  good
chromium@439093  3.9773   1.05968  30  bad    <--

Bisect job ran on: android_nexus9_perf_bisect
Bug ID: 675534

Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests blink_perf.layout
Test Metric: flexbox-lots-of-data/flexbox-lots-of-data
Relative Change: 19.98%

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus9_perf_bisect/builds/2332
Job details: https://chromeperf.appspot.com/buildbucket_job_status/8992862555179790096


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

| 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!

Got it! I'm working on it and try to reduce this.

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

commit 340893b524c359dbd40b7ad656e9a6c8240724e4
Author: cathiechen <cathiechen@tencent.com>
Date: Wed Dec 21 05:15:59 2016

Fix the performance issuse called by last CL.

Adding superclusters into m_potentiallyInconsistentSuperclusters too frequently caused this issuse.

Try to reduce unnecessary adding in these two ways:

1. skip everHadLayout blocks in record.
2. skip UnknownAmountOfText superclusters

BUG= 675534 

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

[modify] https://crrev.com/340893b524c359dbd40b7ad656e9a6c8240724e4/third_party/WebKit/Source/core/layout/TextAutosizer.cpp
[modify] https://crrev.com/340893b524c359dbd40b7ad656e9a6c8240724e4/third_party/WebKit/Source/core/layout/TextAutosizer.h

This issue is related to the autosize of third columns.

According to the logic of flexbox layout. See the example below. 
The lines of item3's content will determine if item1 and item2 should relayout in LayoutFlexibleBox::repositionLogicalHeightDependentFlexItems().
1. If item3 is one-line, all these 3 items share a same height, so LayoutFlexibleBox::repositionLogicalHeightDependentFlexItems() won't trigger the layout of item1 and item2. 
2. If item3 is multi-line, item1 and item2 should stretch their height to item3's height. So it will trigger another layout of item1 and item2. 
3. So item3 with multi-line content will cost more.

	<div id="flexbox" style="display:flex">
		<div id="item1">one-line text</div>
		<div id="item2">one-line text</div>
		<div id="item3">one-line / multi-line text</div>
	</div>

Back to this issue. The autosize of third column will make many one-line items to multi-line. See the pictures attached below. As multi-line cost more, the autosized one cost more too! At the example of flexbox-lots-of-data.html, autosized one trigger 600+ times layout more than non-autosized one.

Why the third column will be autosized after this CL?
1. The two preceding items of the first flexbox are empty. 
2. At the first layout pass: The supercluster of flexboxes(class="row") will find the third item(class="cell col-text") as width provider. The text is enough for this width provider. So this supercluster will be autosized.
3. At the second and subsequent layout pass: The supercluster will find flexbox(class="row") as its width provider. Text isn't enough now. So this supercluster won't be autosized this time. After this CL, the superclusters are across layout passes. So this supercluster won't recalculate its multiplier.

See the compare table below:
------------------------------------------------------------------
          | first layout pass | second and subsequent layout pass 
------------------------------------------------------------------
before CL | autosize item3    | don't autosize item3              
------------------------------------------------------------------
after CL  | autosize item3    | autosize item3             
------------------------------------------------------------------

This test case tests the performance of layout, and break-line is a common action. So I think 
maybe we could make a slightly change to flexbox-lots-of-data.html. Make the item1 and item2 of the first flexbox non empty.

As to if we should update superclusters' multiplier at the second and subsequent layout pass, we could make superclusters updateable. But I'm not sure. Any suggestions? pdr@chromium.org skobes@chromium.org

Thanks for the analysis.  It sounds like

- we now autosize some content in some scenarios where we previously did not
- cost of autosizing is compounded by flex-item height stretching
- due to the above, we regressed mobile performance of a flexbox layout test
- regression is fairly specific to the exact conditions of this test
- it is correct (per current design of TextAutosizer) that we autosize this content

Based on the above I would be fine closing this as WAI.  @pdr do you agree?

Sorry, I forgot to attach files.

Deleted: textautosize_perf1.png 24.7 KB

	textautosize_perf1.png 24.7 KB View Download

Deleted: textautosize_perf2.png 22.2 KB

	textautosize_perf2.png 22.2 KB View Download

Deleted: domtree_screenshot.png 99 KB

	domtree_screenshot.png 99 KB View Download

Great analysis. I agree that this should be closed because it's working as intended.