Issue 5735 - Use JS Map instead of Object for global keyByFilter in elemHide.js

lib/elemHide.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 {FilterNotifier} = require("filterNotifier");
 
 /**
  * Lookup table, filters by their associated key
  * @type {Object}
  */
 let filterByKey = [];
 
 /**
  * Lookup table, keys of the filters by filter text
- * @type {Object}
+ * @type {Map.<string,string>}

[sergei, 2017/09/21 12:02:10]

Pay attention that after switching from Object to Map one can use Objects as
keys, so here it could be Map.<Filter,string>, I will measure it and if it's
better then either update this codereview or create a  new one.

[kzar, 2017/09/22 10:50:15]

Yea, it looks like we only used the Filter's text as the key since we had to. I
agree using the Filter itself as the key would be nicer, please could you change
that in this review?

[Wladimir Palant, 2017/09/25 10:59:31]

Agreed, we should use Filter objects as keys here.

[sergei, 2017/09/25 13:41:23]

Done.

I have checked when the type of key here is string and object and there is no
visible change in the memory usage (RSS).

[Wladimir Palant, 2017/09/25 13:57:16]

A change in memory usage would be unexpected, both approaches reuse existing
objects as keys instead of copying data around. Performance might be better with
objects however because comparing them is simpler than comparing strings.

  */
-let keyByFilter = Object.create(null);
+let keyByFilter = new Map();
 
 /**
  * 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 {Map.<string,Map.<string,(Filter|boolean)>>}
  */
 let filtersByDomain = new Map();
 
 /**
  * Lookup table, filter key by selector. (Only used for selectors that are
@@ skipping 36 matching lines @@
  * Container for element hiding filters
  * @class
  */
 let ElemHide = exports.ElemHide = {
   /**
    * Removes all known filters
    */
   clear()
   {
     filterByKey = [];
-    keyByFilter = Object.create(null);
+    keyByFilter = new Map();
     filtersByDomain = new Map();
     filterKeyBySelector = Object.create(null);
     unconditionalSelectors = unconditionalFilterKeys = null;
     knownExceptions = Object.create(null);
     exceptions = Object.create(null);
     FilterNotifier.emit("elemhideupdate");
   },
 
   _addToFiltersByDomain(key, filter)
   {
@@ skipping 31 matching lines @@
       {
         this._addToFiltersByDomain(filterKey, filterByKey[filterKey]);
         delete filterKeyBySelector[selector];
         unconditionalSelectors = unconditionalFilterKeys = null;
       }
 
       knownExceptions[filter.text] = true;
     }
     else
     {
-      if (filter.text in keyByFilter)
+      if (keyByFilter.has(filter.text))
         return;
 
       let key = filterByKey.push(filter) - 1;
-      keyByFilter[filter.text] = key;
+      keyByFilter.set(filter.text, key);
 
       if (!(filter.domains || filter.selector in exceptions))
       {
         // The new filter's selector is unconditionally applied to all domains
         filterKeyBySelector[filter.selector] = key;
         unconditionalSelectors = unconditionalFilterKeys = null;
       }
       else
       {
         // The new filter's selector only applies to some domains
@@ skipping 36 matching lines @@
         return;
 
       let list = exceptions[filter.selector];
       let index = list.indexOf(filter);
       if (index >= 0)
         list.splice(index, 1);
       delete knownExceptions[filter.text];
     }
     else
     {
-      if (!(filter.text in keyByFilter))
+      let key = keyByFilter.get(filter.text);
+      if (typeof key == "undefined")
         return;
 
-      let key = keyByFilter[filter.text];
       delete filterByKey[key];
-      delete keyByFilter[filter.text];
+      keyByFilter.delete(filter.text);
       this._removeFilterKey(key, filter);
     }
 
     FilterNotifier.emit("elemhideupdate");
   },
 
   /**
    * Checks whether an exception rule is registered for a filter on a particular
    * domain.
    * @param {Filter} filter
@@ skipping 158 matching lines @@
 
       let nextDot = currentDomain.indexOf(".");
       currentDomain = nextDot == -1 ? "" : currentDomain.substr(nextDot + 1);
     }
 
     if (provideFilterKeys)
       return [selectors, filterKeys];
     return selectors;
   }
 };