Use string rather than map for single-domain filters

[mjethani]

Background

Adblock Plus's memory footprint is an important concern. One of the areas where this issue can be tackled is the domains property of the ActiveFilter class. The Map instance for ActiveFilter.domains is quite heavy; even with only 3 entries, it can have a shallow size of ~150 bytes, which for ~65,000 filters is ~10 MB on the heap. We can reduce the memory usage here by replacing all Map instances for single-domain filters (e.g. example.com##.foo) with just the domain name. In my tests, this saves ~3 MB on the heap with EasyList+AA.

Out of ~30,000 element hiding and emulation filters and exceptions in EasyList+AA, ~12,000 are single-domain cases.

What to change

In the domains getter of the ActiveFilter class prototype, for the single-domain case, set the value of the property to just the name of the domain. Update the domains getter to return a temporary Map instance for the single-domain case.

Hints for testers

Make sure that domain matching still works correctly, for both blocking and hiding filters, with domain exclusions (example.com,~www.example.com##.foo) and exceptions (example.com#@#.foo), for no-domain, single-domain, and multi-domain cases.

For example, example.com,~www.example.com##.foo should apply on ​https://example.com/ but not on ​https://www.example.com/ or if there are two filters ##.foo and www.example.com#@#.foo then the first one should apply on ​https://example.com/ but not on ​https://www.example.com/

[mjethani]

@kzar Sergei seems to agree that this is a good idea, what do you think?

[kzar]

Sounds like a good idea to me, if we can save even 2 MB for our users then we should do it.

[abpbot]

A commit referencing this issue has landed:
​Issue 6727 - Use string rather than map for single-domain filters

[mjethani]

With changes like these, we have brought the memory footprint down by ~9.2 MB since ​changeset e59042524857.

[kzar]

Please could you briefly explain what should happen for those examples? I just mean to give a couple of examples of domains which a filter would/wouldn't apply to with the examples. That would make testing much easier I think.

[mjethani]

@kzar don't the testers already know what should happen? This is not a new feature, we have only modified an implementation detail that should have no visible effect on the outcome.

[kzar]

Well, the idea is to make life as easy as possible for the testers. It'll speed up the QA process, and makes regressions slipping through less likely. It's good you gave some example filters, they very quickly illustrate roughly what needs to be tested. If you give some quick examples of what the desired outcomes (blocked/not blocked domains) are expected to be it'll help even more.

[mjethani]

But aren't these desired outcomes already covered by the current tests? I would say just run all the tests again to make sure everything is in order. There is no new functionality, the old functionality is already covered by the existing tests, so I'm not sure what I should add here.

[mjethani]

Alright, I have added some concrete examples now.

[kzar]

Thanks, I appreciate it.

[Ross]

Done. Domain matching still appears to be working correctly for both hiding and blocking filters (plus exceptions)

ABP 3.2.0.2103
Chrome 68 / 55 / 49 / Windows 10
Firefox 61 / 55 / 51 / Windows 10