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

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=1cd7e39175a0e96dd5ba592e8fc77c4d26b5f895a56b70e3dfdd180b9c89abdb


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

android-webview-nexus6

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

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

Hi nainar@chromium.org, 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 : Naina Raisinghani
  Commit : b71f5b7df345f1a8603b7cf7b59edf48288e7d96
  Date   : Mon Dec 04 01:01:29 2017
  Subject: Remove calls to styleForLayoutObject() in LayoutTreeBuilder::style()

Bisect Details
  Configuration: android_webview_nexus6_aosp_perf_bisect
  Benchmark    : system_health.memory_mobile
  Metric       : memory:webview:all_processes:reported_by_chrome:skia:effective_size_avg/browse_news/browse_news_cnn
  Change       : 7.44% | 885438.0 -> 951316.0

Revision             Result                 N
chromium@521228      885438 +- 10211.6      6      good
chromium@521234      885449 +- 10240.5      6      good
chromium@521237      887192 +- 3616.59      6      good
chromium@521239      887144 +- 8030.83      6      good
chromium@521240      983991 +- 159076       6      bad       <--
chromium@521251      951316 +- 140892       6      bad

Please refer to the following doc on diagnosing memory regressions:
  https://chromium.googlesource.com/chromium/src/+/master/docs/memory-infra/memory_benchmarks.md

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 --story-filter=browse.news.cnn system_health.memory_mobile

More information on addressing performance regressions:
  http://g.co/ChromePerformanceRegressions

Debug information about this bisect:
  https://chromeperf.appspot.com/buildbucket_job_status/8961032546296977024


For feedback, file a bug with component Speed>Bisection

 Issue 792082  has been merged into this issue.

I think this is possibly due to me not clearing NonAttachedStyle at the end of the rebuildLayoutTree phase. 

As a first step adding a few DCHECKs to validate this assumption and then lets work from there

 Issue 792078  has been merged into this issue.

 Issue 792090  has been merged into this issue.

CL with missing DCHECKS here: https://chromium-review.googlesource.com/c/chromium/src/+/809989

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

commit 9e8728dc66f2e67f317c3c9c8b7b4073e3e1ac72
Author: Naina Raisinghani <nainar@chromium.org>
Date: Wed Dec 06 08:39:42 2017

Assert that NonAttachedStyle is deleted after Node::AttachLayoutTree()

This patch asserts that we clear the NonAttachedStyle at the end of
attaching LayoutTree. This ensures correctness of
https://chromium-review.googlesource.com/c/chromium/src/+/765097

Bug:  792080 
Change-Id: Ia3a1d376bddb84fb4c8751739144ef07e3bd18ae
Reviewed-on: https://chromium-review.googlesource.com/809989
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522044}
[modify] https://crrev.com/9e8728dc66f2e67f317c3c9c8b7b4073e3e1ac72/third_party/WebKit/Source/core/dom/Element.cpp
[modify] https://crrev.com/9e8728dc66f2e67f317c3c9c8b7b4073e3e1ac72/third_party/WebKit/Source/core/dom/Node.cpp

 Issue 792671  has been merged into this issue.

Issue 792089 has been merged into this issue.

 Issue 792445  has been merged into this issue.

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

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

=== BISECT JOB RESULTS ===
Perf regression found with culprit

Suspected Commit
  Author : Naina Raisinghani
  Commit : b71f5b7df345f1a8603b7cf7b59edf48288e7d96
  Date   : Mon Dec 04 01:01:29 2017
  Subject: Remove calls to styleForLayoutObject() in LayoutTreeBuilder::style()

Bisect Details
  Configuration: android_webview_nexus6_aosp_perf_bisect
  Benchmark    : system_health.memory_mobile
  Metric       : memory:webview:all_processes:reported_by_chrome:skia:effective_size_avg/browse_news/browse_news_cnn
  Change       : 9.35% | 884929.333333 -> 967637.333333

Revision             Result                 N
chromium@521228      884929 +- 6777.61      6      good
chromium@521234      889515 +- 6256.42      6      good
chromium@521237      886569 +- 6236.17      6      good
chromium@521239      888925 +- 5776.17      6      good
chromium@521240      944387 +- 151740       6      bad       <--
chromium@521251      967637 +- 153885       6      bad

Please refer to the following doc on diagnosing memory regressions:
  https://chromium.googlesource.com/chromium/src/+/master/docs/memory-infra/memory_benchmarks.md

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 --story-filter=browse.news.cnn system_health.memory_mobile

More information on addressing performance regressions:
  http://g.co/ChromePerformanceRegressions

Debug information about this bisect:
  https://chromeperf.appspot.com/buildbucket_job_status/8960350080894287568


For feedback, file a bug with component Speed>Bisection

=== BISECT JOB RESULTS ===
Perf regression found with culprit

Suspected Commit
  Author : Naina Raisinghani
  Commit : b71f5b7df345f1a8603b7cf7b59edf48288e7d96
  Date   : Mon Dec 04 01:01:29 2017
  Subject: Remove calls to styleForLayoutObject() in LayoutTreeBuilder::style()

Bisect Details
  Configuration: android_webview_nexus6_aosp_perf_bisect
  Benchmark    : system_health.memory_mobile
  Metric       : memory:webview:all_processes:reported_by_chrome:skia:effective_size_avg/browse_news/browse_news_cnn
  Change       : 5.39% | 886558.666667 -> 934362.666667

Revision             Result                 N
chromium@521228      886559 +- 6253.82      6      good
chromium@521234      887701 +- 2862.25      6      good
chromium@521237      887695 +- 7587.86      6      good
chromium@521239      886651 +- 7978.03      6      good
chromium@521240      941107 +- 102038       6      bad       <--
chromium@521251      934363 +- 111698       6      bad

Please refer to the following doc on diagnosing memory regressions:
  https://chromium.googlesource.com/chromium/src/+/master/docs/memory-infra/memory_benchmarks.md

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 --story-filter=browse.news.cnn system_health.memory_mobile

More information on addressing performance regressions:
  http://g.co/ChromePerformanceRegressions

Debug information about this bisect:
  https://chromeperf.appspot.com/buildbucket_job_status/8960349404235765024


For feedback, file a bug with component Speed>Bisection

These graphs have reverted back to baseline metrics. 

The graphs have definitely not reverted back to prior baseline.

Making this available, since Naina isn't on the team any more, and raising priority, because the regression is pretty big.

Looking at the traces, the memory regression/instability stems from different used size of the glyph caches in skia. I don't know how the identified change could affect that.

I've started looking at the perf regression in the shadow dom micro benchmark, and I can definitely reproduce it on Linux if I turn SlotInFlatTree off. Reverting b71f5b7df345f1a8603b7cf7b59edf48288e7d96 does not have an effect when SlotInFlatTree is on.

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

commit bddb211b216ed6844cb64a7ec51b069e0ac044b5
Author: Rune Lillesveen <futhark@chromium.org>
Date: Mon Mar 12 13:16:32 2018

[Squad] Don't compute style for attaching non-flat-tree elements.

RecalcStyleForReattach would compute style for slot elements even
though they were not part of the flat tree before SlotInFlatTree runtime
flag was introduced.

Moved SetNonAttachedStyle(nullptr) to Element to make sure we only ever
set or clear non-attached style on Elements, and added DCHECKs to
support that and the fact that we only set non-attached style on
elements which are part of the flat tree.

This was partly the regression for 792080, but will not have an effect
for the performance tests on master since SlotInFlatTree is enabled.

Bug:  792080 
Change-Id: I47659300dea9cf4411a4e06f574dbad15c29fdfb
Reviewed-on: https://chromium-review.googlesource.com/958465
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542470}
[modify] https://crrev.com/bddb211b216ed6844cb64a7ec51b069e0ac044b5/third_party/WebKit/Source/core/dom/Element.cpp
[modify] https://crrev.com/bddb211b216ed6844cb64a7ec51b069e0ac044b5/third_party/WebKit/Source/core/dom/Node.cpp

📍 Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/11fd551e440000

For the skia glyph cache change bisected for, looking at the history and what has happened after the reported regression, it doesn't look like the glyph cache sizes has regressed particularly looking at the big picture.

The exceptional part was really the stable results the period just before the reported regression. Other platforms also has changes in skia avg memsize results, but only Nexus6 WebView has something that looks like a regression in the range reported here.

The clear regression from the suspected commit was fixed by the CL in comment #24. Although, as mentioned in the commit description, it will not have an effect as long as SlotInFlatTree is enabled.

I don't think there's more I can do here now.

For the record, the SlotInFlatTree regression has been WontFixed in another issue as it will double the number of elements having ComputedStyle for the microbenchmark in question.

herb@ assigning to you based on git log on SkGlyphCache.cpp in case you have any ideas about what could affect glyph cache behavior here.

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

commit 62221431022ae38f900136df249d1923efb67506
Author: Rune Lillesveen <futhark@chromium.org>
Date: Thu Mar 15 15:37:20 2018

[Squad] Don't compute style for attaching non-flat-tree elements.

RecalcStyleForReattach would compute style for slot elements even
though they were not part of the flat tree before SlotInFlatTree runtime
flag was introduced.

Moved SetNonAttachedStyle(nullptr) to Element to make sure we only ever
set or clear non-attached style on Elements, and added DCHECKs to
support that and the fact that we only set non-attached style on
elements which are part of the flat tree.

This was partly the regression for 792080, but will not have an effect
for the performance tests on master since SlotInFlatTree is enabled.

Bug:  792080 ,  821361 
Change-Id: I47659300dea9cf4411a4e06f574dbad15c29fdfb
Reviewed-on: https://chromium-review.googlesource.com/958465
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#542470}(cherry picked from commit bddb211b216ed6844cb64a7ec51b069e0ac044b5)
Reviewed-on: https://chromium-review.googlesource.com/964501
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#269}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/62221431022ae38f900136df249d1923efb67506/third_party/WebKit/Source/core/dom/Element.cpp
[modify] https://crrev.com/62221431022ae38f900136df249d1923efb67506/third_party/WebKit/Source/core/dom/Node.cpp

I don't think any skia cache changes that change performance or memory happened in that time period.