1330412 - (stylo-nightly) [meta] Enable Stylo in the Nightly channel

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P5)

Description

[Chris Peterson [:cpeterson]]

We may need a separate bug for Stylo riding the trains to release. We can use the top-level Stylo meta bug 1243581 for post-Nightly work in the meantime.

Comment 1

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Here's a first attempt at enabling Stylo by default, let's see if it's green:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=96721b0f705a9801aaa60803f3ec438e6acc4e63

Comment 2

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

I guess I'll assign this since I am preparing the code change and reviewing test status here.

Comment 3

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Bug 1330412 - Enable Stylo by default.

Review commit: https://reviewboard.mozilla.org/r/175124/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/175124/

Comment 4

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Bug 1330412 - Stylo seq. runs for memory / perf only.

We no longer need to run unit / integration tests on Stylo sequential, but we'd
like to retain it for memory and performance checks.

In addition, we no longer need to explicitly `--enable-stylo` since it is the
default.  This also drops the `stylo` tag in Perfherder.

Review commit: https://reviewboard.mozilla.org/r/175126/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/175126/

Comment 5

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Bug 1330412 - Limit reftest-stylo to linux64.

Review commit: https://reviewboard.mozilla.org/r/175128/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/175128/

Comment 6

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Bug 1330412 - Schedule Linux Stylo Talos like others.

Make Linux Stylo Talos jobs explicit in the task list, like they are for macOS
and Windows.  This way we avoid have different approaches per platform.

(This is still the Stylo enabled form.  The next patch coverts to Stylo
disabled for all platforms.)

Review commit: https://reviewboard.mozilla.org/r/175130/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/175130/

Comment 7

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Bug 1330412 - Convert Stylo jobs to Stylo disabled.

Convert all jobs that were exercising Stylo enabled to Stylo disabled instead.

Stylo enabled is now handled by the default jobs.

In Perfherder, Stylo enabled jobs will be untagged and take over the existing
Gecko series.  Stylo disabled jobs will have a new `stylo-disabled` tag and
create a new series.

Review commit: https://reviewboard.mozilla.org/r/175132/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/175132/

Comment 8

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Here's a try run with the current state:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4c2998f33d9a493d9e5350a63da832d61333f24b

The renamed Talos jobs aren't being scheduled on Linux / Windows yet, but I believe that's expected because they'll need BB changes too.

Comment 9

[Bobby Holley (:bholley)]

Comment on attachment 8903339 [details]
Bug 1330412 - Enable Stylo by default.

https://reviewboard.mozilla.org/r/175124/#review180182

::: toolkit/moz.configure:591
(Diff revision 1)
>          if target.os == 'Android':
>              # Stylo on Android is happening Later(tm).
>              pass
>          else:
>              build_stylo = True
> +            enable_stylo = milestone.is_nightly

Hm, it seems safer just to set this to true, and then change it to do this if some disaster strikes and we decide not to ship stylo in 57. Otherwise it seems likely that we might forget and then have a surprise on beta when we think stylo is on but it's not.

r=me with that.

Comment 10

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Add Stylo disabled Talos suites to Buildbot

:kmoir, I think this should add the Stylo disabled Talos suites to Buildbot, but I've never attempted a Buildbot change.

Please take a look.  I'm not sure how to produce a builder diff, so it would be great if you could do so.

Comment 11

[Joel Maher ( :jmaher ) (UTC -8) (PTO'ish until July 5)]

Comment on attachment 8903340 [details]
Bug 1330412 - Stylo seq. runs for memory / perf only.

https://reviewboard.mozilla.org/r/175126/#review180400

Comment 12

[Joel Maher ( :jmaher ) (UTC -8) (PTO'ish until July 5)]

Comment on attachment 8903341 [details]
Bug 1330412 - Limit reftest-stylo to linux64.

https://reviewboard.mozilla.org/r/175128/#review180402

Comment 13

[Joel Maher ( :jmaher ) (UTC -8) (PTO'ish until July 5)]

Comment on attachment 8903342 [details]
Bug 1330412 - Schedule Linux Stylo Talos like others.

https://reviewboard.mozilla.org/r/175130/#review180404

::: taskcluster/ci/test/test-sets.yml:139
(Diff revision 1)
>  
> +linux-talos-stylo:
> +    - talos-chrome-stylo
> +    - talos-dromaeojs-stylo
> +    - talos-g1-stylo
> +    - talos-g2-stylo

as a note, we run talos-g3 on linux only; in this case we need to add it here as talos-g3-stylo and then create an entry in tests.yml for talos-g3-stylo.

Comment 14

[Joel Maher ( :jmaher ) (UTC -8) (PTO'ish until July 5)]

Comment on attachment 8903343 [details]
Bug 1330412 - Convert Stylo jobs to Stylo disabled.

https://reviewboard.mozilla.org/r/175132/#review180406

a lot of changes, overall this is great.

::: taskcluster/ci/test/test-sets.yml:140
(Diff revision 1)
> -linux-talos-stylo:
> -    - talos-chrome-stylo
> -    - talos-dromaeojs-stylo
> -    - talos-g1-stylo
> -    - talos-g2-stylo
> -    - talos-g4-stylo
> +linux-talos-stylo-disabled:
> +    - talos-chrome-stylo-disabled
> +    - talos-dromaeojs-stylo-disabled
> +    - talos-g1-stylo-disabled
> +    - talos-g2-stylo-disabled
> +    - talos-g4-stylo-disabled

note: in the previous patch, I mentioned a need for talos-g3-stylo, that will need to be updated here as well.

::: taskcluster/ci/test/tests.yml:1313
(Diff revision 1)
>  
> -talos-chrome-stylo:
> -    description: "Talos Stylo chrome"
> +talos-chrome-stylo-disabled:
> +    description: "Talos Stylo disabled chrome"
>      suite: talos
> -    try-name: chromez-stylo
> +    try-name: chromez-stylo-disabled
>      treeherder-symbol: tc-Ts(c)

I think we should consider changing the treeherder-symbol, currently it is tc-Ts(x), but Ts stands for Talos Stylo- possibly we clean this up for the talos jobs in a followup (you need to add tc-Tsd(x) in many places)

Comment 15

[Nathan Froyd [:froydnj]]

Comment on attachment 8903339 [details]
Bug 1330412 - Enable Stylo by default.

https://reviewboard.mozilla.org/r/175124/#review180482

I think I agree with bholley here.  The alternate path would be to enable for `milestone.is_nightly` now and then prior to the merge, if nothing has shown up, flip it to `True`.  But that would require remembering, and with a slightly accelerated merge date, we'd have to remember a little sooner than we would otherwise.  So let's flip it now, and then we can flip back if there's a problem.

Comment 16

[Kim Moir [:kmoir] ET]

Attachment: bug1330412builder.diff

builder diff from the buildbot talos changes, keep in mind that I ran the diff on a test master just configured to run windows tests.

Comment 17

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8903632 [details]
bug1330412builder.diff

Great, this diff looks correct to me.  We have added stylo-disabled variants for all the Talos runs as expected.

Please review the BB change, then we can land and reconfigure BB.

Comment 18

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

BB changes landed, :kmoir will merge to prod once tests pass:

https://hg.mozilla.org/build/buildbot-configs/rev/02c40bbc7908

Comment 19

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Bug 1330412 - Clean up Stylo Treeherder symbols.

Clean up and standardize Treeherder symbols for Talos and AWSY tasks:

* Stylo disabled groups include `sd`
* Stylo sequential groups include `ss`

Review commit: https://reviewboard.mozilla.org/r/175482/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/175482/

Comment 20

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8903339 [details]
Bug 1330412 - Enable Stylo by default.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/175124/diff/1-2/

Comment 21

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8903340 [details]
Bug 1330412 - Stylo seq. runs for memory / perf only.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/175126/diff/1-2/

Comment 22

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8903341 [details]
Bug 1330412 - Limit reftest-stylo to linux64.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/175128/diff/1-2/

Comment 23

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8903342 [details]
Bug 1330412 - Schedule Linux Stylo Talos like others.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/175130/diff/1-2/

Comment 24

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8903343 [details]
Bug 1330412 - Convert Stylo jobs to Stylo disabled.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/175132/diff/1-2/

Comment 25

[Joel Maher ( :jmaher ) (UTC -8) (PTO'ish until July 5)]

Comment on attachment 8903342 [details]
Bug 1330412 - Schedule Linux Stylo Talos like others.

https://reviewboard.mozilla.org/r/175130/#review180594

Comment 26

[Joel Maher ( :jmaher ) (UTC -8) (PTO'ish until July 5)]

Comment on attachment 8903729 [details]
Bug 1330412 - Clean up Stylo Treeherder symbols.

https://reviewboard.mozilla.org/r/175482/#review180598

lots of great changes!

Comment 27

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8903339 [details]
Bug 1330412 - Enable Stylo by default.

https://reviewboard.mozilla.org/r/175124/#review180182

> Hm, it seems safer just to set this to true, and then change it to do this if some disaster strikes and we decide not to ship stylo in 57. Otherwise it seems likely that we might forget and then have a surprise on beta when we think stylo is on but it's not.
> 
> r=me with that.

Okay, sounds reasonable to me!  (I wasn't sure which approach was preferred here...  I only did the "nightly only" approach originally because a separate "stylo-release" / "let it ride the trains" bug 1374034 exists.)

Comment 28

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8903342 [details]
Bug 1330412 - Schedule Linux Stylo Talos like others.

https://reviewboard.mozilla.org/r/175130/#review180404

> as a note, we run talos-g3 on linux only; in this case we need to add it here as talos-g3-stylo and then create an entry in tests.yml for talos-g3-stylo.

Thanks for catching that!  I've added g3 stylo here.  I also noticed the stylo variants weren't set to run on Linux, so I've changed that too.

Comment 29

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

I believe this should be ready to land, but here's a fresh try so we can be more confident:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=76f6ebfb6bde1bc6216802be556a0a31692646fc

Even if it is ready, we don't plan to land this change until next Tues. Sept 5th because the SETA team will need to make extra changes immediately after it lands, and we're worried about staffing on Friday and then Monday is a US holiday.

Comment 30

[Nathan Froyd [:froydnj]]

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #27)
> > Hm, it seems safer just to set this to true, and then change it to do this if some disaster strikes and we decide not to ship stylo in 57. Otherwise it seems likely that we might forget and then have a surprise on beta when we think stylo is on but it's not.
> > 
> > r=me with that.
> 
> Okay, sounds reasonable to me!  (I wasn't sure which approach was preferred
> here...  I only did the "nightly only" approach originally because a
> separate "stylo-release" / "let it ride the trains" bug 1374034 exists.)

We could set it to `milestone.is_nightly or milestone.is_beta` (would have to write in support for is_beta) if we wanted to let bug 1374034 have something to do--or if we're super-scared about letting Stylo slip out to release accidentally.

Comment 31

[Bobby Holley (:bholley)]

(In reply to Nathan Froyd [:froydnj] from comment #30)
> (In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #27)
> > > Hm, it seems safer just to set this to true, and then change it to do this if some disaster strikes and we decide not to ship stylo in 57. Otherwise it seems likely that we might forget and then have a surprise on beta when we think stylo is on but it's not.
> > > 
> > > r=me with that.
> > 
> > Okay, sounds reasonable to me!  (I wasn't sure which approach was preferred
> > here...  I only did the "nightly only" approach originally because a
> > separate "stylo-release" / "let it ride the trains" bug 1374034 exists.)
> 
> We could set it to `milestone.is_nightly or milestone.is_beta` (would have
> to write in support for is_beta) if we wanted to let bug 1374034 have
> something to do--or if we're super-scared about letting Stylo slip out to
> release accidentally.

I am much more worried about accidentally _not_ enabling stylo on release. We're planning to ship it, so let's orient the gears towards that. If we decide to disable it for 57, it will be a very careful and deliberate process, and we'll be sure to get it right. If we don't decide to disable it for 57, there is no obvious process by which we'll remember to enable it for the next train.

So let's just do the simple thing. Stylo should be the default for all trains starting with 57, and we can switch it off reactively.

Comment 33

[GitHub Autolander Bot]

Attachment: [treeherder] jryans:stylo-disabled-talos > mozilla:master

Comment 34

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Added Treeherder changes so that the new Talos runs from BB will show up correctly.

Comment 35

[Treeherder GitHub Bugbot]

Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/dfa920cbd8819ae72af4884e319081a914ae7477
Bug 1330412 - Add stylo-disabled Talos jobs, update tp6 groups (#2750)

Comment 36

[Ed Morley [:emorley]]

(In reply to Treeherder Bugbot from comment #35)
> Commit pushed to master at https://github.com/mozilla/treeherder
> 
> https://github.com/mozilla/treeherder/commit/dfa920cbd8819ae72af4884e319081a914ae7477
> Bug 1330412 - Add stylo-disabled Talos jobs, update tp6 groups (#2750)

I've cherrypicked this to the Treeherder production branch since I'm still waiting on bug 1215587 comment 49 before I can deploy master. It should be deployed within ~10 minutes, at which point there'll be an IRC notification in #treeherder, and an email to:
https://groups.google.com/forum/#!forum/mozilla.tools.treeherder

Comment 38

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8903339 [details]
Bug 1330412 - Enable Stylo by default.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/175124/diff/2-3/

Comment 39

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8903340 [details]
Bug 1330412 - Stylo seq. runs for memory / perf only.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/175126/diff/2-3/

Comment 40

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8903341 [details]
Bug 1330412 - Limit reftest-stylo to linux64.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/175128/diff/2-3/

Comment 41

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8903342 [details]
Bug 1330412 - Schedule Linux Stylo Talos like others.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/175130/diff/2-3/

Comment 42

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8903343 [details]
Bug 1330412 - Convert Stylo jobs to Stylo disabled.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/175132/diff/2-3/

Comment 43

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8903729 [details]
Bug 1330412 - Clean up Stylo Treeherder symbols.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/175482/diff/1-2/

Comment 44

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8e6a363b03aeb0a32345d9328bcdedaf16806e9b

Comment 45

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7b3215e18443
Enable Stylo by default. r=bholley,froydnj
https://hg.mozilla.org/integration/autoland/rev/d37df1ba1ca9
Stylo seq. runs for memory / perf only. r=jmaher
https://hg.mozilla.org/integration/autoland/rev/ca968311ed00
Limit reftest-stylo to linux64. r=jmaher
https://hg.mozilla.org/integration/autoland/rev/d6c23b724e86
Schedule Linux Stylo Talos like others. r=jmaher
https://hg.mozilla.org/integration/autoland/rev/76e1adb5416d
Convert Stylo jobs to Stylo disabled. r=jmaher
https://hg.mozilla.org/integration/autoland/rev/62cebea1d157
Clean up Stylo Treeherder symbols. r=jmaher

Comment 46

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/7b3215e18443
https://hg.mozilla.org/mozilla-central/rev/d37df1ba1ca9
https://hg.mozilla.org/mozilla-central/rev/ca968311ed00
https://hg.mozilla.org/mozilla-central/rev/d6c23b724e86
https://hg.mozilla.org/mozilla-central/rev/76e1adb5416d
https://hg.mozilla.org/mozilla-central/rev/62cebea1d157

Comment 47

[Joel Maher ( :jmaher ) (UTC -8) (PTO'ish until July 5)]

here is the difference between stylo and non-stylo firefox according to talos:
== Change summary for alert #9238 (as of September 05 2017 20:06 UTC) ==

Regressions:

  4%  damp summary windows7-32 opt e10s     275.34 -> 286.46
  3%  tp5o Private Bytes linux64 pgo e10s   991,235,072.54 -> 1,021,448,764.77
  3%  tp5o Private Bytes linux64 opt e10s   984,508,752.03 -> 1,012,869,617.46
  3%  damp summary windows7-32 pgo e10s     220.16 -> 226.24
  3%  tp5o_webext Main_RSS linux64 opt e10s 188,299,423.56 -> 193,335,479.51
  3%  tp5o_webext Main_RSS linux64 pgo e10s 181,101,414.59 -> 185,885,824.40
  2%  tp5o Main_RSS linux64 opt e10s        186,032,430.68 -> 189,797,789.83

Improvements:

 90%  bloom_basic_singleton summary windows7-32 opt e10s     951.17 -> 98.04
 87%  bloom_basic_singleton summary windows10-64 opt e10s    712.40 -> 92.85
 86%  bloom_basic_singleton summary osx-10-10 opt e10s       782.15 -> 106.48
 83%  bloom_basic summary windows10-64 opt e10s              6.55 -> 1.08
 79%  bloom_basic summary windows7-32 opt e10s               2.86 -> 0.59
 78%  bloom_basic_singleton summary windows7-32 pgo e10s     398.69 -> 87.22
 73%  bloom_basic_singleton summary linux64 pgo e10s         318.11 -> 85.19
 73%  bloom_basic_singleton summary windows10-64 pgo e10s    319.41 -> 86.76
 71%  bloom_basic_singleton summary linux64 opt e10s         309.87 -> 89.94
 27%  tp6_amazon summary windows7-32 opt e10s                957.73 -> 701.92
 27%  tp6_amazon summary windows10-64 opt e10s               779.96 -> 572.29
 23%  tp6_amazon summary windows10-64 pgo e10s               629.33 -> 483.08
 22%  tp6_amazon summary windows7-32 pgo e10s                677.25 -> 530.21
  8%  a11yr summary windows10-64 opt e10s                    593.95 -> 544.29
  8%  tp5o_webext summary windows7-32 opt e10s               323.13 -> 298.30
  7%  tp5o summary windows7-32 opt e10s                      322.72 -> 299.28
  7%  a11yr summary windows7-32 opt e10s                     748.45 -> 694.64
  7%  tp5o_webext summary windows10-64 opt e10s              286.51 -> 266.33
  7%  tp5o summary windows10-64 opt e10s                     289.67 -> 269.89
  5%  a11yr summary linux64 opt e10s                         581.70 -> 549.92
  4%  tp5o summary osx-10-10 opt e10s                        272.70 -> 261.91
  3%  tp5o_webext summary linux64 opt e10s                   293.33 -> 283.50
  3%  tp6_google summary windows7-32 opt e10s                515.08 -> 498.29
  3%  tp5o summary windows10-64 pgo e10s                     226.14 -> 218.98
  3%  tp5o summary windows7-32 pgo e10s                      226.01 -> 219.32

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=9238

do we believe these are accurate?  I am excited to see the large wins, and only some small memory and devtools regressions.

Comment 48

[Bobby Holley (:bholley)]

(In reply to Joel Maher ( :jmaher) (UTC-5) from comment #47)
> do we believe these are accurate?  I am excited to see the large wins, and
> only some small memory and devtools regressions.

Yep! Glad to see it reflected across various other suites I hadn't been tracking.

Comment 49

[Caspy7]

Per the metrics in comment 47, can anyone say how many cores talos is using?

I'm under the impression that Stylo's performance has a somewhat linear correlation to CPU cores.

Comment 50

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to Caspy7 from comment #49)
> Per the metrics in comment 47, can anyone say how many cores talos is using?

It differs by platform and test type.  Here's a summary of cores available and threads used by Stylo in different situations:

https://docs.google.com/spreadsheets/d/1z9aPLTFtFvuLvlDEaD2DFzAKtF2u-miJol-XauO7SOM/edit

Comment 51

[Joel Maher ( :jmaher ) (UTC -8) (PTO'ish until July 5)]

now here is a list of memory regressions:
== Change summary for alert #9239 (as of September 05 2017 20:06 UTC) ==

Regressions:

 26%  Images summary windows7-32 pgo      4,345,721.15 -> 5,457,115.99
 20%  Images summary windows7-32 opt      4,438,344.04 -> 5,329,100.60
 18%  Heap Unclassified summary windows10-64 pgo 47,031,798.71 -> 55,380,832.57
 18%  Images summary windows10-64 opt     5,654,544.77 -> 6,648,518.57
 17%  Heap Unclassified summary windows10-64 opt 46,969,653.15 -> 55,153,061.35
 17%  Images summary osx-10-10 opt        5,379,158.77 -> 6,300,984.78
 14%  Images summary linux64 opt          5,156,052.18 -> 5,868,148.55
 14%  Heap Unclassified summary windows7-32 pgo 39,807,626.24 -> 45,297,380.30
 14%  Heap Unclassified summary windows7-32 opt 39,750,311.57 -> 45,207,843.72
 13%  Heap Unclassified summary linux64 opt 55,363,623.45 -> 62,777,263.63
 10%  Heap Unclassified summary osx-10-10 opt 69,689,260.72 -> 76,409,872.41
  8%  Explicit Memory summary windows7-32 opt 226,844,163.55 -> 244,667,087.05
  8%  Explicit Memory summary windows10-64 pgo 296,065,177.27 -> 318,684,944.36
  7%  Explicit Memory summary windows7-32 pgo 226,196,686.15 -> 243,142,482.22
  7%  Explicit Memory summary linux64 opt 295,081,056.88 -> 315,783,152.15
  7%  Explicit Memory summary windows10-64 opt 296,619,695.65 -> 316,909,818.14
  6%  Resident Memory summary windows7-32 opt 290,401,474.10 -> 309,109,527.62
  6%  Resident Memory summary linux64 opt 424,821,351.92 -> 451,728,421.81
  6%  Explicit Memory summary osx-10-10 opt 308,346,756.13 -> 325,895,170.89
  5%  Resident Memory summary windows10-64 opt 440,113,115.57 -> 463,174,702.35
  5%  Resident Memory summary windows10-64 pgo 432,057,921.38 -> 452,979,401.56

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=9239

can we confirm these are expected

Comment 52

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

The explicit and resident size changes agree with what we were tracking.  But I don't recall discussion of the images size change.  :bholley, do we need to track this?

Comment 53

[Andrew McCreight [:mccr8]]

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #52)
> The explicit and resident size changes agree with what we were tracking. 
> But I don't recall discussion of the images size change.  :bholley, do we
> need to track this?

njn's recent memory reporting work (bug 1394729) caused some memory used by Stylo to be shifted from heap-unclassified to images (as seen in bug 1395576). I'd guess that this is the same thing. So it isn't a problem per se, as long as explicit is still changing as you expected (as images is a subset of explicit), but it might highlight an area that could be targeted for memory improvements.

Comment 54

[Bobby Holley (:bholley)]

Acknowledge on the memory regressions. There will be some regression from stylo, but we should be able to shrink it a good bit.

Nick has a nice explanation for the image stuff in bug 1395576 comment 11.