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 199 matching lines @@
       ElemHide.remove(filter);
     else if (filter instanceof ElemHideEmulationFilter)
       ElemHideEmulation.remove(filter);
     else
       ElemHideExceptions.remove(filter);
   }
   else if (filter instanceof SnippetFilter)
     Snippets.remove(filter);
 }
 
-const primes = [101, 109, 131, 149, 163, 179, 193, 211, 229, 241];
-
-function addSubscriptionFilters(subscription)
-{
-  // We add filters using pseudo-random ordering. Reason is that ElemHide will
-  // assign consecutive filter IDs that might be visible to the website. The
-  // randomization makes sure that no conclusion can be made about the actual
-  // filters applying there. We have ten prime numbers to use as iteration step,
-  // any of those can be chosen as long as the array length isn't divisible by
-  // it.
-  let len = subscription.filterCount;
-  if (!len)
-    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)
-  {
-    let filter = Filter.fromText(subscription.filterTextAt(current));
-    filter.addSubscription(subscription);
-    addFilter(filter);
-  }
-}
-
 function onSubscriptionAdded(subscription)
 {
   FilterListener.setDirty(1);
 
   if (!subscription.disabled)
-    addSubscriptionFilters(subscription);
+  {
+    for (let text of subscription.filterText())
+      addFilter(Filter.fromText(text));
+  }
 }
 
 function onSubscriptionRemoved(subscription)
 {
   FilterListener.setDirty(1);
 
   if (!subscription.disabled)
   {
     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);
+      for (let text of subscription.filterText())
+        addFilter(Filter.fromText(text));
     }
     else
     {
       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, oldFilterText)
 {
   FilterListener.setDirty(1);
 
   if (!subscription.disabled &&
       filterStorage.knownSubscriptions.has(subscription.url))
   {
     for (let text of oldFilterText)
       removeFilter(Filter.fromText(text));
 
-    addSubscriptionFilters(subscription);
+    for (let text of subscription.filterText())
+      addFilter(Filter.fromText(text));
   }
 }
 
 function onFilterHitCount(filter, newValue)
 {
   if (newValue == 0)
     FilterListener.setDirty(0);
   else
     FilterListener.setDirty(0.002);
 }
@@ skipping 40 matching lines @@
 
   defaultMatcher.clear();
   ElemHide.clear();
   ElemHideEmulation.clear();
   ElemHideExceptions.clear();
   Snippets.clear();
 
   for (let subscription of filterStorage.subscriptions())
   {
     if (!subscription.disabled)
-      addSubscriptionFilters(subscription);
+    {
+      for (let text of subscription.filterText())
+        addFilter(Filter.fromText(text));
+    }
   }
 }
 
 function onSave()
 {
   isDirty = 0;
 }