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

Side by Side 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.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View unified diff | Download patch
« no previous file with comments | « lib/elemHide.js ('k') | test/filterClasses.js » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 /* 1 /*
2 * This file is part of Adblock Plus <https://adblockplus.org/>, 2 * This file is part of Adblock Plus <https://adblockplus.org/>,
3 * Copyright (C) 2006-present eyeo GmbH 3 * Copyright (C) 2006-present eyeo GmbH
4 * 4 *
5 * Adblock Plus is free software: you can redistribute it and/or modify 5 * Adblock Plus is free software: you can redistribute it and/or modify
6 * it under the terms of the GNU General Public License version 3 as 6 * it under the terms of the GNU General Public License version 3 as
7 * published by the Free Software Foundation. 7 * published by the Free Software Foundation.
8 * 8 *
9 * Adblock Plus is distributed in the hope that it will be useful, 9 * Adblock Plus is distributed in the hope that it will be useful,
10 * but WITHOUT ANY WARRANTY; without even the implied warranty of 10 * but WITHOUT ANY WARRANTY; without even the implied warranty of
(...skipping 429 matching lines...) Expand 10 before | Expand all | Expand 10 after
440 domainSource: null, 440 domainSource: null,
441 441
442 /** 442 /**
443 * Separator character used in domainSource property, must be 443 * Separator character used in domainSource property, must be
444 * overridden by subclasses 444 * overridden by subclasses
445 * @type {string} 445 * @type {string}
446 */ 446 */
447 domainSeparator: null, 447 domainSeparator: null,
448 448
449 /** 449 /**
450 * Number of times {@link ActiveFilter#domains} has been accessed.
451 * @type {number}
452 * @private
453 */
454 _domainsAccessCount: 0,
455
456 /**
457 * Maximum number of times {@link ActiveFilter#domains} can be accessed
458 * before the value is cached.
459 * @type {number}
460 * @private
461 */
462 _domainsAccessCacheThreshold: 3,
463
464 /**
450 * Map containing domains that this filter should match on/not match 465 * Map containing domains that this filter should match on/not match
451 * on or null if the filter should match on all domains 466 * on or null if the filter should match on all domains
452 * @type {?Map.<string,boolean>} 467 * @type {?Map.<string,boolean>}
453 */ 468 */
454 get domains() 469 get domains()
455 { 470 {
456 let domains = null; 471 let domains = null;
457 472
458 if (this.domainSource) 473 if (this.domainSource)
459 { 474 {
475 // For some filter types this property is accessed only rarely,
476 // especially when the subscriptions are initially loaded. We defer any
477 // caching for such filters.
478 let cacheValue = ++this._domainsAccessCount >
479 this._domainsAccessCacheThreshold;
480
460 let source = this.domainSource.toLowerCase(); 481 let source = this.domainSource.toLowerCase();
461 482
462 let knownMap = knownDomainMaps.get(source); 483 let knownMap = knownDomainMaps.get(source);
463 if (knownMap) 484 if (knownMap)
464 { 485 {
465 domains = knownMap; 486 domains = knownMap;
466 } 487 }
467 else 488 else
468 { 489 {
469 let list = source.split(this.domainSeparator); 490 let list = source.split(this.domainSeparator);
(...skipping 26 matching lines...) Expand all
496 if (!domains) 517 if (!domains)
497 domains = new Map(); 518 domains = new Map();
498 519
499 domains.set(domain, include); 520 domains.set(domain, include);
500 } 521 }
501 522
502 if (domains) 523 if (domains)
503 domains.set("", !hasIncludes); 524 domains.set("", !hasIncludes);
504 } 525 }
505 526
506 if (domains) 527 if (!domains || cacheValue)
507 knownDomainMaps.set(source, domains); 528 knownDomainMaps.set(source, domains);
508 } 529 }
509 530
510 this.domainSource = null; 531 if (!domains || cacheValue)
532 {
533 this.domainSource = null;
534 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
535 }
511 } 536 }
512 537
513 Object.defineProperty(this, "domains", {value: domains}); 538 return domains;
514 return this.domains;
515 }, 539 },
516 540
517 /** 541 /**
518 * Array containing public keys of websites that this filter should apply to 542 * Array containing public keys of websites that this filter should apply to
519 * @type {?string[]} 543 * @type {?string[]}
520 */ 544 */
521 sitekeys: null, 545 sitekeys: null,
522 546
523 /** 547 /**
524 * Checks whether this filter is active on a domain. 548 * Checks whether this filter is active on a domain.
(...skipping 712 matching lines...) Expand 10 before | Expand all | Expand 10 after
1237 1261
1238 /** 1262 /**
1239 * Script that should be executed 1263 * Script that should be executed
1240 * @type {string} 1264 * @type {string}
1241 */ 1265 */
1242 get script() 1266 get script()
1243 { 1267 {
1244 return this.body; 1268 return this.body;
1245 } 1269 }
1246 }); 1270 });
OLDNEW
« 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