Skip to content

Commit

Permalink
Fixes #1089
Browse files Browse the repository at this point in the history
* Define a general purpose memoize decorator, use it for GM_isGreasemonkeyable() and convert2RegExp().
  • Loading branch information
Anthony Lieuallen authored and arantius committed Apr 2, 2010
1 parent 2e698d2 commit 9beb729
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 0 deletions.
1 change: 1 addition & 0 deletions content/convert2RegExp.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

28 changes: 28 additions & 0 deletions content/utils.js
Expand Up @@ -326,6 +326,7 @@ function GM_isGreasemonkeyable(url) {

return false;
}
GM_isGreasemonkeyable = GM_memoize(GM_isGreasemonkeyable);

function GM_getEnabled() {
return GM_prefRoot.getValue("enabled", true);
Expand All @@ -334,3 +335,30 @@ function GM_getEnabled() {
function GM_setEnabled(enabled) {
GM_prefRoot.setValue("enabled", enabled);
}

// Decorate a function with a memoization wrapper, with a limited-size cache
// to reduce peak memory utilization. Simple usage:
//
// function foo(arg1, arg2) { /* complex operation */ }
// foo = GM_memoize(foo);
//
// The memoized function may have any number of arguments, but they must be
// primitives (able to be used as object keys).

This comment has been minimized.

Copy link
@Ventero

Ventero Apr 2, 2010

Contributor

This is not entirely correct. Object keys are always strings, so as long as all arguments can be converted to strings, they can be used (so even functions and objects, though using objects is pretty dangerous, since object literals always have the string representation "[object Object]", while objects generated with new Object(x) always have x.toString() as string representation).
This means that the current approach will use the same cached result for foo(2, 3), foo("2,3") and foo(new Object(2), 3).
While this might not be a problem for the current use-cases, I think it might be worth to add a note in the comments.

This comment has been minimized.

Copy link
@arantius

arantius Apr 3, 2010

Collaborator

I know it's not 100% correct, but the comment is clear enough IMO. You do point out a good flaw re (2,3) vs ("2,3"), and once that's fixed, changing the comment to more literally reflect what is really happening, is even better.

function GM_memoize(func, limit) {
limit = limit || 1000;
var cache = {};

This comment has been minimized.

Copy link
@satyr

satyr Apr 8, 2010

Should be:

var cache = {__proto__: null};

to avoid conflict with Object.prototype members (e.g. "toString", "constructor" etc.).

var keylist = [];

return function(a) {
var args = Array.prototype.slice.call(arguments);
if (args in cache) return cache[args];

var result = func.apply(null, args);

cache[args] = result;
keylist.push(args);
while (keylist.length > limit) keylist.shift();

This comment has been minimized.

Copy link
@erikvold

erikvold Apr 2, 2010

Contributor

This should be:


while (keylist.length > limit) cache[keylist.shift()] = undefined;

This comment has been minimized.

Copy link
@erikvold

erikvold Apr 2, 2010

Contributor

I wrote a fix here in this commit

This comment has been minimized.

Copy link
@arantius

arantius Apr 3, 2010

Collaborator

Actually, no. That will leave the key in the cache with undefined as a value -- any further lookup for that key will return undefined, rather than the proper value. I noticed this, but missed fixing it in the back and forth I was doing. Fix imminent.

This comment has been minimized.

Copy link
@satyr

satyr Apr 8, 2010

keylist.push(args);
while (keylist.length > limit) delete cache[keylist.shift()];

Which can be simplified as:

if (keylist.push(args) > limit) delete cache[keylist.shift()];

return result;
}
}

0 comments on commit 9beb729

Please sign in to comment.