Create a cache backend that pre loads multiple items in one getMultiple() call

Just some really quick review.

1. +++ b/core/lib/Drupal/Core/Cache/MultiBackend.php
   @@ -0,0 +1,210 @@
   +    $this->bin = 'cache_multi_' . $bin;
   

   Do we really want to have an entire bin per wrapped bin?

2. +++ b/core/lib/Drupal/Core/Cache/MultiBackend.php
   @@ -0,0 +1,210 @@
   +  public function get($cid, $allow_invalid = FALSE) {
   +    return isset($this->cacheItems[$cid]) ? $this->cacheItems[$cid] : FALSE;
   +  }
   ...
   +   */
   +  public function getMultiple(&$cids, $allow_invalid = FALSE) {
   +    return array_intersect_key($this->cacheItems, array_flip($cids));
   +  }
   

   Shouldn't we otherwise call out to the wrapped cache backend, if we can't find some entry?

3. +++ b/core/lib/Drupal/Core/Cache/MultiBackend.php
   @@ -0,0 +1,210 @@
   +   */
   +  public function delete($cid) {
   +    $this->cacheBackend->delete($cid);
   +    $this->removeMultiKeys([$cid]);
   +  }
   

   any reason to not call out to deleteMultiple? I guess deleting doesn't happen that often?

Wow it's green. Nice work @damiankloip

1. +++ b/core/lib/Drupal/Core/Cache/MultiBackend.php
   @@ -0,0 +1,344 @@
   +      $this->set($cid, $item['data'], isset($item['expire']) ? $item['expire'] : CacheBackendInterface::CACHE_PERMANENT, isset($item['tags']) ? $item['tags'] : []);
   
   Looking at the <code>set

   and setMultiple code I think this should be $this->cacheItems[$cid] = $this->createCacheObject($cid, $data, $expire, $tags);

2. +++ b/core/lib/Drupal/Core/Cache/MultiBackend.php
   @@ -0,0 +1,344 @@
   +      $this->set($cid, $item['data'], isset($item['expire']) ? $item['expire'] : CacheBackendInterface::CACHE_PERMANENT, isset($item['tags']) ? $item['tags'] : []);
   ...
   +    $return = $this->cacheBackend->setMultiple($items);
   

   Isn't this call redundant after above call? We can copy the return value of set in $return[$cid].

3. +++ b/core/lib/Drupal/Core/Cache/MultiBackend.php
   @@ -0,0 +1,344 @@
   +    $this->addMultiKeys(array_keys($items));
   

   If we are calling $this->set then this is also useless.

4. +++ b/core/lib/Drupal/Core/Cache/MultiBackend.php
   @@ -0,0 +1,344 @@
   +   * Implements Drupal\Core\Cache\CacheBackendInterface::invalidateTags().
   

   @inheritdoc

5. +++ b/core/lib/Drupal/Core/Cache/MultiBackend.php
   @@ -0,0 +1,344 @@
   +   * @param $cid
   +   * @param $data
   +   * @param $expire
   +   * @param array $tags
   ...
   +   * @return \stdClass
   

   Description block missing.

1. +++ b/core/lib/Drupal/Core/Cache/MultiBackend.php
   @@ -0,0 +1,355 @@
   +/**
   + * A proxy cache backend to preload multiple items.
   + */
   +class MultiBackend implements CacheBackendInterface, CacheTagsInvalidatorInterface {
   

   Do you mind added an explanation about the strategies used internally and maybe usecases of that implementation?

2. +++ b/core/lib/Drupal/Core/Cache/MultiBackend.php
   @@ -0,0 +1,355 @@
   +    $this->multiCidKey = 'cache_multi_' . $bin;
   

   Should we maybe make the CID overridable as well? This sounds like it could potentially easily collide.

3. +++ b/core/lib/Drupal/Core/Cache/MultiBackend.php
   @@ -0,0 +1,355 @@
   +
   +    // @todo Lazy load this?
   +    $this->preloadMultiple();
   

   It feels like its wrong to do it that early

4. +++ b/core/modules/views/views.services.yml
   @@ -1,61 +1,64 @@
   +  cache.views_plugin:
   +    class: Drupal\Core\Cache\MultiBackend
   +    arguments: ['@cache.discovery', views_plugins]
   

   I guess we can mark this service as not public

A quick review

1. +++ b/core/lib/Drupal/Core/Cache/MultiBackend.php
   @@ -9,6 +9,14 @@
   + * This is useful for cases like Views. Where there are multiple plugin types
   + * that are always needed to execute a view. Instead of loading all cached
   

   Where ... view, that sentence sounds kinda wrong

2. +++ b/core/lib/Drupal/Core/Cache/MultiBackend.php
   @@ -48,26 +56,35 @@ class MultiBackend implements CacheBackendInterface, CacheTagsInvalidatorInterfa
   +  public function __construct(CacheBackendInterface $cache_backend, $bin, $multi_cid_prefix = 'cache_multi') {
   ...
   +    $this->multiCidKey = $multi_cid_prefix . ':' . $bin;
   

   +1

As well as locking, we need to ensure the list is always additive to avoid race conditions.

So lock -> get existing entry -> add items -> set.

Then no cross-request flip-flopping for different sets of items in the list. This is the same approach that CacheCollector takes. I think that's equally necessary whether we write on every set or at end of request.

And also need to lock when deleting with the same lock name as set uses.

1. +++ b/core/lib/Drupal/Core/Cache/MultiBackend.php
   @@ -0,0 +1,419 @@
   +      // @todo Move this to set on destruct?
   +      $this->cacheBackend->set($this->multiCidKey, $multi_cid_items);
   

   so DestructableInterface? Well the question is whether we want to propagate that information rather earlier than later to potential new requests

2. +++ b/core/lib/Drupal/Core/Cache/MultiBackend.php
   @@ -0,0 +1,419 @@
   +      'created' => round(microtime(TRUE), 3),
   

   What is the idea for that? Maybe some docs?

I think destructable interface is OK here. In the case of a cache miss, it just means we fetch individual cache items instead of the multiple get. That is probably not much more expensive (or could even be cheaper) than trying to acquire/release the lock several times in a request.

Just doing some D8 Accelerate house-cleaning. Any update as to where this is at? I don't see a report at https://assoc.drupal.org/d8-accelerate-awarded-grants and there hasn't been movement here in a couple of months.

Since this hasn't been updated in 7 years wonder if this is still a valid task.

If so think an issue summary update could help + there is feedback requested in #24

Since there hasn't been a follow up in 11 months and 7 years before that going to close as outdated for now.

If still valid though please reopen.