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

Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg7J-M7QgM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgnLjMpgkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg7ImUsQoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg7Na8tQsM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg7IaspAsM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDgrPnXtwoM


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

chromium-rel-mac11-pro
chromium-rel-mac12
chromium-rel-mac12-mini-8gb
chromium-rel-win7-gpu-ati

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

=== BISECT JOB RESULTS ===
Bisect was unable to run to completion

The bisect was able to narrow the range, you can try running with:
  good_revision: 62349ad779505d73be7c1dbd3db83dc9a38dc99f
  bad_revision : b62e2e3d17205ea1e128d85fb6211192e62855d9

If failures persist contact the team (see below) and report the error.


Bisect Details
  Configuration: mac_pro_perf_bisect
  Benchmark    : blink_perf.layout
  Metric       : long-line-nowrap-spans-collapse/long-line-nowrap-spans-collapse
  Change       : 12.30% | 35.3455 -> 39.6941666667

Revision             Result                  N
chromium@459423      35.3455 +- 1.99386      6      good
chromium@459456      35.3938 +- 2.0165       6      good
chromium@459472      36.1857 +- 2.72459      6      good
chromium@459480      35.9047 +- 2.31867      6      good
chromium@459484      36.2423 +- 2.77677      6      good
chromium@459486      38.8718 +- 2.57904      6      bad
chromium@459488      39.1932 +- 3.6495       6      bad
chromium@459552      39.6942 +- 2.60593      6      bad

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

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

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


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

This actually completed, not sure why it reported that it didn't.

{
  "author": "kojii",
  "body": "\nThis patch disables the inline capacity of WordMeasurements but to\nreserve the heap once per line, in order to avoid stack overflow in\nLayoutBlockFlow::layoutRunsAndFloatsInRange() when stack is critical,\nsuch as when V8 initiates the layout.\n\nCurrently, LayoutBlockFlow::layoutRunsAndFloatsInRange() is the only\nplace we instantiate WordMeasurements.\n\nThis change is expected to save stack space by ~2k.\n\nBUG=704536\n\nReview-Url: https://codereview.chromium.org/2765353004\nCr-Commit-Position: refs/heads/master@{#459485}",
  "date": "Fri Mar 24 18:35:00 2017",
  "email": "kojii@chromium.org",
  "subject": "Avoid inline capacity of WordMeasurements"
}

Thank you for this report. I'll revert. I've merged this change to M58 10 hours ago, so I guess I'll need to revert that too.

I wouldn't necessarily revert yet. If it also impact the page cycler then yes, we should but this is a micro benchmark testing a known worst-case.

lanwei@, could you advise? I'm not very familiar with perf.

So far:
1. The change was once merged to M58, but was reverted (crbug.com/704536#c47)
2. Thought to revert on trunk too, but with #6, the revert was suspended.
3. The suspected change allocates memory once per run/line, so these worst-cases regressed is understandable.
4. Looked at a few page_cycler_v2 tests at the point but I do not see obvious regressions.
   chromium-rel-win7-x64-dual/page_cycler_v2.typical_25 / timeToFirstContentfulPaint_avg
   android-nexus5/page_cycler_v2.tough_layout_cases / timeToFirstContentfulPaint_avg

crbug.com/704536#c42 increased the stack size by 1MB, while the suspected change saves the stack only by 2KB, so reverting on trunk should be ok. While it's still better to not to use stack much.

Briefly talked with eae@ and dominicc@, both didn't seem to have definitive opinion atm. dominicc@ mildly prefer to revert since saving 2KB isn't worth much after the 1MB fix.

I think questions are:
1. Which tests/bots are good to look for more real case regressions?
2. If bad regression occurs in page cycler, I should have been notified already; so not having other bugs means there were not, or not significant. Is my understanding correct?
3. If we prefer to keep this CL, what is the expected action for me? Just WontFix this issue?

Thank you for your advise in advance.

I'm slightly leaning towards to revert before the branch point, as the "increase stack" landed in trunk and merged to M58.

The downside is not visible from other tests/platforms, only these 2 tests on Mac/Win7, but I suppose our coverage on RTL and large text file isn't great at this moment.

Appreciate anyone to comment if different thoughts.

Given that the increase in stack space change has landed I tend to agree with dominicc and yourself that we should probably revert it. 

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

commit 165039831ad0a01bcc90b66d023c9d90dcc964d1
Author: kojii <kojii@chromium.org>
Date: Mon Apr 10 05:29:04 2017

Revert of Avoid inline capacity of WordMeasurements (patchset #1 id:1 of https://codereview.chromium.org/2765353004/ )

Reason for revert:
Reverting due to 10.2%-11.9% regression in blink_perf.layout,
reported in  issue 706907 .

Original issue's description:
> Avoid inline capacity of WordMeasurements
>
> This patch disables the inline capacity of WordMeasurements but to
> reserve the heap once per line, in order to avoid stack overflow in
> LayoutBlockFlow::layoutRunsAndFloatsInRange() when stack is critical,
> such as when V8 initiates the layout.
>
> Currently, LayoutBlockFlow::layoutRunsAndFloatsInRange() is the only
> place we instantiate WordMeasurements.
>
> This change is expected to save stack space by ~2k.
>
> BUG=704536
>
> Review-Url: https://codereview.chromium.org/2765353004
> Cr-Commit-Position: refs/heads/master@{#459485}
> Committed: https://chromium.googlesource.com/chromium/src/+/704ca0cfcc046d199d31e6502639bc94b89fddb0

TBR=eae@chromium.org,dcheng@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=704536,  706907 

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

[modify] https://crrev.com/165039831ad0a01bcc90b66d023c9d90dcc964d1/third_party/WebKit/Source/core/layout/LayoutBlock.h
[modify] https://crrev.com/165039831ad0a01bcc90b66d023c9d90dcc964d1/third_party/WebKit/Source/core/layout/LayoutBlockFlowLine.cpp