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

Unified Diff: lib/filterClasses.js

Issue 29841558: Issue 6815 - Avoid multiple copies of domain maps (Closed) Base URL: https://hg.adblockplus.org/adblockpluscore/
Patch Set: Add tests Created July 30, 2018, 5:11 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 | « no previous file | test/filterClasses.js » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: lib/filterClasses.js
===================================================================
--- a/lib/filterClasses.js
+++ b/lib/filterClasses.js
@@ -21,16 +21,22 @@
* @fileOverview Definition of Filter class and its subclasses.
*/
const {FilterNotifier} = require("./filterNotifier");
const {extend} = require("./coreUtils");
const {filterToRegExp} = require("./common");
/**
+ * All known unique domain sources mapped to their parsed values.
+ * @type {Map.<string,Map.<string,boolean>>}
+ */
+let knownDomainMaps = new Map();
+
+/**
* Abstract base class for filters
*
* @param {string} text string representation of the filter
* @constructor
*/
function Filter(text)
{
this.text = text;
@@ -393,74 +399,79 @@
/**
* Map containing domains that this filter should match on/not match
* on or null if the filter should match on all domains
* @type {?Map.<string,boolean>}
*/
get domains()
{
- let prop = Object.getOwnPropertyDescriptor(this, "_domains");
- if (prop)
- {
- let {value} = prop;
- return typeof value == "string" ?
- new Map([[value, true], ["", false]]) : value;
- }
-
let domains = null;
if (this.domainSource)
{
let source = this.domainSource;
if (!this.domainSourceIsLowerCase)
{
// RegExpFilter already have lowercase domains
source = source.toLowerCase();
}
- let list = source.split(this.domainSeparator);
- if (list.length == 1 && list[0][0] != "~")
+
+ let knownMap = knownDomainMaps.get(source);
+ if (knownMap)
{
- // Fast track for the common one-domain scenario
- domains = list[0];
+ domains = knownMap;
}
else
{
- let hasIncludes = false;
- for (let i = 0; i < list.length; i++)
+ let list = source.split(this.domainSeparator);
+ if (list.length == 1 && list[0][0] != "~")
{
- let domain = list[i];
- if (domain == "")
- continue;
+ // Fast track for the common one-domain scenario
+ domains = new Map([[list[0], true], ["", false]]);
+ }
+ else
+ {
+ let hasIncludes = false;
+ for (let i = 0; i < list.length; i++)
+ {
+ let domain = list[i];
+ if (domain == "")
+ continue;
- let include;
- if (domain[0] == "~")
- {
- include = false;
- domain = domain.substr(1);
- }
- else
- {
- include = true;
- hasIncludes = true;
+ let include;
+ if (domain[0] == "~")
+ {
+ include = false;
+ domain = domain.substr(1);
+ }
+ else
+ {
+ include = true;
+ hasIncludes = true;
+ }
+
+ if (!domains)
+ domains = new Map();
+
+ domains.set(domain, include);
}
- if (!domains)
- domains = new Map();
+ if (domains)
+ domains.set("", !hasIncludes);
+ }
- domains.set(domain, include);
- }
if (domains)
- domains.set("", !hasIncludes);
+ knownDomainMaps.set(source, domains);
}
this.domainSource = null;
}
- Object.defineProperty(this, "_domains", {value: domains});
+ Object.defineProperty(this, "domains", {value: domains});
return this.domains;
},
/**
* Array containing public keys of websites that this filter should apply to
* @type {?string[]}
*/
sitekeys: null,
« no previous file with comments | « no previous file | test/filterClasses.js » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld