Skip to content

Commit

Permalink
The RegExp object - small performance improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
@janekptacijarabaci
janekptacijarabaci committed May 2, 2017
1 parent 90b9579 commit 9608b0d
Show file tree
Hide file tree
Showing 7 changed files with 22 additions and 15 deletions.
4 changes: 2 additions & 2 deletions content/addons-overlay.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ gDragDrop.onDrop = function GM_onDrop(aEvent) {
let urls = droppedUrls(aEvent);

let droppedNonUserScript = false;
let _re = new RegExp(GM_CONSTANTS.fileScriptExtensionRegexp + "$", "");
for (let i = urls.length - 1, url = null; url = urls[i]; i--) {
if (url.match(
new RegExp(GM_CONSTANTS.fileScriptExtensionRegexp + "$", ""))) {
if (url.match(_re)) {
GM_util.showInstallDialog(url);
} else {
droppedNonUserScript = true;
Expand Down
11 changes: 8 additions & 3 deletions content/framescript.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ Cu.import("chrome://greasemonkey-modules/content/processScript.js", {})

// \\ // \\ // \\ // \\ // \\ // \\ // \\ // \\ // \\ // \\ // \\ // \\ // \\ //

const URL_ABOUT_PART2_REGEXP = new RegExp(
GM_CONSTANTS.urlAboutPart2Regexp, "");
const URL_USER_PASS_STRIP_REGEXP = new RegExp(
GM_CONSTANTS.urlUserPassStripRegexp, "");

var gScope = this;

// \\ // \\ // \\ // \\ // \\ // \\ // \\ // \\ // \\ // \\ // \\ // \\ // \\ //
Expand Down Expand Up @@ -82,7 +87,7 @@ function browserLoadEnd(aEvent) {
if (GM_util.getEnabled()) {
// See #1820, #2371, #2195.
if ((href == GM_CONSTANTS.urlAboutPart1)
|| (href.match(new RegExp(GM_CONSTANTS.urlAboutPart2Regexp, "")))) {
|| (href.match(URL_ABOUT_PART2_REGEXP))) {
runScripts("document-end", contentWin);
runScripts("document-idle", contentWin);
}
Expand Down Expand Up @@ -204,7 +209,7 @@ function urlForWin(aContentWin) {
// so always use it when deciding whether to run scripts.
let url = aContentWin.document.documentURI;
// But (see #1631) ignore user/pass in the URL.
return url.replace(new RegExp(GM_CONSTANTS.urlStripUserPassRegexp, ""), "$1");
return url.replace(URL_USER_PASS_STRIP_REGEXP, "$1");
}

function windowIsTop(aContentWin) {
Expand Down Expand Up @@ -232,7 +237,7 @@ function windowCreated(aEvent) {
let href = contentWin.location.href;
// See #1820, #2371, #2195.
if ((href == GM_CONSTANTS.urlAboutPart1)
|| (href.match(new RegExp(GM_CONSTANTS.urlAboutPart2Regexp, "")))) {
|| (href.match(URL_ABOUT_PART2_REGEXP))) {
runScripts("document-start", contentWin);
}
}
Expand Down
8 changes: 4 additions & 4 deletions content/newscript.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,18 +147,18 @@ function createScriptSource() {
return true;
}

var re1 = new RegExp("[^\\s]+", "g");
function replaceMultiVal(aMetaName) {
let replaceKey = "%" + aMetaName + "%";
if (source.indexOf(replaceKey) == -1) {
return undefined;
};
let replaceVal = document.getElementById(aMetaName).value.match(
new RegExp("[^\\s]+", "g"));
let replaceVal = document.getElementById(aMetaName).value.match(re1);
if (!replaceVal || (replaceVal.length == 0)) {
removeMetaLine(aMetaName);
} else {
let re = new RegExp("(.+)" + replaceKey);
let match = source.match(re);
let re2 = new RegExp("(.+)" + replaceKey);
let match = source.match(re2);
source = source.replace(replaceKey, replaceVal.join("\n" + match[1]));
}
}
Expand Down
2 changes: 1 addition & 1 deletion modules/constants.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ const GM_CONSTANTS = {
"urlAboutPart1": "about:blank",
"urlAboutPart1Regexp": "^about:blank",
"urlAboutPart2Regexp": "^about:reader",
"urlStripUserPassRegexp": "(://)([^:/]+)(:[^@/]+)?@",
"urlUserPassStripRegexp": "(://)([^:/]+)(:[^@/]+)?@",
"versionChecker": Cc["@mozilla.org/xpcom/version-comparator;1"]
.getService(Ci.nsIVersionComparator),
"xulAppInfo": Cc["@mozilla.org/xre/app-info;1"]
Expand Down
4 changes: 2 additions & 2 deletions modules/parseScript.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,11 @@ function parse(aSource, aUri, aFailWhenMissing) {
}

var resourceNames = {};
let _re = new RegExp("\\s+$", "");
for (let i = 0, metaLine = ""; metaLine = meta[i]; i++) {
var data;
try {
data = GM_util.parseMetaLine(
metaLine.replace(new RegExp("\\s+$", ""), ""));
data = GM_util.parseMetaLine(metaLine.replace(_re, ""));
} catch (e) {
// Ignore invalid/unsupported meta lines.
continue;
Expand Down
5 changes: 3 additions & 2 deletions modules/remoteScript.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ const CALLBACK_IS_NOT_FUNCTION = "callback is not a function.";

const TIMEOUT = 500;

const FILENAME_DISALLOWED_CHARACTERS = new RegExp("[\\\\/:*?'\"<>|]", "g");

// https://msdn.microsoft.com/en-us/library/aa365247.aspx#maxpath
// Actual limit is 260; 240 ensures e.g. ".user.js" and slashes still fit.
// The "/ 2" thing is so that we can have a directory, and a file in it.
Expand All @@ -45,11 +47,10 @@ function assertIsFunction(aFunc, aMessage) {
}
}

var disallowedFilenameCharacters = new RegExp("[\\\\/:*?'\"<>|]", "g");
function cleanFilename(aFilename, aDefault) {
// Blacklist problem characters (slashes, colons, etc.).
let filename = (aFilename || aDefault)
.replace(disallowedFilenameCharacters, "");
.replace(FILENAME_DISALLOWED_CHARACTERS, "");

// Make whitespace readable.
filename = filename.replace(new RegExp("(\\s|%20)+", "g"), "_");
Expand Down
3 changes: 2 additions & 1 deletion modules/sync.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,12 +190,13 @@ ScriptStore.prototype = {
"getAllIDs": function () {
let syncIds = {};
let scripts = GM_util.getService().config.scripts;
let _re = new RegExp("^file:", "");
for (let i = 0, iLen = scripts.length; i < iLen; i++) {
let script = scripts[i];
if (!script.downloadURL) {
continue;
}
if (script.downloadURL.match(new RegExp("^file:", ""))) {
if (script.downloadURL.match(_re)) {
continue;
}
syncIds[syncId(script)] = 1;
Expand Down

2 comments on commit 9608b0d

@adisib
Copy link

@adisib adisib commented on 9608b0d May 3, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want regex performance improvements, then don't use String.prototype.match() for the if statements here. Instead use RegExp.prototype.test() or even String.prototype.search(). E.g., instead of doing " if (url.match(_re)) { " do " if (_re.test(url)) { " or " if (url.search(_re) !== -1) { ". Do note that RegExp.prototype.test() is not a drop in replacement, as it has the behavior that using it will not reset the lastIndex property, so subsequent calls may not give the result that was expected.

I suggest replacing String.prototype.match() inside of if statements with String.prototype.search() as it will work fine as a drop in replacement for if statements, and will give a much better performance improvement than what you have already done here.

@janekptacijarabaci
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. But I will not change it now. Maybe in the future.
But I know sites like https://jsperf.com/ etc.

Changes were only symbolic - high performance is not needed.

But in some cases it makes sense, e.g.
greasemonkey#2318

Please sign in to comment.