[PP-1] Clean up MemoryCache and MemoryBackend naming issues

Created on 16 May 2018, over 6 years ago
Updated 23 April 2023, over 1 year ago

Problem/Motivation

Drupal\Core\Cache\MemoryCache\MemoryCache is a viable cache backend that can be used as a static cache replacement on its own, or in combination with BackendChain. See 📌 Remove drupal_static from BookManager Fixed for an example.

Drupal\Core\Cache\MemoryBackend is a testing-only implementation of a cache backend, designed to mimic APC or database caching using PHP memory (i.e. it serializes and unserializes).

MemoryBackend should probably move under a testing namespace somewhere (with the current class deprecated), and MemoryCache should be refactored to not extend from it. Potentially we could even reverse the inheritance so that the testing backend inherits from MemoryCache.

Remaining tasks

The installer users MemoryBackend - likely because MemoryCache wasn't available at the time. However changing this should probably happen in isolation in a spin-off issue since it will change behaviour between serialize/unserialize.
#3223580: Use MemoryCache (not MemoryBackend) in the installer

User interface changes

API changes

Data model changes

📌 Task
Status

Postponed

Version

10.1

Component
Cache 

Last updated about 1 hour ago

Created by

🇬🇧United Kingdom catch

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • 🇩🇪Germany geek-merlin Freiburg, Germany
  • 🇧🇪Belgium kristiaanvandeneynde Antwerp, Belgium

    Just want to post the summary of a discussion I had with @catch here.

    MemoryBackend behaves like a proper cache bin in a sense that it always returns a new copy of the cached data. This opposed to MemoryCache that always returns the same object. For this reason we chose not to introduce a factory for MemoryCache nor to tag implementations of said cache with "cache.bin" as we don't want Cache::getBins() to return cache backends that don't really behave like a cache.

    As a result of this, we should also not flag BackendChain implementations using MemoryCache as "cache.bin" but rather as "cache_tags_invalidator". This, as the name implies, ensures that caches still get invalidated based on cache tags, but does not allow the service to make it into the list of cache bins. We currently have 2 implementations in book and jsonapi that need updating as such.

    Finally, we should also adjust RefreshVariablesTrait to reset instances of MemoryCache that were tagged as cache_tags_invalidator because the moment we rename their tag from "cache.bin" to "cache_tags_invalidator", some browser tests might start failing. As demonstrated in #3376846-26: Implement the new access policy API

    An alternative that was also discussed is to introduce a new tag to flag these "I'm a static cache bin, but not really" type of services, pick those up in ListCacheBinsPass and then also call those from the CacheTagsInvalidator and RefreshVariablesTrait.

    With all of the above said, I no longer have any objection to renaming MemoryBackend to something that implies it's to be used in testing only. For all intents and purposes people are encouraged to use MemoryCache, provided we don't let them flag it as a proper cache bin. It's faster in every way and supports cache tags, you just need to treat it as a static in a sense that it always returns the same copy of the cached data (and people might therefore alter it for everyone).

  • 🇧🇪Belgium kristiaanvandeneynde Antwerp, Belgium

    Created 📌 Fix MemoryCache discovery and DX Needs review in light of my latest comment. Once that's cleaned up, it should be easier to move people to MemoryCache and then unblock this issue so we can rename MemoryBackend to something more suitable.

  • Status changed to Active about 1 month ago
  • 🇩🇪Germany geek-merlin Freiburg, Germany

    The other issue is fixed.

  • 🇧🇪Belgium kristiaanvandeneynde Antwerp, Belgium

    Yup, to summarize the other issue: We can now use MemoryCache as long as we declare it as cache.bin.memory. Its factory is cache.backend.memory.memory.

    Example:

      cache.access_policy_memory:
        class: Drupal\Core\Cache\CacheBackendInterface
        tags:
          - { name: cache.bin.memory, default_backend: cache.backend.memory.memory }
        factory: ['@cache_factory', 'get']
        arguments: [access_policy_memory]
    

    This means we should encourage anyone who wasn't using MemoryBackend because of how it works to switch to MemoryCache. We can then rename MemoryBackend to something that implies we only use it in testing to mimic a real cache.

  • 🇩🇪Germany geek-merlin Freiburg, Germany

    #58: I don't see the other issue blocking this.

  • 🇧🇪Belgium kristiaanvandeneynde Antwerp, Belgium

    See #48

Production build 0.71.5 2024