flru icon indicating copy to clipboard operation
flru copied to clipboard

Addl methods?

Open lukeed opened this issue 6 years ago • 5 comments
trafficstars

Could add keys, values, and/or forEach methods... but the problem is whether or not to include the "stale" (prev) cache values. These are technically on their way out & not necessarily current values, but may be. 🤔

Since a LRU is really just a set and get anyway, this may fall out of scope anyway.

Could also add a remove method, but it still falls out of scope. At least it doesn't have the same stale-vs-not concern.

Extracted snippet before release. Removing methods is breaking, but adding them is not~

remove: function (key) {
  if (curr[key] !== void 0) {
    curr[key] = void 0;
  }
  if (prev[key] !== void 0) {
    prev[key] = void 0;
  }
},
keys: function () {
  var k, arr=[];
  for (k in curr) arr.push(k);
  return arr;
},
values: function () {
  var k, arr=[];
  for (k in curr) arr.push(curr[k]);
  return arr;
},
forEach: function (fn, ctx) {
  ctx = ctx || this;
  for (var k in curr) {
    fn.call(ctx, curr[k], k, curr);
  }
}

lukeed avatar Dec 31 '18 23:12 lukeed

How about extending Map, then you get those for free?

exside avatar Mar 25 '23 17:03 exside

Please see tmp-cache. Does exactly this. And it’s much slower than flru as a result

lukeed avatar Mar 25 '23 17:03 lukeed

I checked ;), but like your short code better, how badly do you think will this refactor affect performance?

function LRU(max) {
	let num = 0;
	let curr = new Map();
	let prev = new Map();
	const limit = max || 1;

	function keep(key, value) {
		if (++num > limit) {
			prev = curr;
			reset(1);
			num++;
		}
		curr.set(key, value);
	}

	function reset(isPartial) {
		num = 0;
		curr.clear();
		isPartial || prev.clear();
	}

	reset();

	return {
		clear: () => reset(),
		has: key => curr.has(key) || prev.has(key),
		get: key => {
			let val = curr.get(key);
			if (val !== undefined) return val;
			if (prev.has(key)) {
				val = prev.get(key);
				keep(key, val);
				return val;
			}
		},
		set: (key, value) => curr.has(key) ? curr.set(key, value) : keep(key, value)
	};
}

exside avatar Mar 25 '23 17:03 exside

tmp-cache is also mine :) Initializing two Maps is costly, significantly more than object dictionaries. At scale, one can’t implement the map methods more efficiently than what Map already does, which is why the setup costs as much as it does.

That aside, you’re welcome to bench it! But Map performance aside, I don’t see any reason to need Maps in this code snippet. The same API can be done & be more performant than Map backend below a certain item count threshold

lukeed avatar Mar 25 '23 18:03 lukeed

Oh, no other reason than some kind of code-OCD 🤣 , I just like the "cleanness" of Map's .has .get .clear...that's all, but if it messes up performance it's not worth it...

exside avatar Mar 25 '23 21:03 exside