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

Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDguN3muAoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg2J-V8ggM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDguIy9sAoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg2IuhtAkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDguIKfvQkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDguJmUqAkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDguKKaoQkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDguKimuAkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDguIGWtAkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDguKjosAsM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDguMGFtgkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg2M_D7AgM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDguNz5iAoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDguOaSvgoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDguK3lsQkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg2Jnn4AgM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDguIzirgoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDguNTI8ggM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDguJiruQkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDguOKJowoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDguP6Q5goM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDguPrUpAsM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg2M_DrAsM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDg2MGy5QgM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDguKTe4goM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgIDguMT75AkM


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

android-nexus5
android-nexus6
android-nexus7v2
android-one
android-webview-nexus5X

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

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

Hi csharrison@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 : csharrison
  Commit : bd80f2b1e755bce061294a2ecb8a28b988c9a30a
  Date   : Tue Feb 14 16:47:00 2017
  Subject: [LazyParseCSS] Add field trial configs for testing

Bisect Details
  Configuration: android_one_perf_bisect
  Benchmark    : system_health.memory_mobile
  Metric       : memory:chrome:all_processes:reported_by_os:system_memory:native_heap:proportional_resident_size_avg/load_social/load_social_pinterest
  Change       : 10.06% | 50964020.0 -> 56092894.6667

Revision             Result                   N
chromium@450360      50964020 +- 2471058      6      good
chromium@450380      50334260 +- 128486       6      good
chromium@450381      55563828 +- 1325887      6      bad       <--
chromium@450382      56186761 +- 2290066      6      bad
chromium@450383      55668617 +- 2498262      6      bad
chromium@450385      55340767 +- 999244       6      bad
chromium@450390      55461087 +- 1039055      6      bad
chromium@450399      55370292 +- 365475       6      bad
chromium@450438      56092895 +- 2955809      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-chromium --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=load.social.pinterest system_health.memory_mobile

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

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


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

Yes this is my change, unfortunately. I can take a look but just note that this is an experimental branch.

Note that the perf bots always use the field trial testing configs. Do we want them running this experimental branch?

Yes we want them running the experimental branch for exactly this reason :)

A few questions:
1. how bad is this regression? I looks like it's only on the AndroidOne bots? If this is an unlandable regression I will need to re-evaluate the experiment.

2. What is this measuring? Is this memory after everything is stabilized?

I spun up a trace here for BBC:
https://codereview.chromium.org/2691943011

1. If you expand the group of 27 by clicking on [27] here: https://chromeperf.appspot.com/group_report?bug_id=692932
You'll see that it's on all our android devices, and the last column shows regressions in the MiB range for most tests. So that's pretty bad, almost definitely launch-blocking
2. +perezju, primiano, can you explain the timing of the measurement?

Also about traces, the UI is a little confusing but if you click the trace link (not the button again so sorry it's a mess) you can see the traces from the run uploaded to the perf dashboard. Here's an example:

Pinterest on N7
Before: https://console.developers.google.com/m/cloudstorage/b/chrome-telemetry-output/o/trace-file-id_34-2017-02-14_05-09-49-90511.html
After: https://console.developers.google.com/m/cloudstorage/b/chrome-telemetry-output/o/trace-file-id_34-2017-02-14_15-53-13-65578.html

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

^^^ there you also find information about what and when are things measured.

Thanks everyone for the pointers. I've tracked down a few 1MB allocations in the native heap (PSS) that are present after but not before.

This is surprising to me because the the change should have introduced many smaller allocations, not a few big ones.

Is there a way to symbolize the traces from the bots? I am very curious if I caused those allocations.

you can try locally and pass --extra-browser-args=--enable-heap-profiling (or --extra-browser-args=--enable-heap-profiling=native depending on the level of details you want) to tools/perf/run_benchmark

Instructions for the benchmark: 
https://chromium.googlesource.com/chromium/src/+/master/docs/memory-infra/memory_benchmarks.md

Instructions for the heap profiler and semantic of the switches:
https://chromium.googlesource.com/chromium/src/+/master/docs/memory-infra/heap_profiler.md

+esprehn, meade, timloh

Scratch the comment in #9, I think this is attributed to the fact that CSSParserTokens are a much less compressed version of a StylePropertySet than a real StylePropertySet.

I ran a calculation on pinterest and it does seem like we introduce a lot of extra memory by holding on to the tokens, I can share a spreadsheet for those interested. It seems like ~5x is not uncommon per set.

There are a few paths forward:
1. Parse the styles in small chunks during main thread idle periods. Downside is this adds complexity and reduces total CPU savings.
2. Reduce the number of rules we lazily parse (not sure about which rules to include and exclude here). There isn't a silver bullet here.
3. Invest time into launching the streaming CSS parser, which makes lazy parsing have 0 CPU or memory overhead (additionally, it makes tokenization lazy too, so even bigger win). Tim's WIP CL is here: https://codereview.chromium.org/2503683003/.
4. Abandon the project. I would like for the experiment to cook in Beta for a while longer before turning it down to get a sense of the wins for the current design though.

What are the absolute numbers of memory regression? Where are the 1MB allocations coming from?

We can always retokenize under memory pressure, ex. we could discard the token lists for lazy property sets but keep the start/end offset into the underlying string.

I don't think we should revert this, WebKit is already shipping it too.

The 1MB allocations are not this, sorry it was a red herring and I was just guessing based on misreading memory-infra. We're basically allocating ~5MB of tokens + overhead (the "closure-ish" object that holds the tokens) due to huge CSS on pinterest, which makes me think that the regression numbers on the bots are accurately reporting the true regression (in the low MiB depending on the site).

Can we really re-tokenize without the streaming parser infrastructure? If that's doable without too much added complexity I feel like it is the way to go.

As I mentioned in  Issue#642722 , this is causing 1-3MB regression across all tested pagesets in the perf lab, and 5MB on cnn.com specifically. Chris - you mentioned having a fix in flight, what's the ETA? Which milestone are you targeting for this feature - assuming 58 as this already shipped with WebKit.

The tradeoffs you're making are a bit concerning, as well. e.g. Ignition bought us 3MB of memory, yet cost 6 SWE years, so regressing by 3MB average is extremely expensive from a memory point of view. I'm not sure what the cost/benefit is for loading, so any information there would be helpful.

Hey, I am Charlie not Chris (easily mixed up).

This feature is completely gated on finch. We won't ship this on Stable until this issue is resolved. In the meantime, it seems like having this on the perf bots is causing a lot of stress for everybody, so I propose in the short term we:
1. Stop perf bots from using this experiment
2. Stop experimenting on Beta channel (which requires bots to use the experiment config)

I have a CL for discarding tokens [1], but it is highly experimental and unreviewed.

esprehn@: WDYT about the plan? Now that we know which metrics to look for, we could detect this before hand with a manual bot bisect before making this the default for bots.

[1] https://codereview.chromium.org/2711853003/

Charlie, your plan sounds good to me :). Sorry about that.

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

commit 279edadbd6962b7afa97c166896ae69ceb797da4
Author: csharrison <csharrison@chromium.org>
Date: Mon Mar 06 17:05:49 2017

Turn off LazyParseCSS on trybots for now

This is causing a regression on trybots. Let's turn it off for now
(and abort the experiment on Beta) until this is resolved.

BUG= 692932 

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

[modify] https://crrev.com/279edadbd6962b7afa97c166896ae69ceb797da4/testing/variations/fieldtrial_testing_config.json

Is it okay to mark this bug "fixed" as LazyParseCSS is turned off, and we'll likely move to a new bug to turn it on again?

Yep, that sounds good.