Add support for generics

[SoMuchForSubtlety]

This PR adds support for generics.

Because there is no generic list type in the standard library yet, I added a minimal version of container/list with added generics support (I kept the licence header, please let me know if it should be removed).

It would also be possible to keep using container/list as is, and just update the public API (take care of interface casts internally), but most of the performance gain comes from removing interface conversions.

Benchmark results against master

name         old time/op    new time/op    delta
2Q_Rand-16     1.22µs ±10%    0.52µs ± 8%   -57.69%  (p=0.000 n=20+19)
2Q_Freq-16     1.03µs ±10%    0.44µs ±12%   -57.42%  (p=0.000 n=18+20)
ARC_Rand-16    1.51µs ± 9%    0.70µs ±15%   -53.63%  (p=0.000 n=20+20)
ARC_Freq-16    1.22µs ±12%    0.54µs ± 7%   -56.20%  (p=0.000 n=20+20)
LRU_Rand-16     401ns ± 9%     215ns ± 6%   -46.46%  (p=0.000 n=20+20)
LRU_Freq-16     376ns ±13%     194ns ± 7%   -48.51%  (p=0.000 n=19+20)

name         old alloc/op   new alloc/op   delta
2Q_Rand-16       136B ± 0%       67B ± 0%   -50.74%  (p=0.000 n=20+18)
2Q_Freq-16       124B ± 0%       59B ± 0%   -52.42%  (p=0.000 n=16+20)
ARC_Rand-16      165B ± 0%       84B ± 1%   -49.03%  (p=0.000 n=20+20)
ARC_Freq-16      139B ± 3%       68B ± 5%   -51.01%  (p=0.000 n=20+20)
LRU_Rand-16     76.0B ± 0%     36.0B ± 0%   -52.63%  (p=0.000 n=20+20)
LRU_Freq-16     71.0B ± 0%     33.0B ± 0%   -53.52%  (p=0.000 n=19+20)

name         old allocs/op  new allocs/op  delta
2Q_Rand-16       5.00 ± 0%      1.00 ± 0%   -80.00%  (p=0.000 n=20+20)
2Q_Freq-16       5.00 ± 0%      1.00 ± 0%   -80.00%  (p=0.000 n=20+20)
ARC_Rand-16      6.00 ± 0%      1.00 ± 0%   -83.33%  (p=0.000 n=20+20)
ARC_Freq-16      5.00 ± 0%      1.00 ± 0%   -80.00%  (p=0.000 n=20+20)
LRU_Rand-16      3.00 ± 0%      0.00       -100.00%  (p=0.000 n=20+20)
LRU_Freq-16      3.00 ± 0%      0.00       -100.00%  (p=0.000 n=20+20)

[jefferai]

Overall this looks great! Some minor feedback here and there that will be easy to address.

[kasegao]

Thank you for your great PR.

In order to keep backwward compability, however, wouldn't it be better to keep the existing implementation as suggested in AskAlexSharov's comment?

#101 (comment)

[jefferai]

@SoMuchForSubtlety Please don't force-push when people are reviewing, it tends to really screw up comments. If you must force-push, please do it just before merging or before marking ready for review.

[jefferai]

@kasegao If this is marked as v2 of the lib we don't need backwards compat.

[kasegao]

@jefferai I misunderstood. Thank you!

[SoMuchForSubtlety]

golangci-lint fails because of a few false positives. I could disable the offending linters (golint and gosec).
https://github.com/SoMuchForSubtlety/golang-lru/actions/runs/3347498929

There are warnings about other enabled linters being deprecated

WARN [runner] The linter 'structcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter.  Replaced by unused. 
WARN [runner] The linter 'deadcode' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter.  Replaced by unused. 
WARN [runner] The linter 'scopelint' is deprecated (since v1.39.0) due to: The repository of the linter has been deprecated by the owner.  Replaced by exportloopref. 
WARN [runner] The linter 'varcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter.  Replaced by unused. 
WARN [linters_context] structcheck is disabled because of generics. You can track the evolution of the generics support by following the https://github.com/golangci/golangci-lint/issues/2649. 

[jefferai]

I just bumped the workflows version in CI in main, you may want to try updating from there and seeing if that helps.

[SoMuchForSubtlety]

I just bumped the workflows version in CI in main, you may want to try updating from there and seeing if that helps.

Rebasing on master gets rid of the math/rand warnings, but I still get the errors from golint. It's deprecated¹ and does not have support for generics, leading to it getting confused by generic method signatures.

Footnotes

1. https://github.com/golang/go/issues/38968 ↩

[jefferai]

I removed some deprecated linters from the config -- didn't even realize we had a config and weren't using defaults. Hopefully that fixes it.

[SoMuchForSubtlety]

I removed some deprecated linters from the config -- didn't even realize we had a config and weren't using defaults. Hopefully that fixes it.

Thanks, I rebased on master and at least locally golangci-lint doesn't complain any more.

Is there anything else open I need to change?

[jefferai]

I will do another pass -- as for stuff outstanding I still have a slight preference not to do bare return when nil/default values are being returned from Get functions considering we're not actually populating the named return parameters anywhere. Keeping them named is nice from a doc perspective, but we could explicitly return the false/nil values. But it's more a style thing than anything else. 🤷

[SoMuchForSubtlety]

Keeping them named is nice from a doc perspective, but we could explicitly return the false/nil values.

Do you mean

// Peek returns the key value (or undefined if not found) without updating
// the "recently used"-ness of the key.
func (c *LRU[K, V]) Peek(key K) (value V, ok bool) {
	var ent *entry[K, V]
	if ent, ok = c.items[key]; ok {
		return ent.value, true
	}
	return value, false
}

or

// Peek returns the key value (or undefined if not found) without updating
// the "recently used"-ness of the key.
func (c *LRU[K, V]) Peek(key K) (value V, ok bool) {
	var ent *entry[K, V]
	if ent, ok = c.items[key]; ok {
		return ent.value, true
	}
	var empty V
	return empty, false
}

They are the same, but the seconds one makes it clearer that we are returning an empty value (at the cost of an added line).

I would be happy to switch to either style.

[jefferai]

Hm. I keep forgetting that V is not necessarily nil anymore :-)

Leave it be.

[jefferai]

Looks great! I have a question for anyone following this:

I'm tempted to cut a final v0 tag, set this as v2, and have v1 point to a branch that has a README that basically says, use v0 if you need backwards compat or use v2. This would break users that auto-upgrade until they select a specific v0 (or v2) tag, but also pushes people towards using v2 given that it's faster and more capable.

Thoughts?

[bep]

Thoughts?

I agree (I do, though, wish Go Modules had a way to handle this cleaner).

[jefferai]

@SoMuchForSubtlety Any thoughts/preferences on my above question?

[SoMuchForSubtlety]

Sounds reasonable, I have no objections :)

[jefferai]

Amazing work! Thanks!

[Kubuxu]

As the eviction tracking LRU caches don't use the value, whouldn't it be better to use LRUCache[K, struct{}] here? I might be missing something, but I can open a PR if I don't.