Opened 9 months ago

Closed 9 months ago

Last modified 7 months ago

#7244 closed change (fixed)

Maintain cache of domain-specific style sheets

Reported by: mjethani Assignee: mjethani
Priority: P2 Milestone:
Module: Core Keywords:
Cc: Blocked By:
Blocking: #7000 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/29987596/

Description (last modified by mjethani)

Background

On sites with multiple frames from the same known domain (e.g. msn.com), we generate the domain-specific part of the style sheet for every frame and on each page load. A lot of this redundant computation can be avoided if we simply cache the domain-specific part of the style sheet (~750 selectors in EasyList). The size of the cache could be limited to 100 entries. Even at full capacity, this would take up only ~4 MB while significantly cutting down the CPU usage in practice.

In my benchmark loading Alexa Top 50 and profiling for one minute, I was able to reduce the amount of time spent in generateStyleSheetForDomain in half using this strategy.

What to change

In the generateStyleSheetForDomain in lib/elemHide.js, for the common case (not specific-only, no selectors needed), if a known domain suffix is found, cache the generated style sheet by that suffix.

If the cache hits an upper limit of 100 entries, clear it and start over.

Hints for testers

Test that element hiding filters work in general. If there's a filter in EasyList like ###A9AdsMiddleBoxTop, it should get applied to an element <div id="A9AdsMiddleBoxTop">Ad</div>.

Make sure that cached style sheets are cleared when filters are updated. For this, add a custom filter like ##.foo and then see that it is hiding the element <div class="foo">Ad</div> on a test page. Remove the custom filter and make sure the element is no longer getting hidden after a page refresh (without extension restart).

Change History (7)

comment:1 Changed 9 months ago by mjethani

  • Blocking 7000 added
  • Description modified (diff)
  • Ready set
  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:2 Changed 9 months ago by mjethani

  • Owner set to mjethani

comment:3 Changed 9 months ago by abpbot

A commit referencing this issue has landed:
Issue 7244 - Maintain cache of domain-specific style sheets

comment:4 Changed 9 months ago by mjethani

  • Description modified (diff)

comment:5 Changed 9 months ago by mjethani

  • Description modified (diff)
  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:6 Changed 9 months ago by abpbot

A commit referencing this issue has landed:
Issue 7244 - Maintain cache of domain-specific style sheets

comment:7 Changed 7 months ago by rscott

  • Verified working set
Note: See TracTickets for help on using tickets.