1320872 - 10-20X regression in rule-processor-cache memory usage in ABP 2.8.0 compared to 2.7.3

Status: RESOLVED INACTIVE

(Firefox :: Extension Compatibility, defect)

Description

[Cameron McCormack (:heycam)]

Attachment: user's memory usage report

A user reports that ABP 2.8.0 is using significantly more memory than 2.7.3.  See the attachment for their report.

Maybe something changed in the way ABP generates/uses style sheets that defeats the rule processor sharing work from bug 988266?

Comment 1

[Wladimir Palant]

Yes, starting with Adblock Plus 2.8.0 @-moz-document rules are no longer being used. There is still one shared stylesheet containing all the rules which are common to all domains (loaded via nsIStyleSheetService.preloadSheet() and applied via nsIDOMWindowUtils.addSheet()). But there is also a much smaller site-specific stylesheet applied via nsIDOMWindowUtils.loadSheetUsingURIString() which cannot be cached. This change addresses a number of consistency issues but it is also expected to increase the memory footprint to some degree - should not be very noticeable for realistic use cases, and by far not as bad as the upcoming WebExtensions move.

Comment 3

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

If I visit [1], ABP 2.8 takes around 600MB in rule-processor-cache. ABP 2.7.3 has no such problem. Do you know why this particular page is such a problem? Also, what's the advantage of moving away from @-moz-document?

[1] http://metalelf0.github.io/VimColorSchemeTest-Ruby/python.html

Comment 4

[Eric Rahm [:erahm]]

(In reply to Wladimir Palant from comment #1)
> should not be very noticeable for realistic use cases

It looks like this is more common than expected.

For reference the relevant measurement is:

> ABP 2.7.3
> just HC set (~72 tabs)
> 35.11 MB (04.77%) ── rule-processor-cache (plus 15.28 MB (01.94%) ── style-sheet-service)
> 
> ABP 2.8
> just HC set (~72 tabs)
> 308.64 MB (29.13%) ── rule-processor-cache
> close, no minimize mem
> 608.95 MB (45.72%) ── rule-processor-cache

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

So we have one bug report showing 305MB-608MB, and another showing 618MB.

It's worth noting that the rule processor is per-cascade-level.  So if you're introducing a small site-specific sheet as a user style sheet that's different for each site, we'll end up caching the *entire* set of cascade results separately for each site, including the cache of the large sheet as well.  (This is fundamentally necessary, since the CSS Cascade mixes the rules together, and the object we're caching is the result of the CSS cascade.)

Furthermore, if the sheet is reloaded by URI rather than the CSSStyleSheet* object cached, then we won't even share them within different pages for the same "site", since the key to the sheets is the outer object returned from the loading, not the copy-on-write-shared inner object that's the same for multiple loads of the same style sheet.

Comment 6

[Wladimir Palant]

(In reply to Bill McCloskey (:billm) from comment #3)
> Do you know why this particular page is such a
> problem?

Because it's not a single page but rather at least 156 pages - lots of iframes.

> Also, what's the advantage of moving away from @-moz-document?

@-moz-document doesn't have any usable way to handle exceptions, for rules that apply everywhere but on a bunch of domains. We had to resort to fragile -moz-binding tricks which caused nasty side-effect.

Also, generating an individual list of rules for each site is how we do it on all platforms, and also how we will have to do it on Firefox anyway once we move over to Web Extensions.

(In reply to David Baron :dbaron: ⌚️UTC-8 from comment #5)
> It's worth noting that the rule processor is per-cascade-level.  So if
> you're introducing a small site-specific sheet as a user style sheet that's
> different for each site, we'll end up caching the *entire* set of cascade
> results separately for each site, including the cache of the large sheet as
> well.

What is a cascade level here? Do you mean that all user stylesheets are cached together?

Comment 7

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

This is a pretty significant regression. I'm told that scrolling through a long Twitter feed can cause the rule processor cache to grow by hundreds of megabytes.

Wladimir, if we added a @-moz-not-document at-rule, would that allow you to switch back to a single CSS file? This solution would work in the future with WebExtensions, although it would not be Chrome compatible.

Comment 8

[Cameron McCormack (:heycam)]

(In reply to Wladimir Palant from comment #6)
> What is a cascade level here? Do you mean that all user stylesheets are
> cached together?

Yes, the result of cascading all of the sheets at the User level is what is cached.  If you have five documents, each with slightly different style sheets or sets of style sheets loaded at the User level, then there won't be any sharing of the cached data.

If not using @-moz-document, then what would allow the cached data to be re-used is if the non-site-specific rules (which is most of the rules) are loaded at one level of the cascade and the site-specific rules in another.  The current caching mechanism is only used for Agent and User level sheets, though, and it's probably not great to add ABP rules to the Agent level.

Still, I don't know what the WebExtensions API looks like for style sheet loading (can someone point me to some docs for that?  I had trouble finding them when I looked recently) and perhaps that API doesn't let you load sheets at different cascade levels anyway.

Comment 9

[David Durst [:ddurst]]

Moving...

Comment 10

[David Durst [:ddurst]]

(Actually, I'm going to resolve this as inactive based on age and leave it where it was.)