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: Update JSDoc Created Oct. 13, 2018, 11:41 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
Index: lib/filterClasses.js
===================================================================
--- a/lib/filterClasses.js
+++ b/lib/filterClasses.js
@@ -442,22 +442,43 @@
/**
* 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()
{
+ // 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 domains = null;
if (this.domainSource)
{
let source = this.domainSource.toLowerCase();
let knownMap = knownDomainMaps.get(source);
if (knownMap)
@@ -498,25 +519,28 @@
domains.set(domain, include);
}
if (domains)
domains.set("", !hasIncludes);
}
- if (domains)
+ if (!domains || cacheValue)
Manish Jethani 2018/10/15 15:36:59 There is a change in the semantics here in that we
knownDomainMaps.set(source, domains);
}
- this.domainSource = null;
+ if (!domains || cacheValue)
+ this.domainSource = null;
}
- Object.defineProperty(this, "domains", {value: domains});
- return this.domains;
+ if (!domains || cacheValue)
+ Object.defineProperty(this, "domains", {value: domains});
+
+ return domains;
},
/**
* Array containing public keys of websites that this filter should apply to
* @type {?string[]}
*/
sitekeys: null,

Powered by Google App Engine
This is Rietveld