Issue 4071 - Speed up ElemHide.getSelectorsForDomain for domains with no filters

lib/elemHide.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2016 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 37 matching lines @@
 var usingGetSelectorsForDomain = !("nsIStyleSheetService" in Ci);
 
 /**
  * Nested lookup table, filter (or false if inactive) by filter key by domain.
  * (Only contains filters that aren't unconditionally matched for all domains.)
  * @type Object
  */
 var filtersByDomain = Object.create(null);
 
 /**
+ * Lookup table, count of filter exceptions by domain.
+ * @type Object
+ */
+var exceptionCountByDomain = Object.create(null);
+
+/**
  * Lookup table, filters by selector. (Only contains filters that have a
  * selector that is unconditionally matched for all domains.)
  */
 var filtersBySelector = Object.create(null);
 
 /**
  * This array caches the keys of filtersBySelector table (selectors which
  * unconditionally apply on all domains). It will be null if the cache needs to
  * be rebuilt.
  */
@@ skipping 60 matching lines @@
 
   /**
    * Removes all known filters
    */
   clear: function()
   {
     filterByKey = [];
     keyByFilter = Object.create(null);
     filtersByDomain = Object.create(null);
     filtersBySelector = Object.create(null);
+    exceptionCountByDomain = Object.create(null);
     unconditionalSelectors = null;
     knownExceptions = Object.create(null);
     exceptions = Object.create(null);
     ElemHide.isDirty = false;
     ElemHide.unapply();
   },
 
   _addToFiltersByDomain: function(filter)
   {
     let key = keyByFilter[filter.text];
     let domains = filter.domains || defaultDomains;
     for (let domain in domains)
     {
       let filters = filtersByDomain[domain];
       if (!filters)
         filters = filtersByDomain[domain] = Object.create(null);
 
       if (domains[domain])
+      {
         filters[key] = filter;
+      }
       else
+      {
         filters[key] = false;
+        if (domain in exceptionCountByDomain)
+          exceptionCountByDomain[domain] += 1;
+        else
+          exceptionCountByDomain[domain] = 1;
+      }
     }
   },
 
   /**
    * Add a new element hiding filter
    * @param {ElemHideFilter} filter
    */
   add: function(filter)
   {
     if (filter instanceof ElemHideException)
     {
       if (filter.text in knownExceptions)
         return;
 
       let selector = filter.selector;
       if (!(selector in exceptions))
         exceptions[selector] = [];
       exceptions[selector].push(filter);
 
       if (usingGetSelectorsForDomain)
       {
+        let domains = filter.domains || defaultDomains;
+        for (let domain in domains)
+        {
+          if (domain in exceptionCountByDomain)
+            exceptionCountByDomain[domain] += 1;
+          else
+            exceptionCountByDomain[domain] = 1;
+        }
+
         // If this is the first exception for a previously unconditionally
         // applied element hiding selector we need to take care to update the
         // lookups.
         let unconditionalFilters = filtersBySelector[selector];
         if (unconditionalFilters)
         {
           for (let f of unconditionalFilters)
             this._addToFiltersByDomain(f);
           delete filtersBySelector[selector];
           unconditionalSelectors = null;
@@ skipping 59 matching lines @@
       if (!(filter.text in keyByFilter))
         return;
 
       let key = keyByFilter[filter.text];
       delete filterByKey[key];
       delete keyByFilter[filter.text];
       ElemHide.isDirty = true;
 
       if (usingGetSelectorsForDomain)
       {
+        let domains = filter.domains || defaultDomains;
+        for (let domain in domains)
+        {
+          let count = exceptionCountByDomain[domain];
+          if (count > 1)
+            exceptionCountByDomain[domain] -= 1;
+          else if (count)

[Sebastian Noack, 2016/05/26 11:46:43]

It seems the condition to check for non-zero count is redundant. Then you also
don't need the count variable.

However, I wonder whether we should delete properties from
exceptionCountByDomain anyway. It seems we don't do that for filtersByDomain
either.

+            delete exceptionCountByDomain[domain];
+        }
+
         let filters = filtersBySelector[filter.selector];
         if (filters)
         {
           if (filters.length > 1)
           {
             let index = filters.indexOf(filter);
             filters.splice(index, 1);
           }
           else
           {
@@ skipping 225 matching lines @@
    */
   getSelectorsForDomain: function(/**String*/ domain, /**Boolean*/ specificOnly)
   {
     if (!usingGetSelectorsForDomain)
       throw new Error("getSelectorsForDomain can not be used in Firefox!");
 
     if (!unconditionalSelectors)
       unconditionalSelectors = Object.keys(filtersBySelector);
     let selectors = specificOnly ? [] : unconditionalSelectors.slice();
 
+    let anyExceptions = false;
+
     let seenFilters = Object.create(null);
     let currentDomain = domain ? domain.toUpperCase() : "";
     while (true)
     {
-      if (specificOnly && currentDomain == "")
-        break;
+      if (currentDomain == "")
+      {
+        if (specificOnly)
+          break;
+
+        // For domains with no exceptions we can add all generic rules
+        if (!anyExceptions)
+        {
+          let filters = filtersByDomain[""];
+          for (let filterKey in filters)
+          {
+            let filter = filters[filterKey];
+            if (filter)

[Sebastian Noack, 2016/05/26 11:46:43]

Just for my understanding, what is the scenario when "filter" evaluates to false
here?

+              selectors.push(filters[filterKey].selector);

[Sebastian Noack, 2016/05/26 11:46:43]

Use the "filter" variable here?

+          }
+          break;
+        }
+      }
+      anyExceptions = anyExceptions || exceptionCountByDomain[currentDomain];

[Sebastian Noack, 2016/05/26 11:46:43]

I think it's preferable to write it like this:

 if (exceptionCountByDomain[currentDomain])
    anyExceptions = true;

If for nothing else, to not change the type of the "anyExceptions" variable.

 
       let filters = filtersByDomain[currentDomain];
       if (filters)
       {
         for (let filterKey in filters)
         {
           if (filterKey in seenFilters)
             continue;
           seenFilters[filterKey] = true;
 
           let filter = filters[filterKey];
           if (filter && !this.getException(filter, domain))
             selectors.push(filter.selector);
         }
       }
 
       if (currentDomain == "")
         break;
 
       let nextDot = currentDomain.indexOf(".");
       currentDomain = nextDot == -1 ? "" : currentDomain.substr(nextDot + 1);
     }
 
     return selectors;
   }
 };