Consolidate cache_memoize state and functionality into a CacheContext.

Review Request #12377 — Created June 15, 2022 and submitted


We pass a lot of state around the various functions that comprise
cache_memoize() and cache_memoize_iter(), and we have a lot of key
building that we duplicate everywhere. Some upcoming work will be making
a lot of this even more complex, so in preparation for that, this change
cleans up much of this state and logic.

There's now an internal _CacheContext class, which stores the state
being used for a caching operation. This includes:

  • The cache connection
  • The base key name passed to cache_memoize()/cache_memoize_iter()
  • the full key name generated through make_cache_key()
  • The timeout
  • Whether to compress large data

This simplifies a lot of function signatures.

It then provides functions for generating suitable keys (wrapping
make_cache_key(), subkeys (used for storing chunks), loading cached
data, and storing cached data. Right now, these are mostly very simple
wrappers, but they'll soon be extended for additional checks and
functionality, hence the consolidation of this logic.

There's also a performance improvement/race condition fix here. Both
cache_memoize() and cache_memoize_iter() previously checked if a
value was in cache, and then pulled it out. This requires two hits to
cache instead of one, and it's possible (though unlikely) that the value
could disappear in the meantime. In practice, we'd gracefully handle the
failure, but the two cache hits weren't necessary.

We now pull the value out, defaulting the result to a _NO_RESULTS
sentinel if not found. We then make a determination based on this.

Aside from the performance improvement, there are no changes to how
the cache is utilized. This does make extensive improvements to our
documentation, and improves some logging (adding a missing "Cache miss"
for cache_memoize() to go along with that in cache_memoize_iter()),
and logging full cache keys).

All Djblets and Review Board unit tests pass.

Consolidate cache_memoize state and functionality into a CacheContext.
Description From Last Updated

Since this is purely internal API, do we really need to document changes?

  2. djblets/cache/ (Diff revision 1)

    Since this is purely internal API, do we really need to document changes?

    1. Probably not. I just felt, does more good than harm to record when this changed, since I was updating docs anyway.

  1. Ship It!
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-3.x (615f5dc)