Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: lib/elemHide.js

Issue 29743730: Issue 6559 - Use maps and sets where appropriate (Closed) Base URL: https://hg.adblockplus.org/adblockpluscore/
Patch Set: Created April 5, 2018, 5:38 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « lib/downloader.js ('k') | lib/elemHideEmulation.js » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: lib/elemHide.js
===================================================================
--- a/lib/elemHide.js
+++ b/lib/elemHide.js
@@ -41,18 +41,19 @@
* (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
* unconditionally matched for all domains.)
+ * @type {Map.<string,number>}
*/
-let filterKeyBySelector = Object.create(null);
+let filterKeyBySelector = new Map();
/**
* This array caches the keys of filterKeyBySelector table (selectors which
* unconditionally apply on all domains). It will be null if the cache needs to
* be rebuilt.
*/
let unconditionalSelectors = null;
@@ -65,43 +66,43 @@
/**
* Object to be used instead when a filter has a blank domains property.
*/
let defaultDomains = new Map([["", true]]);
/**
* Lookup table, keys are known element hiding exceptions
kzar 2018/04/06 11:42:20 Nit: This comment reads a bit weirdly now that it'
Manish Jethani 2018/04/06 12:44:09 Done.
- * @type {Object}
+ * @type {Set.<string>}
*/
-let knownExceptions = Object.create(null);
+let knownExceptions = new Set();
/**
* Lookup table, lists of element hiding exceptions by selector
- * @type {Object}
+ * @type {Map.<string,Filter>}
*/
-let exceptions = Object.create(null);
+let exceptions = new Map();
/**
* Container for element hiding filters
* @class
*/
let ElemHide = exports.ElemHide = {
/**
* Removes all known filters
*/
clear()
{
+ for (let collection of [keyByFilter, filtersByDomain, filterKeyBySelector,
Manish Jethani 2018/04/06 05:36:16 Note that there's no need to create new objects fo
+ knownExceptions, exceptions])
+ {
+ collection.clear();
+ }
filterByKey = [];
- 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)
{
let domains = filter.domains || defaultDomains;
for (let [domain, isIncluded] of domains)
{
@@ -115,66 +116,68 @@
/**
* Add a new element hiding filter
* @param {ElemHideFilter} filter
*/
add(filter)
{
if (filter instanceof ElemHideException)
{
- if (filter.text in knownExceptions)
+ if (knownExceptions.has(filter.text))
return;
let {selector} = filter;
- if (!(selector in exceptions))
- exceptions[selector] = [];
- exceptions[selector].push(filter);
+ let list = exceptions.get(selector);
+ if (list)
+ list.push(filter);
+ else
+ exceptions.set(selector, [filter]);
// If this is the first exception for a previously unconditionally
// applied element hiding selector we need to take care to update the
// lookups.
- let filterKey = filterKeyBySelector[selector];
+ let filterKey = filterKeyBySelector.get(selector);
if (typeof filterKey != "undefined")
{
this._addToFiltersByDomain(filterKey, filterByKey[filterKey]);
- delete filterKeyBySelector[selector];
+ filterKeyBySelector.delete(selector);
unconditionalSelectors = unconditionalFilterKeys = null;
}
- knownExceptions[filter.text] = true;
+ knownExceptions.add(filter.text);
}
else
{
if (keyByFilter.has(filter))
return;
let key = filterByKey.push(filter) - 1;
keyByFilter.set(filter, key);
- if (!(filter.domains || filter.selector in exceptions))
+ if (!(filter.domains || exceptions.has(filter.selector)))
{
// The new filter's selector is unconditionally applied to all domains
- filterKeyBySelector[filter.selector] = key;
+ filterKeyBySelector.set(filter.selector, key);
unconditionalSelectors = unconditionalFilterKeys = null;
}
else
{
// The new filter's selector only applies to some domains
this._addToFiltersByDomain(key, filter);
}
}
FilterNotifier.emit("elemhideupdate");
},
_removeFilterKey(key, filter)
{
- if (filterKeyBySelector[filter.selector] == key)
+ if (filterKeyBySelector.get(filter.selector) == key)
{
- delete filterKeyBySelector[filter.selector];
+ filterKeyBySelector.delete(filter.selector);
unconditionalSelectors = unconditionalFilterKeys = null;
return;
}
// We haven't found this filter in unconditional filters, look in
// filtersByDomain.
let domains = filter.domains || defaultDomains;
for (let domain of domains.keys())
@@ -188,24 +191,24 @@
/**
* Removes an element hiding filter
* @param {ElemHideFilter} filter
*/
remove(filter)
{
if (filter instanceof ElemHideException)
{
- if (!(filter.text in knownExceptions))
+ if (!knownExceptions.has(filter.text))
return;
- let list = exceptions[filter.selector];
+ let list = exceptions.get(filter.selector);
let index = list.indexOf(filter);
if (index >= 0)
list.splice(index, 1);
- delete knownExceptions[filter.text];
+ knownExceptions.delete(filter.text);
}
else
{
let key = keyByFilter.get(filter);
if (typeof key == "undefined")
return;
delete filterByKey[key];
@@ -220,20 +223,20 @@
* Checks whether an exception rule is registered for a filter on a particular
* domain.
* @param {Filter} filter
* @param {string} docDomain
* @return {ElemHideException}
*/
getException(filter, docDomain)
{
- if (!(filter.selector in exceptions))
+ let list = exceptions.get(filter.selector);
Manish Jethani 2018/04/06 05:36:16 If list is falsy we know it's undefined, since we
kzar 2018/04/06 11:42:20 Acknowledged.
+ if (!list)
return null;
- let list = exceptions[filter.selector];
for (let i = list.length - 1; i >= 0; i--)
{
if (list[i].isActiveOnDomain(docDomain))
return list[i];
}
return null;
},
@@ -276,34 +279,29 @@
/**
* Returns a list of selectors that apply on each website unconditionally.
* @returns {string[]}
*/
getUnconditionalSelectors()
{
if (!unconditionalSelectors)
- unconditionalSelectors = Object.keys(filterKeyBySelector);
+ unconditionalSelectors = [...filterKeyBySelector.keys()];
return unconditionalSelectors.slice();
},
/**
* Returns a list of filter keys for selectors which apply to all websites
* without exception.
* @returns {number[]}
*/
getUnconditionalFilterKeys()
{
if (!unconditionalFilterKeys)
Manish Jethani 2018/04/06 05:36:16 Essentially we're getting the values of filterKeyB
kzar 2018/04/06 11:42:20 Previously we were caching though. But come to th
Manish Jethani 2018/04/06 12:44:09 So it looks like we could do away with the concept
kzar 2018/04/06 13:07:54 Cool, so looks like we could remove that parameter
Manish Jethani 2018/04/06 13:34:37 Sure, go ahead.
- {
- let selectors = this.getUnconditionalSelectors();
- unconditionalFilterKeys = [];
- for (let selector of selectors)
- unconditionalFilterKeys.push(filterKeyBySelector[selector]);
- }
+ unconditionalFilterKeys = [...filterKeyBySelector.values()];
return unconditionalFilterKeys.slice();
},
/**
* Constant used by getSelectorsForDomain to return all selectors applying to
* a particular hostname.
*/
@@ -346,31 +344,31 @@
if (criteria < ElemHide.NO_UNCONDITIONAL)
{
selectors = this.getUnconditionalSelectors();
if (provideFilterKeys)
filterKeys = this.getUnconditionalFilterKeys();
}
let specificOnly = (criteria >= ElemHide.SPECIFIC_ONLY);
- let seenFilters = Object.create(null);
+ let seenFilters = new Set();
let currentDomain = domain ? domain.toUpperCase() : "";
while (true)
{
if (specificOnly && currentDomain == "")
break;
let filters = filtersByDomain.get(currentDomain);
if (filters)
{
for (let [filterKey, filter] of filters)
{
- if (filterKey in seenFilters)
+ if (seenFilters.has(filterKey))
kzar 2018/04/06 11:42:20 This is a hotspot, we spent quite some time gettin
Manish Jethani 2018/04/06 12:44:09 I ran this particular test and it's 2s vs 1.6s on
kzar 2018/04/06 13:07:54 Acknowledged.
Manish Jethani 2018/04/06 13:44:11 BTW I tried replacing `filterKey in seenFilters` w
continue;
- seenFilters[filterKey] = true;
+ seenFilters.add(filterKey);
if (filter && !this.getException(filter, domain))
{
selectors.push(filter.selector);
// It is faster to always push the key, even if not required.
filterKeys.push(filterKey);
}
}
« no previous file with comments | « lib/downloader.js ('k') | lib/elemHideEmulation.js » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld