Issue 235 - Speed up ElemHide.getSelectorsForDomain

lib/elemHide.js

Index: lib/elemHide.js
diff --git a/lib/elemHide.js b/lib/elemHide.js
index 4b25c62d0daf0079b7e8f54f962cf3d4bddf9792..dec027b4dfac587d2e82cca7030011d1b5d26f78 100644
--- a/lib/elemHide.js
+++ b/lib/elemHide.js
@@ -40,6 +40,19 @@ var filterByKey = Object.create(null);
 var keyByFilter = Object.create(null);
 
 /**
+ * Nested lookup table, filter (or false if inactive) by filter text by domain
+ * @type Object
+ */
+var filtersByDomain = Object.create(null);
+
+/**
+ * Indicates whether we are using (and maintaining) the filtersByDomain lookup.
+ * (Will be false for Firefox)
+ * @type Boolean
+ */
+var usingFiltersByDomain = !("nsIStyleSheetService" in Ci);
+
+/**
  * Lookup table, keys are known element hiding exceptions
  * @type Object
  */
@@ -70,7 +83,7 @@ var ElemHide = exports.ElemHide =
   isDirty: false,
 
   /**
-   * Inidicates whether the element hiding stylesheet is currently applied.
+   * Indicates whether the element hiding stylesheet is currently applied.
    * @type Boolean
    */
   applied: false,
@@ -99,6 +112,7 @@ var ElemHide = exports.ElemHide =
   {
     filterByKey = Object.create(null);
     keyByFilter = Object.create(null);
+    filtersByDomain = Object.create(null);
     knownExceptions = Object.create(null);
     exceptions = Object.create(null);
     ElemHide.isDirty = false;
@@ -134,6 +148,20 @@ var ElemHide = exports.ElemHide =
 
       filterByKey[key] = filter;
       keyByFilter[filter.text] = key;
+
+      if (usingFiltersByDomain)
+      {
+        let domainMatches = filter.domains || {"": true};

[Wladimir Palant, 2016/05/18 13:38:06]

You are iterating over object keys below but {"": true} has more properties than
just "" (unlike filter.domains which doesn't have a prototype). It's probably
best to have a global defaultDomains variable that has this object without
prototype.

[Sebastian Noack, 2016/05/18 13:48:25]

Also note that for..in loops on objects in hash-table mode cause the function to
de-deoptimized in V8. However, whether this is the case here depends a lot of
the context. So please make sure that there is no deopt. And if necessary resort
to Object.keys() + for(;;)

[kzar, 2016/05/18 14:23:00]

Done.

[kzar, 2016/05/18 14:23:00]

The only way I know to check if a function is deoptimized is to find it in the
profiler. It was a little hard to find it but finally found it and the
Elemhide.add function is not deoptimized with this change apparently.
(Unfortunately I only found it in Botttom Up view so far, finding it in Top Down
was even harder.)

+        for (let domain in domainMatches)
+        {
+          domain = domain.toUpperCase();

[Wladimir Palant, 2016/05/18 13:38:06]

This operation is unnecessary, the domains are already upper-case.

[kzar, 2016/05/18 14:23:00]

Done.

+          let filters = filtersByDomain[domain];
+          if (!filters)
+            filters = filtersByDomain[domain] = Object.create(null);
+          filters[filter.text] = domainMatches[domain] && filter;

[Wladimir Palant, 2016/05/18 13:38:05]

I know, Sebastian loves such constructs. Still, I consider this too non-obvious.
Could you write this out, with a proper if statement?

  if (domainMatches[domain])
    filters[filter.text] = filter;
  else
    filters[filter.text] = false;

[kzar, 2016/05/18 14:23:00]

Done.

+        }
+      }
+
       ElemHide.isDirty = true;
     }
   },
@@ -368,22 +396,45 @@ var ElemHide = exports.ElemHide =
   },
 
   /**
-   * Returns a list of all selectors active on a particular domain (currently
-   * used only in Chrome, Opera and Safari).
+   * Returns a list of all selectors active on a particular domain, must not be
+   * used in Firefox (when usingFiltersByDomain is false).
    */
   getSelectorsForDomain: function(/**String*/ domain, /**Boolean*/ specificOnly)
   {
-    let result = [];
-    let keys = Object.getOwnPropertyNames(filterByKey);
-    for (let key of keys)
+    if (!usingFiltersByDomain)
+      throw new Error("getSelectorsForDomain can not be used in Firefox!");
+
+    let selectors = [];
+
+    let seenFilters = Object.create(null);
+    let currentDomain = domain ? domain.toUpperCase() : "";
+    while (true)
     {
-      let filter = filterByKey[key];
-      if (specificOnly && (!filter.domains || filter.domains[""]))
-        continue;
+      if (specificOnly && currentDomain == "")
+        break;
+
+      let filters = filtersByDomain[currentDomain];
+      if (filters)
+      {
+        for (let filterText in filters)
+        {
+          if (filterText in seenFilters)
+            continue;
+          seenFilters[filterText] = true;
+
+          let filter = filters[filterText];
+          if (filter && !this.getException(filter, domain))
+            selectors.push(filter.selector);
+        }
+      }
+
+      if (currentDomain == "")
+        break;
 
-      if (filter.isActiveOnDomain(domain) && !this.getException(filter, domain))
-        result.push(filter.selector);
+      let nextDot = currentDomain.indexOf(".");
+      currentDomain = nextDot == -1 ? "" : currentDomain.substr(nextDot + 1);
     }
-    return result;
+
+    return selectors;
   }
 };