Mayank Bansal Reporter
	Description • 7 years ago

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

	Mayank Bansal Reporter
	Comment 1 • 7 years ago

about:memory with stylo disabled

	Mayank Bansal Reporter
	Comment 2 • 7 years ago

about:support

	Mayank Bansal Reporter
	Comment 3 • 7 years ago

about:memory was taken after doing "minimize memory"
Also, I suspect AdblockPlus as involved somehow.

	Mayank Bansal Reporter
	Comment 4 • 7 years ago

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

	Mayank Bansal Reporter
	Comment 5 • 7 years ago

ABP enabled STYLo disabled

	Mayank Bansal Reporter
	Comment 6 • 7 years ago

ABP disabled. Stylo disabled

	Mayank Bansal Reporter
	Comment 7 • 7 years ago

My filter lists for ABP

	Mayank Bansal Reporter
	Comment 8 • 7 years ago

with a fresh profile. Imported the filters from old profile.
Stylo enabled. ABP enabled

	Mayank Bansal Reporter
	Comment 9 • 7 years ago

Fresh profile. Filters imported from old profile.
Stylo enabled. ABP disabled.

	Bobby Holley (:bholley)
	Comment 10 • 7 years ago

Thanks for the report! I suspect bug 1368290 will improve this situation significantly - we should finish that and remeasure.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 11 • 7 years ago

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

	Danial Horton
	Comment 12 • 7 years ago

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)

	Chris Peterson [:cpeterson]
	Comment 13 • 7 years ago

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.

	Danial Horton
	Comment 14 • 7 years ago

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?

	Mayank Bansal Reporter
	Comment 15 • 7 years ago

(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.

	Danial Horton
	Comment 16 • 7 years ago

(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.

	Danial Horton
	Comment 17 • 7 years ago

webextension versions can be obtained from https://downloads.adblockplus.org/devbuilds/adblockplusfirefox/

	Danial Horton
	Comment 18 • 7 years ago

Can confirm that webextension versions of ABP are also demonstrating this issue.

	Bobby Holley (:bholley)
	Comment 19 • 7 years ago

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).

	Mayank Bansal Reporter
	Comment 20 • 7 years ago

Can not reproduce with todays Nightly: https://hg.mozilla.org/mozilla-central/rev/63e261ce8cb04c913d2e6b19ea451b7078d24dc1

	Mayank Bansal Reporter
	Comment 22 • 7 years ago

(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

	Mayank Bansal Reporter
	Comment 23 • 7 years ago

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.

	Mayank Bansal Reporter
	Comment 24 • 7 years ago

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.

	Mayank Bansal Reporter
	Comment 25 • 7 years ago

spoke to soon. There is still heap-unclassified, but the amount seems to have reduced by ~100MB.

	Bobby Holley (:bholley)
	Comment 26 • 7 years ago

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.

	Mayank Bansal Reporter
	Comment 27 • 7 years ago

Also happens on gmail. Stylo+ABP on gmail uses ~1200MB.

	Chris Peterson [:cpeterson]
	Comment 28 • 7 years ago

Boris, this is the Adblock Plus memory usage bug that Bobby mentioned yesterday.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 29 • 7 years ago

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.

	Mayank Bansal Reporter
	Comment 32 • 7 years ago

(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.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 33 • 7 years ago

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

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 34 • 7 years ago

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

	Mayank Bansal Reporter
	Comment 35 • 7 years ago

(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.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 36 • 7 years ago

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.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 37 • 7 years ago

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

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 38 • 7 years ago

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)...

	Mayank Bansal Reporter
	Comment 42 • 7 years ago

(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.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 43 • 7 years ago

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.

	Mayank Bansal Reporter
	Comment 44 • 7 years ago

This seems much better now!
This is whatwg page, with stylo+ABP.

	Bobby Holley (:bholley)
	Comment 45 • 7 years ago

Ok. Resolving this as filed, let's move further discussion of memory improvements into bug 1367854.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 46 • 7 years ago

Fwiw, I spun bug 1397380 off specifically to look at the html spec with stylo vs no-stylo.