Filter Options are slow..? Any reason?

[Geld]

Hello hello.
We were discussing about the performance of Filter Options, and found Adblock Plus checks any filters even if they have no effective Option. For example, it checks the filter "adbanner$script" for URLs of "image".

Current source of chrome/adblockplus.jar/content/prefs.js is:

matchesAnyInternal: function(location, contentType) {
if (this.shortcuts > 0) {
// Optimized matching using shortcuts
var text = location.toLowerCase();
var endPos = text.length - shortcutLength + 1;
for (var i = 0; i <= endPos; i++) {
var substr = text.substr(i, shortcutLength);
var pattern = this.shortcutHash[substr];
if (typeof pattern != "undefined" && pattern.regexp.test(location) &&
(!("contentType" in pattern) || typeMap[contentType] & pattern.contentType))
return pattern;
}
}

var list = this.regexps;
for (i = 0; i < list.length; i++)
if (list.regexp.test(location) && (!("contentType" in list) || typeMap[contentType] & list.contentType))
return list;

return null;
},

We think this would skip filters with invalid Options and run faster:

matchesAnyInternal: function(location, contentType) {
if (this.shortcuts > 0) {
// Optimized matching using shortcuts
var text = location.toLowerCase();
var endPos = text.length - shortcutLength + 1;
for (var i = 0; i <= endPos; i++) {
var substr = text.substr(i, shortcutLength);
var pattern = this.shortcutHash[substr];
if (typeof pattern != "undefined" && (!("contentType" in pattern) || typeMap[contentType] & pattern.contentType) &&
pattern.regexp.test(location))
return pattern;
}
}

var list = this.regexps;
for (i = 0; i < list.length; i++)
if ((!("contentType" in list) || typeMap[contentType] & list.contentType) && list.regexp.test(location))
return list;

return null;
},

Is there any reason for currrent (and probably slow) Filter Options?

Thanks for your reading.

[Wladimir Palant]

Yes, there is a reason. If you test for two necessary conditions you will test the one first that is more likely to be false (you will be able to skip the second then). As it stands now, the address is far more likely to fail when tested against the regexp, than it is to fail because of options - simply because options aren't used too often. So testing the regexp first is better performance-wise.

Anyway, we are not really talking about relevant performance changes here. By design, the first check will be executed very rarely - only if the shortcut matches. And any list causing the second check to be executed often is by definition not optimized for Adblock Plus. These lists are even more unlikely to use filter options.

[Geld]

Hmm... well, then is it nonsense to put Filter Options on all filters I wrote, at least for better performance?
Even if so, it will be greatly useful to avoid an accidental bombing by Adblock Plus though.

[Wladimir Palant]

No, as long as you do the usual precautions (use simple filters and make sure they are long enough) it doesn't really matter whether there are options. I doubt that the performance difference is even measurable.

[Geld]

Aha, okay. Thanks very much.