Opened on 06/04/2018 at 05:56:32 PM

Closed on 06/26/2018 at 09:28:14 AM

Last modified on 09/30/2018 at 09:48:05 AM

#6727 closed change (fixed)

Use string rather than map for single-domain filters

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

Description (last modified by mjethani)


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 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. 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 (, and exceptions (, for no-domain, single-domain, and multi-domain cases.

For example,, should apply on but not on or if there are two filters and then the first one should apply on but not on

Attachments (0)

Change History (18)

comment:1 Changed on 06/04/2018 at 05:58:16 PM by mjethani

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

comment:2 Changed on 06/04/2018 at 05:58:38 PM by mjethani

  • Review URL(s) modified (diff)

comment:3 Changed on 06/05/2018 at 06:12:22 AM by mjethani

  • Owner set to mjethani

comment:4 Changed on 06/05/2018 at 04:52:26 PM by kzar

  • Priority changed from Unknown to P2
  • Ready set

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

comment:5 Changed on 06/26/2018 at 09:25:23 AM by abpbot

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

comment:6 Changed on 06/26/2018 at 09:28:02 AM by mjethani

  • Description modified (diff)

comment:7 Changed on 06/26/2018 at 09:28:14 AM by mjethani

  • Resolution set to fixed
  • Status changed from new to closed

comment:8 Changed on 06/26/2018 at 06:58:03 PM by mjethani

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

comment:9 Changed on 07/20/2018 at 11:55:03 AM by mjethani

  • Description modified (diff)

comment:10 Changed on 07/20/2018 at 12:35:41 PM by 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.

comment:11 Changed on 07/20/2018 at 01:00:50 PM by 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.

comment:12 Changed on 07/20/2018 at 01:28:52 PM by 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.

comment:13 Changed on 07/20/2018 at 01:38:48 PM by 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.

comment:14 Changed on 07/20/2018 at 03:15:56 PM by mjethani

  • Description modified (diff)

comment:15 Changed on 07/20/2018 at 03:16:22 PM by mjethani

Alright, I have added some concrete examples now.

comment:16 Changed on 07/20/2018 at 03:23:56 PM by kzar

Thanks, I appreciate it.

comment:17 Changed on 08/21/2018 at 11:51:54 AM by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

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

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

comment:18 Changed on 09/30/2018 at 09:48:05 AM by mjethani

  • Blocking 7000 added

Add Comment

Modify Ticket

Change Properties
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from mjethani.
Note: See TracTickets for help on using tickets.