Ensure flexicache is LRU cached

[pydanny]

Originating ticket: #678

Problem

The Python docs say that dict.popitem is LIFO, which means it is Last In First Out. That is the opposite of FIFO, which is part of the algorithm often used in LRU (Least Recently Used).

Reference: https://docs.python.org/3/library/stdtypes.html#dict.popitem

Solution

Since the cache is constantly being popped and reset, treat the order of the cache keys as an iterator and pick the first cache key set. To do this we just call the next command once on iter(cache).

if len(cache) > maxsize: del cache[next(iter(cache))]

Tests

As cache invalidation is hard, new tests are added to flexicache that document cache invalidation and resetting.

Alternative approach: OrderedDict

collections.OrderedDict has a popitem() method with a last boolean that replicates our design, as show in the code example below:

cache = OrderedDict() ... if len(cache) > maxsize: cache.popitem(last=False)

I rejected this approach because it adds an extra import and cache.popitem() is probably a very tiny bit slower than del cache[next(iter(cache))]. However, this approach is arguably more clear. In any case, I'm not opposed to changing out the current solution for the one presented here.

[jph00]

Good find @pydanny, thanks! Let's use the OrderedDict approach, since we value simplicity most highly. BTW looking at imports, I see that asyncio is imported in that function and not used, so perhaps replace it with the collections import?

[jph00]

Oh BTW don't forget nbdev_clean. Otherwise the diffs are hard to read in PRs.

[pydanny]

I switched the PR to use the OrderedDict approach. As for the use of asyncio, I moved it out of the decorator and to the initial module imports. It is used in async_wrapper of flexicache.

Ran nbdev_clean but it still looks messy. 😞

[jph00]

Oh sorry you're right I didn't see asyncio being used at the bottom there! OK in that case you should put it back where it was -- in fastcore.xtras we prefer to only import rarely used modules if/when they're actually used. You should also only import collections inside the function for the same reason. (And thank you, the diff is much cleaner and easier to read than before cleaning :) )

[pydanny]

Why are all the tests now passing on GitHub?

I was stumped by the failing test that had emerged, being unable to replicate it locally. Here is the failing command:

nbdev_test --flags '' --n_workers 3 --pause 1.5 --file_re "[0-9][0-1].*"

Now having moved the imports to within the function all the tests are now passing on GitHub. Why is that the case?

[jph00]

I don't know - I didn't see the failing tests so can't really guess. But glad it's working now :)

[pydanny]

I still can't replicate it locally. For now I'm going to consider it a temporary GitHub problem.