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

Unified Diff: lib/filterClasses.js

Issue 29909555: Issue 7046 - Defer caching of domain maps (Closed) Base URL: https://hg.adblockplus.org/adblockpluscore/
Patch Set: Do not cache value for generic filters Created Oct. 14, 2018, 12:26 a.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/elemHide.js ('k') | 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
@@ -442,26 +442,47 @@
/**
* Separator character used in domainSource property, must be
* overridden by subclasses
* @type {string}
*/
domainSeparator: null,
/**
+ * Number of times {@link ActiveFilter#domains} has been accessed.
+ * @type {number}
+ * @private
+ */
+ _domainsAccessCount: 0,
+
+ /**
+ * Maximum number of times {@link ActiveFilter#domains} can be accessed
+ * before the value is cached.
+ * @type {number}
+ * @private
+ */
+ _domainsAccessCacheThreshold: 3,
+
+ /**
* 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 domains = null;
if (this.domainSource)
{
+ // For some filter types this property is accessed only rarely,
+ // especially when the subscriptions are initially loaded. We defer any
+ // caching for such filters.
+ let cacheValue = ++this._domainsAccessCount >
+ this._domainsAccessCacheThreshold;
+
let source = this.domainSource.toLowerCase();
let knownMap = knownDomainMaps.get(source);
if (knownMap)
{
domains = knownMap;
}
else
@@ -498,25 +519,28 @@
domains.set(domain, include);
}
if (domains)
domains.set("", !hasIncludes);
}
- if (domains)
+ if (!domains || cacheValue)
knownDomainMaps.set(source, domains);
}
- this.domainSource = null;
+ if (!domains || cacheValue)
+ {
+ this.domainSource = null;
+ Object.defineProperty(this, "domains", {value: domains});
Manish Jethani 2018/10/15 15:37:00 It just occurred to me that we don't have to set t
+ }
}
- Object.defineProperty(this, "domains", {value: domains});
- return this.domains;
+ return domains;
},
/**
* Array containing public keys of websites that this filter should apply to
* @type {?string[]}
*/
sitekeys: null,
« no previous file with comments | « lib/elemHide.js ('k') | test/filterClasses.js » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld