Fix Issue 11644 - EvictingStrategy.LRU for std.functional.memoize

[jercaianu]

I implemented the O(1) algorithm for "get" and "set" from the cache similar to what is described here[1].

Thanks,
Alex

[dlang-bot]

Thanks for your pull request, @jercaianu! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.

Some tips to help speed things up:

• smaller, focused PRs are easier to review than big ones

• try not to mix up refactoring or style changes with bug fixes or feature enhancements

• provide helpful commit messages explaining the rationale behind each change

Bear in mind that large or tricky changes may require multiple rounds of review and revision.

Please see CONTRIBUTING.md for more information.

Bugzilla references

Auto-close	Bugzilla	Description
✓	11644	EvictingStrategy.LRU for std.functional.memoize

[MartinNowak]

Thanks for your PR, looks interesting.

The usage of a doubly linked list and classes bothers me a little from a performance perspective.
Could you use indices and a lazily pre-allocated array instead?

Also most of the issue has been addressed by the random replacement from the cuckoo hashing. Do you have an example that has severe performance issues with that approach?

[andralex]

Where is the link to [1]?

[andralex]

This is a great instance of "academia vs. real world" clash :)

[andralex]

I searched this page and found no use of lru.

[andralex]

A class with dynamic allocation and linked in a doubly linked list is textbook-correct but certainly out of character for phobos and for high-performance applications. Per @MartinNowak we'd be looking at a properly managed contiguous array,

[andralex]

Hmmm, the defaults here are suprising - most functions will NOT return the default result for the default parameter values. Where are the defaults used?

[andralex]

size_t probably

[andralex]

By the way LRU sux we should use LIRS: https://en.wikipedia.org/wiki/LIRS_caching_algorithm

[MartinNowak]

It's very welcome to review and properly benchmark the current approach.
#2591
If we find this to be inadequate for certain usages, we could look at different replacement policies.

Not too certain we really need a fancy eviction policy here, since we have a uniformly distributed hash (MD5), random replacement should be fairly OKish, and just using a slightly bigger cache will fix performance problems.
Of course a better eviction policy can further optimize for cache size vs. hit ratio.

If we go that route we should replace the current cuckoo hash implementation.

Hopefully we can find a general purpose replacement policy. Making it selectable via a template parameter does increase the complexity of memoize even further. An adaptive cache size would be nice as well (but hard), as it's often difficult to come up with a good number ahead of time.

[ben-manes]

LIRS is very complicated to implement correctly. Most implementations miss key details that degrades the hit rates, have memory leaks due to the paper neglecting to bound the ghost entries (as done in their C code), and may have jitter due to slow stack pruning if bounded inefficiently. However, it does have an excellent hit rate across many workloads and can be speedy.

After doing a deep dive into approaches for my caching library, I chose TinyLFU with some minor adaptions. It is very easy to implement, time/space efficient, and beats LIRS in many real-world workloads. As an admission policy, it can be paired with any eviction policy (such as an array-based approach rather than linked lists - e.g. sampled LRU).

[jercaianu]

@ben-manes @MartinNowak
Thanks for the comments!
I have left this issue on the backburner, since I've been working more on allocators.
I will definitely take a look at TinyLFU.

Alex