Issue 7094 - Keep subscription filters by text only

lib/filterListener.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 14 matching lines @@
 const {Services} = Cu.import("resource://gre/modules/Services.jsm", {});
 const {XPCOMUtils} = Cu.import("resource://gre/modules/XPCOMUtils.jsm", {});
 
 const {filterStorage} = require("./filterStorage");
 const {filterNotifier} = require("./filterNotifier");
 const {ElemHide} = require("./elemHide");
 const {ElemHideEmulation} = require("./elemHideEmulation");
 const {ElemHideExceptions} = require("./elemHideExceptions");
 const {Snippets} = require("./snippets");
 const {defaultMatcher} = require("./matcher");
-const {ActiveFilter, RegExpFilter,
+const {Filter, ActiveFilter, RegExpFilter,
        ElemHideBase, ElemHideFilter, ElemHideEmulationFilter,
        SnippetFilter} = require("./filterClasses");
 const {SpecialSubscription} = require("./subscriptionClasses");
 const {Prefs} = require("prefs");
 
 /**
  * Increases on filter changes, filters will be saved if it exceeds 1.
  * @type {number}
  */
 let isDirty = 0;
@@ skipping 186 matching lines @@
     return;
 
   let current = (Math.random() * len) | 0;
   let step;
   do
   {
     step = primes[(Math.random() * primes.length) | 0];
   } while (len % step == 0);
 
   for (let i = 0; i < len; i++, current = (current + step) % len)
-    addFilter(subscription.filterAt(current));
+  {
+    let filter = Filter.fromText(subscription.filterTextAt(current));
+    filter.addSubscription(subscription);
+    addFilter(filter);
+  }
 }
 
 function onSubscriptionAdded(subscription)
 {
   FilterListener.setDirty(1);
 
   if (!subscription.disabled)
     addSubscriptionFilters(subscription);
 }
 
 function onSubscriptionRemoved(subscription)
 {
   FilterListener.setDirty(1);
 
   if (!subscription.disabled)
   {
-    for (let filter of subscription.filters())
-      removeFilter(filter);
+    for (let text of subscription.filterText())
+      removeFilter(Filter.fromText(text));

[hub, 2018/11/30 01:18:07]

here remove filter takes longer since we have to go with `Filter.fromText()`
first...

[Manish Jethani, 2018/11/30 07:28:05]

Yes, this is a cost of this change, and by itself there is no benefit in fact.
But if we take the changes in aggregate, then this is the tradeoff:

 * significantly fewer filter objects in memory (~10 MB)
 * the filter objects that don't exist (i.e. "ephemeral") need to be recreated,
or at least the filter text needs to be parsed again
 * the subscriptions to which a filter belongs need to be found again by looking
up the filter text in each known subscription

In the current patch it's just a lookup in `Filter.knownFilters`, which is not
too significant by itself (given that subscriptions are removed or disabled only
rarely).

In the final version, I know that some operations like removing/disabling
subscriptions and filters will become more expensive, though not necessarily
adding/reenabling subscriptions and filters (and certainly not on initial load).

The program spends most of its time doing filter matching, which is unaffected,
as is the initial loading of the subscriptions. For the rest of the operations,
which occur only in response to user action or occasional (once per hour at
most) processing in the background, I believe this is a good tradeoff of memory
vs. performance.

[hub, 2019/01/03 20:22:48]

Acknowledged.

   }
 }
 
 function onSubscriptionDisabled(subscription, newValue)
 {
   FilterListener.setDirty(1);
 
   if (filterStorage.knownSubscriptions.has(subscription.url))
   {
     if (newValue == false)
     {
       addSubscriptionFilters(subscription);
     }
     else
     {
-      for (let filter of subscription.filters())
-        removeFilter(filter);
+      for (let text of subscription.filterText())
+        removeFilter(Filter.fromText(text));

[hub, 2018/11/30 01:18:07]

...so does disable subscription.

[Manish Jethani, 2018/11/30 07:28:05]

See above.

     }
   }
 }
 
-function onSubscriptionUpdated(subscription, oldFilters)
+function onSubscriptionUpdated(subscription, oldFilterText)
 {
   FilterListener.setDirty(1);
 
   if (!subscription.disabled &&
       filterStorage.knownSubscriptions.has(subscription.url))
   {
-    for (let filter of oldFilters)
-      removeFilter(filter);
+    for (let text of oldFilterText)
+      removeFilter(Filter.fromText(text));
 
     addSubscriptionFilters(subscription);
   }
 }
 
 function onFilterHitCount(filter, newValue)
 {
   if (newValue == 0)
     FilterListener.setDirty(0);
   else
@@ skipping 50 matching lines @@
   {
     if (!subscription.disabled)
       addSubscriptionFilters(subscription);
   }
 }
 
 function onSave()
 {
   isDirty = 0;
 }