Issue 7094 - Keep subscription filters by text only

lib/filterListener.js

Index: lib/filterListener.js
===================================================================
--- a/lib/filterListener.js
+++ b/lib/filterListener.js
@@ -27,17 +27,17 @@
 
 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}
@@ -234,65 +234,69 @@
   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)