1392314 - stylo: causes memory consumption to double for gmail

Status: VERIFIED FIXED

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

Description

[Clemens Eisserer]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:56.0) Gecko/20100101 Firefox/56.0
Build ID: 20170819205558

Steps to reproduce:

1. enabled stylo/servo: layout.css.servo.enabled -> true
2. restarted firefox
3. opened my (quite large) gmail inbox


Actual results:

with stylo/servo enabled, the WebContent Process consumes twice the memory compared to before enabling servo (only 1 gmail tab open!):

PID      USER      PR  NI    VIRT    RES    SHR S  %CPU %MEM     TIME+ COMMAND  
13715 ce        20   0 2805724 659432 132376 S  86.4 11.2   0:20.38 firefox-bin                                                
13793 ce        20   0 2988672 845312 121472 R  19.9 14.3   0:18.94 Web Content   




Expected results:

servo/stylo should use aprox. the same amount of RAM for gmail than the old implmentation.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Could you post both about:memory, with and without stylo? Thanks!

Comment 2

[Darkspirit]

"Firefox/56.0" - So you are using Beta?
If I'm not wrong, all recent optimizations were done in Nightly 57 and not uplifted to Beta 56.
Could you check if https://nightly.mozilla.org is better?

Comment 3

[Nathan Froyd [:froydnj]]

A couple of questions:

1) Are these measurements with a clean (preferably an entirely new) profile?
2) Have you looked at about:memory numbers for the two use cases?  If you could provide those as well, that would be most helpful.
3) Can you provide actual numbers for the Web Content process without stylo enabled?  There are several memory-related numbers in the above, and it's not clear which ones you're referencing.
4) Did you try to ensure that both processes were in a comparable state before measuring memory?

Comment 4

[Astley Chen (inactive)]

Attachment: gecko-on-gmail.json.gz

about:memory measure saved on conditions that stylo is off and load gmail inbox with a clean profile.

Web Content process memory usage is around 190MB.

Comment 5

[Astley Chen (inactive)]

Attachment: stylo-on-gmail.json.gz

about:memory measure saved on conditions that stylo is *ON* and load gmail inbox with a clean profile.

Web Content process memory usage is around ~240~250MB in several attempts.

Comment 6

[Astley Chen (inactive)]

The data was collected from Nightly 57.0a1 (2017-08-20) (64-bit) on macOS 10.12.6.

Comment 7

[Astley Chen (inactive)]

There is also a stylo meta bug 1293767 which is chiefly tracking on memory footprint issues.
It would make sense to me to track the bugs akin at one place.

Comment 8

[Bobby Holley (:bholley)]

This is another one possibly related to bug 1367854. Boris and Cameron are going to have a look.

Comment 9

[Bobby Holley (:bholley)]

(Marking P3 for now until we know whether it's distinct from the work in bug 1367854).

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: Gmail about:memory, no stylo

Comment 11

[Boris Zbarsky [:bzbarsky]]

Attachment: Gmail about:memory, STYLO_THREADS=1

Comment 12

[Boris Zbarsky [:bzbarsky]]

Attachment: Gmail about:memory, STYLO_THREADS=3

Comment 13

[Boris Zbarsky [:bzbarsky]]

Attachment: Gmail about:memory, STYLO_THREADS=6

Comment 14

[Boris Zbarsky [:bzbarsky]]

Comparing those stylo about:memory logs to the no-stylo one, I see a 23MB increase on a 209MB baseline from no-stylo to 3-threads stylo.  The diff is a bit confusing to read because the urls of the various hangouts junk gmail loads keep changing, but the upshot is that "style-sheets" is smaller with servo, and servo-style-sets is smaller than gecko-style-sets.  servo-style-structs is 2x larger than gecko-style-structs there, but that's a difference between 400KB and 800KB, which is not much.  Similarly, servo computed-value is 0.34MB vs Gecko style-contexts at 0.11MB, but that's pretty minor.

The big difference is the 38MB increase in heap-unclassified from Gecko to stylo.  Going to look at DMD bits.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Attachment: Gzipped dmd diff output for non-stylo vs stylo (probably with 12 threads, in case it matters) on gmail

Comment 16

[Boris Zbarsky [:bzbarsky]]

OK, so looking at the DMD output we have:

1) At least 5MB coming from background::parse_value doubling vecs.
2) At least 1.5MB directly from QualifiedRuleParser::parse_block.
3) At least 1.3MB from PropertyDeclarationBlock::extend_common reserving space in a vec.

etc, etc.  It all looks like stylesheet data, in my poking at it so far.  I thought we had memory reporting for the servo stylesheets...  I guess njn was just poking at this and discovering that grouping rules are not reported well; maybe that's what's going on...

Comment 17

[Nicholas Nethercote [inactive]]

> 3) At least 1.3MB from PropertyDeclarationBlock::extend_common reserving
> space in a vec.

I think https://github.com/servo/servo/pull/18400 will help with this, because it adds measurement of PageRule::block.

Comment 18

[Nicholas Nethercote [inactive]]

> 1) At least 5MB coming from background::parse_value doubling vecs.

My measurements gave more like 11 MiB, or even more. https://github.com/servo/servo/pull/18404 is measuring this.

Comment 19

[Boris Zbarsky [:bzbarsky]]

OK.  On current tip, but not including the fix for bug 1398322, I see numbers like so:

stylo:

  PID USER      PR  NI    VIRT    RES    SHR S  %CPU %MEM     TIME+ COMMAND
 6562 bzbarsky  20   0 2469176 416200 136396 S   2.3  1.3   0:06.01 Web Content
 6491 bzbarsky  20   0 2185912 286176 132328 S   1.3  0.9   0:03.62 firefox

non-stylo:

  PID USER      PR  NI    VIRT    RES    SHR S  %CPU %MEM     TIME+ COMMAND
 6776 bzbarsky  20   0 2432680 422096 132684 S  63.9  1.3   0:04.85 Web Content
 6705 bzbarsky  20   0 2194064 286012 128152 S  21.2  0.9   0:03.01 firefox

so I think this is fixed at this point; we're about at parity on gmail.

Clemens, do you want to confirm on your end, probably in tomorrow's nightly, once all these pieces merge to m-c?

Comment 20

[Bobby Holley (:bholley)]

Optimistically resolving fix. Clemens, please set VERIFIED when you test.

Comment 21

[Clemens Eisserer]

Thanks for fixing this issue :)