Clean up caches when removing filters

[greiner]

Background

We use various caches to optimize operations such as Filter.fromText() but we don't free some of those when filters get removed. Therefore we should ensure that we clean up in such situations without sacrificing optimizations.

Heap snapshot comparisons have shown that cleaning up ElemHide.exceptions, Filter.knownFilters and Subscription.knownSubscriptions (due to their respective references to Filter objects) eliminates all strings, arrays, Filter and Subscription objects that were created when adding new filters or subscriptions.

Note that the approach suggested below does not clean up after filters that don't belong to a subscription. Those ones would still be leaking memory.

What to change

• Remove all filters from Filter.knownFilters that don't belong to a subscription whenever filters are removed. For that, add a Filter.forget() method and call it in filterListener.js
• Remove all selectors from ElemHide.exceptions that don't have any filters belonging to them anymore. For that, add this logic to ElemHide.remove().
• Remove all subscriptions from ElemHide.knownSubscriptions that are not contained in FilterStorage.knownSubscriptions. For that, add a Subscription.forget() method and call it in filterListener.js
• Add unit tests to test this functionality:
  • Remove all filter lists (including Acceptable Ads), custom filters and whitelisted domains
  • Add the custom filter foo to ensure that the extension doesn't reset when reloading it
  • Reload extension
  • Observe JavaScript heap after garbage collection

• Testing subscriptions
  • Add "EasyList Germany+EasyList" filter list
  • Remove the filter list again
  • Observe JavaScript heap after garbage collection

• Testing custom filters
  • Execute the following in the context of the background page: for (var i = 0; i < 10000; i++) FilterStorage.addFilter(Filter.fromText("foo" + i));
  • Execute the following in the context of the background page: for (var i = 0; i < 10000; i++) FilterStorage.removeFilter(Filter.fromText("foo" + i));
  • Observe JavaScript heap after garbage collection

[kzar]

Will this suggestion still be relevant after we switch to emscripten sergz?

[sergz]

Replying to kzar:

Will this suggestion still be relevant after we switch to emscripten sergz?

Currently in emscripten Filter and Subscription objects are supposed to be removed from such caching structures when they are being destroyed, however IMO it still good to have tests checking that there are at least no bugs (memory leaks). In addition such tests can be useful because I would not exclude the chance that the implementation will be changed there.

[kzar]

...however IMO it still good to have tests checking that there are at least no bugs (memory leaks). In addition such tests can be useful because I would not exclude the chance that the implementation will be changed there.

Alright I've modified the issue description to mention that these tests should be written.

[sebastian]

Sorry, but we switched to GitLab. If this issue is still relevant, please file it again in the ​new issue tracker.