Closed Bug 1384945 Opened 2 years ago Closed 2 years ago

stylo: With ABP installed and Stylo enabled, on HTML Spec page, nightly uses 900MB RAM with 500MB in Heap-unclassified

Categories

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

defect

Tracking

()

RESOLVED WORKSFORME
Tracking Status
firefox-esr52 --- unaffected
firefox55 --- unaffected
firefox56 --- wontfix
firefox57 --- affected

People

(Reporter: mayankleoboy1, Assigned: bzbarsky)

References

()

Details

(Whiteboard: [MemShrink:P1])

Attachments

(16 files, 1 obsolete file)

165.92 KB, application/x-gzip
Details
150.54 KB, application/x-gzip
Details
25.35 KB, text/plain
Details
240.88 KB, application/x-gzip
Details
214.90 KB, application/x-gzip
Details
3.88 MB, text/plain
Details
156.14 KB, application/x-gzip
Details
134.89 KB, application/x-gzip
Details
167.26 KB, application/x-gzip
Details
167.93 KB, application/x-gzip
Details
128.84 KB, application/x-gzip
Details
93.39 KB, application/x-gzip
Details
106.59 KB, application/x-gzip
Details
107.92 KB, application/x-gzip
Details
76.33 KB, application/x-gzip
Details
151.79 KB, application/x-gzip
Details
Attached file memory-report.json.gz
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:56.0) Gecko/20100101 Firefox/56.0
Build ID: 20170727100347

Steps to reproduce:

Using my normal profile
1. Start Nightly
2. Go to https://html.spec.whatwg.org/  measure the memory
3. go to about:config and enable stylo . Dont restart, but open a new tab
4. In the new tab open https://html.spec.whatwg.org/ . measure memory.


Actual results:

900MB usage with stylo enabled. large part in heap-unclassified


Expected results:

probably not so
about:memory with stylo disabled
Attached file aboutsupport.txt
about:support
Flags: needinfo?(bobbyholley)
Summary: With Stylo, on HTML Spec page, nightly uses 900MB RAM with 500MB in Heap-unclassified → stylo: With Stylo, on HTML Spec page, nightly uses 900MB RAM with 500MB in Heap-unclassified
about:memory was taken after doing "minimize memory"
Also, I suspect AdblockPlus as involved somehow.
yes, ABP is involved.

STR:
1. Enable stylo. Enable ABP
2. Go to https://html.spec.whatwg.org/
3. See large ~1GB memory use.

4. Disable ABP from add-on manager
5. refresh the HRML wpec page by pressing f5 or ctrl+shift+r
6. Memory reduces to ~500MB

7 Enable ABP again
8. refresh the HRML wpec page by pressing f5 or ctrl+shift+r
9. Memory again reaches ~1GB
10. do "minimize memory". Nothing much happens
Summary: stylo: With Stylo, on HTML Spec page, nightly uses 900MB RAM with 500MB in Heap-unclassified → stylo: With ABP installed and Stylo enabled, on HTML Spec page, nightly uses 900MB RAM with 500MB in Heap-unclassified
Attached file ABP no stylo.json.gz
ABP enabled STYLo disabled
Attached file no ABP no Stylo.gz
ABP disabled. Stylo disabled
Attached file MyFilters.ini
My filter lists for ABP
with a fresh profile. Imported the filters from old profile.
Stylo enabled. ABP enabled
Fresh profile. Filters imported from old profile.
Stylo enabled. ABP disabled.
Thanks for the report! I suspect bug 1368290 will improve this situation significantly - we should finish that and remeasure.
Depends on: 1368290
Flags: needinfo?(bobbyholley)
Apart from abp maybe reducing element style sharing, why would it being enabled or disabled matter here?

We should definitely remeasure after bug 1368290, and compare the measurements to Gecko, both with and without abp...
Priority: -- → P2
I wonder if this might also help a case where certain adult websites consume an entire multi-core cpu when trying to play video when stylo is enabled with ABP hiding certain elements (such as ##NONE.nonesuch)
Does this memory problem only happen with the legacy (non-WebExtension) add-on version of ABP? Legacy add-ons will no longer be supported in Firefox 57, so this bug might no longer be relevant.
Firefox probably won't be relevant either come 57, so lets not go there.

Anyone got a link to the inferior cut down and mostly broken web extension version to try?
(In reply to Chris Peterson [:cpeterson] from comment #13)
> Does this memory problem only happen with the legacy (non-WebExtension)
> add-on version of ABP? Legacy add-ons will no longer be supported in Firefox
> 57, so this bug might no longer be relevant.


I tried this in a new profile (which AFAIK disables legacy addons), and installed ABP from their website.
So I assume Webextension version of ABP would have been installed.
(In reply to Mayank Bansal from comment #15)
> (In reply to Chris Peterson [:cpeterson] from comment #13)
> > Does this memory problem only happen with the legacy (non-WebExtension)
> > add-on version of ABP? Legacy add-ons will no longer be supported in Firefox
> > 57, so this bug might no longer be relevant.
> 
> 
> I tried this in a new profile (which AFAIK disables legacy addons), and
> installed ABP from their website.
> So I assume Webextension version of ABP would have been installed.

No, that doesn't seem to be the case at all.
webextension versions can be obtained from https://downloads.adblockplus.org/devbuilds/adblockplusfirefox/
Can confirm that webextension versions of ABP are also demonstrating this issue.
Priority: P2 → --
Bug 1382925 may also help here, though that part should be somewhat orthogonal to the size of the page being loaded (and more related to the total number of pages open, unless the html spec has a lot of iframes).
Depends on: 1382925
Priority: -- → P2
Duplicate of this bug: 1386935
Whiteboard: [MemShrink]
(In reply to Mayank Bansal from comment #20)
> Created attachment 8893422 [details]
> memory-report.json.gz
> 
> Can not reproduce with todays Nightly:
> https://hg.mozilla.org/mozilla-central/rev/
> 63e261ce8cb04c913d2e6b19ea451b7078d24dc1

This is because in todays build, ABP is slightly broken.
See bug 1387101
Attached file memory-report.json.gz
with this build:  https://hg.mozilla.org/mozilla-central/rev/a921bfb8a2cf3db4d9edebe9b35799a3f9d035da
and using the latest experimental ABP webext version.

anecdotally, I see 50-150 MB less memory usage on the whatwg page now.
Whiteboard: [MemShrink] → [MemShrink:P1]
Attached file memory-report.json.gz
This is with todays Nightly https://hg.mozilla.org/mozilla-central/rev/564e82f0f289af976da01c2d50507017bbc152b5

It doesnt have any heap-unclassified
Total memory usage is similar to the report attached before.

Ni? so that you can better analyse the memory report now.
Flags: needinfo?(bobbyholley)
spoke to soon. There is still heap-unclassified, but the amount seems to have reduced by ~100MB.
Redirecting NI to Cameron, who is looking at memory stuff.

I did some memory report diffs with ABP with and without stylo. Stylo definitely uses more memory on CVs and style structs without ABP, and the delta grows with ABP installed. I wouldn't really expect that - I'd expect more memory usage in the stylist data structures, but the html spec doesn't have ads, so I wouldn't think we'd match very many of the selectors (and thus generate more style structs). So this definitely needs investigation, and probably more measurements after/during the fix for bug 1367635.
Flags: needinfo?(bobbyholley) → needinfo?(cam)
Also happens on gmail. Stylo+ABP on gmail uses ~1200MB.
Boris, this is the Adblock Plus memory usage bug that Bobby mentioned yesterday.
Flags: needinfo?(bzbarsky)
Assignee: nobody → bzbarsky
OK, so I did some measurements on a pretty tip-ish build (rev 1320c22e2179) on Linux, with the patches from https://github.com/servo/servo/pull/18384 and https://bugzilla.mozilla.org/show_bug.cgi?id=1387958 applied.  These were with https://addons.mozilla.org/en-US/firefox/addon/adblock-for-firefox/?src=ss installed, which I think is the webextension version of abp.  There were some weird errors being dumped to stderr, so I'm not sure how well it actually _worked_.

I'll attach the memory reports, but the headline numbers are like so:

  No stylo: 278MB total memory usage in content process.  Of that, 208MB under the window
            object for the HTML spec page, 24.5M heap-unclassified, 7.43MB heap-overhead.

  Stylo: 337MB total memory usage in content process.  Of that, 247MB under the window
         object for the HTML spec page, 34MB heap-unclassified, 14MB heap-overhead.

The vast majority of the heap-overhead difference is "bin-unused", aka heap fragmentation.

Most of the "under the window object" difference is pretty much all due to more memory used for "computed-values" (25MB) than Gecko uses for "style-contexts" (4MB) and more memory used for style structs (24.5MB vs 3MB).  That in fact accounts for the entirety of the "under the window object" difference, as far I can tell.

The overall regression here from stylo is about 60MB on that 278MB baseline, which is about 22%.  I'll poke at this with DMD, but chances are that will win us at most 10MB.  ~40MB is ComputedValues and style structs.  6-7MB is heap-overhead bits, and the rest is largely noise, afaict (e.g. a 1MB bigger "shared-immutable-strings-cache" under js-nonwindow-runtime).

I guess I'll also look into what sort of ComputedValues we're allocating here, but I expect that we get worse sharing on elements and as a result worse sharing on text and that this accounts for most of the difference between the ComputedValues and style contexts.
(In reply to Boris Zbarsky [:bz] (still digging out from vacation mail) from comment #29)
> OK, so I did some measurements on a pretty tip-ish build (rev 1320c22e2179)
> on Linux, with the patches from https://github.com/servo/servo/pull/18384
> and https://bugzilla.mozilla.org/show_bug.cgi?id=1387958 applied.  These
> were with
> https://addons.mozilla.org/en-US/firefox/addon/adblock-for-firefox/?src=ss
> installed, which I think is the webextension version of abp.  There were
> some weird errors being dumped to stderr, so I'm not sure how well it
> actually _worked_.



https://addons.mozilla.org/en-US/firefox/addon/adblock-for-firefox/?src=ss is not ABP. Please install ABP from https://downloads.adblockplus.org/devbuilds/adblockplusfirefox/ . Also, for an easier page than the WHATWG, you can try on Gmail.
So as far as style contexts vs ComputedValues goes, for Gecko we have:

  12113 elements
  10651 text
   4630 other

For stylo we have:

  49909 elements
  33005 text
  19051 other

For stylo with STYLO_THREADS=1 we have:

  23699 elements
  17853 text
  13803 other

which about matches other pages, actually: STYLO_THREADS=1 is about 2x worse than Gecko at number of ComputedValues/stylecontexts, and multithreaded stylo another 2x worse than that...
> Please install ABP from https://downloads.adblockplus.org/devbuilds/adblockplusfirefox/ 

Ah, I see.  No nightly-compatible abp on AMO yet?  I'll test the one you link to, thank you.  Need to wait for a build without the instrumentation I used to get comment 33 to complete.

> Also, for an easier page than the WHATWG, you can try on Gmail.

That's bug 1392314 and I plan to look into it tomorrow.
(In reply to Boris Zbarsky [:bz] (still digging out from vacation mail) from comment #34)

You might have to do a ctrl+shift+r (force reload) on the whatwg page if the issue does not repro on the first attempt. 



> That's bug 1392314 and I plan to look into it tomorrow.
AFAIU, that bug is about GMAIL+STYLO. But GMAIL+STYLO+ABP is similar to this bug, and the memory bloat is worse than on whatwg page.
OK, so now using "Adblock Plus for Firefox 2.99.0.1837beta" from the page linked in comment 32, I get numbers like so (all for the content process):

  No stylo: 279MB total memory usage in content process.  Of that, 203MB under the window
            object for the HTML spec page, 26MB heap-unclassified, 6.84MB heap-overhead.

  Stylo: 353MB total memory usage in content process.  Of that, 255.5MB under the window
         object for the HTML spec page, 35MB heap-unclassified, 20MB heap-overhead.

  Stylo, no ABP: 331MB total memory usage, 248MB under the window object, 32MB heap-unclassified,
                 13MB heap-overhead,

Diff shows basically the same thing as before but a bit more so: 28MB of ComputedValues, 30MB of servo style structs, 10MB unclassified difference, 13MB heap-overhead difference.  This time in addition to the bin-unused we have 6MB of page-cache in there, in the "stylo + abp" case.

In any case, ABP doesn't seem to be making things that much worse (anymore?) at first glance.
> You might have to do a ctrl+shift+r (force reload) on the whatwg page if the issue does not repro on the first attempt. 

Please do note the patches I mentioned in comment 29 that I have applied.  Those reduce the memory overhead of ABP here by a lot; without those I was seeing much larger numbers.  Those patches landed today, so should be in nightlies tomorrow....
Also, I looked at the memory report from comment 24.  That one shows 280MB of heap-unclassified and 697MB overall.  Again, Emilio's patches I mentioned above help a lot with heap-unclassified bits here (both adding memory reporters and removing a large source of stylo allocations that abp would tickle)...
(In reply to Boris Zbarsky [:bz] (still digging out from vacation mail) from comment #37)
> > You might have to do a ctrl+shift+r (force reload) on the whatwg page if the issue does not repro on the first attempt. 
> 
> Please do note the patches I mentioned in comment 29 that I have applied. 
> Those reduce the memory overhead of ABP here by a lot; without those I was
> seeing much larger numbers.  Those patches landed today, so should be in
> nightlies tomorrow....

That sounds great! I will also test from my end and report back.
And just as an experiment, I backed out the patches I mention above and tried the dev adblock plus with stylo, but without those patches.  Then I get:

    380MB total memory usage in content process.  Of that, 243MB under the window
    object for the HTML spec page, 76MB heap-unclassified, 14MB heap-overhead.

I tried shift-reloading a few times, and I get a bit more heap-unclassified sometimes (up to 150MB) but more or less the same numbers otherwise.  Again, that's without the patches for https://github.com/servo/servo/pull/18384 applied.
Blocks: 1367854
Flags: needinfo?(bzbarsky)
Attached file memory-report.json.gz
This seems much better now!
This is whatwg page, with stylo+ABP.
Ok. Resolving this as filed, let's move further discussion of memory improvements into bug 1367854.
Status: UNCONFIRMED → RESOLVED
Closed: 2 years ago
Resolution: --- → WORKSFORME
Fwiw, I spun bug 1397380 off specifically to look at the html spec with stylo vs no-stylo.
Flags: needinfo?(cam)
You need to log in before you can comment on or make changes to this bug.