From 8cf3625b118344167cb53fb8bfc77cc5b1c225f2 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Mon, 24 Sep 2018 19:16:30 +0200 Subject: [PATCH] [Cache] add "setCallbackWrapper()" on adapters implementing CacheInterface for more flexibility --- .../Component/Cache/Adapter/ChainAdapter.php | 2 +- .../Cache/Adapter/PhpArrayAdapter.php | 2 +- .../Component/Cache/Adapter/ProxyAdapter.php | 2 +- src/Symfony/Component/Cache/LockRegistry.php | 34 ++------- .../Component/Cache/Traits/GetTrait.php | 75 ++++++++++++++----- 5 files changed, 65 insertions(+), 50 deletions(-) diff --git a/src/Symfony/Component/Cache/Adapter/ChainAdapter.php b/src/Symfony/Component/Cache/Adapter/ChainAdapter.php index 51a778bb69..10305cc1e8 100644 --- a/src/Symfony/Component/Cache/Adapter/ChainAdapter.php +++ b/src/Symfony/Component/Cache/Adapter/ChainAdapter.php @@ -100,7 +100,7 @@ class ChainAdapter implements AdapterInterface, CacheInterface, PruneableInterfa if ($adapter instanceof CacheInterface) { $value = $adapter->get($key, $callback, $beta); } else { - $value = $this->doGet($adapter, $key, $callback, $beta ?? 1.0); + $value = $this->doGet($adapter, $key, $callback, $beta); } if (null !== $item) { ($this->syncItem)($lastItem = $lastItem ?? $item, $item); diff --git a/src/Symfony/Component/Cache/Adapter/PhpArrayAdapter.php b/src/Symfony/Component/Cache/Adapter/PhpArrayAdapter.php index 1a11e69f05..cbc3e8af2c 100644 --- a/src/Symfony/Component/Cache/Adapter/PhpArrayAdapter.php +++ b/src/Symfony/Component/Cache/Adapter/PhpArrayAdapter.php @@ -93,7 +93,7 @@ class PhpArrayAdapter implements AdapterInterface, CacheInterface, PruneableInte return $this->pool->get($key, $callback, $beta); } - return $this->doGet($this->pool, $key, $callback, $beta ?? 1.0); + return $this->doGet($this->pool, $key, $callback, $beta); } $value = $this->values[$this->keys[$key]]; diff --git a/src/Symfony/Component/Cache/Adapter/ProxyAdapter.php b/src/Symfony/Component/Cache/Adapter/ProxyAdapter.php index 32d9f90adc..9fb720ca39 100644 --- a/src/Symfony/Component/Cache/Adapter/ProxyAdapter.php +++ b/src/Symfony/Component/Cache/Adapter/ProxyAdapter.php @@ -94,7 +94,7 @@ class ProxyAdapter implements AdapterInterface, CacheInterface, PruneableInterfa public function get(string $key, callable $callback, float $beta = null) { if (!$this->pool instanceof CacheInterface) { - return $this->doGet($this, $key, $callback, $beta ?? 1.0); + return $this->doGet($this, $key, $callback, $beta); } return $this->pool->get($this->getId($key), function ($innerItem) use ($key, $callback) { diff --git a/src/Symfony/Component/Cache/LockRegistry.php b/src/Symfony/Component/Cache/LockRegistry.php index 606eb15dce..258f225e64 100644 --- a/src/Symfony/Component/Cache/LockRegistry.php +++ b/src/Symfony/Component/Cache/LockRegistry.php @@ -11,8 +11,8 @@ namespace Symfony\Component\Cache; -use Psr\Cache\CacheItemInterface; -use Psr\Cache\CacheItemPoolInterface; +use Symfony\Contracts\Cache\CacheInterface; +use Symfony\Contracts\Cache\ItemInterface; /** * LockRegistry is used internally by existing adapters to protect against cache stampede. @@ -75,40 +75,20 @@ class LockRegistry return $previousFiles; } - /** - * @internal - */ - public static function save(string $key, CacheItemPoolInterface $pool, CacheItemInterface $item, callable $callback, float $startTime, &$value): bool + public static function compute(ItemInterface $item, callable $callback, CacheInterface $pool) { - self::$save = self::$save ?? \Closure::bind( - function (CacheItemPoolInterface $pool, CacheItemInterface $item, $value, float $startTime) { - if ($item instanceof CacheItem && $startTime && $item->expiry > $endTime = microtime(true)) { - $item->newMetadata[CacheItem::METADATA_EXPIRY] = $item->expiry; - $item->newMetadata[CacheItem::METADATA_CTIME] = 1000 * (int) ($endTime - $startTime); - } - $pool->save($item->set($value)); - - return $value; - }, - null, - CacheItem::class - ); - - $key = self::$files ? crc32($key) % \count(self::$files) : -1; + $key = self::$files ? crc32($item->getKey()) % \count(self::$files) : -1; if ($key < 0 || (self::$lockedFiles[$key] ?? false) || !$lock = self::open($key)) { - $value = (self::$save)($pool, $item, $callback($item), $startTime); - - return true; + return $callback($item); } try { // race to get the lock in non-blocking mode if (flock($lock, LOCK_EX | LOCK_NB)) { self::$lockedFiles[$key] = true; - $value = (self::$save)($pool, $item, $callback($item), $startTime); - return true; + return $callback($item); } // if we failed the race, retry locking in blocking mode to wait for the winner flock($lock, LOCK_SH); @@ -117,7 +97,7 @@ class LockRegistry unset(self::$lockedFiles[$key]); } - return false; + return $pool->get($item->getKey(), $callback, 0); } private static function open(int $key) diff --git a/src/Symfony/Component/Cache/Traits/GetTrait.php b/src/Symfony/Component/Cache/Traits/GetTrait.php index d55f3ae0df..fd90db67fe 100644 --- a/src/Symfony/Component/Cache/Traits/GetTrait.php +++ b/src/Symfony/Component/Cache/Traits/GetTrait.php @@ -11,48 +11,62 @@ namespace Symfony\Component\Cache\Traits; -use Psr\Cache\CacheItemPoolInterface; +use Symfony\Component\Cache\Adapter\AdapterInterface; use Symfony\Component\Cache\CacheItem; use Symfony\Component\Cache\Exception\InvalidArgumentException; use Symfony\Component\Cache\LockRegistry; +use Symfony\Contracts\Cache\CacheInterface; +use Symfony\Contracts\Cache\ItemInterface; /** - * An implementation for CacheInterface that provides stampede protection via probabilistic early expiration. - * - * @see https://en.wikipedia.org/wiki/Cache_stampede - * * @author Nicolas Grekas * * @internal */ trait GetTrait { + private $callbackWrapper = array(LockRegistry::class, 'compute'); + + /** + * Wraps the callback passed to ->get() in a callable. + * + * @param callable(ItemInterface, callable, CacheInterface):mixed $callbackWrapper + * + * @return callable the previous callback wrapper + */ + public function setCallbackWrapper(callable $callbackWrapper): callable + { + $previousWrapper = $this->callbackWrapper; + $this->callbackWrapper = $callbackWrapper; + + return $previousWrapper; + } + /** * {@inheritdoc} */ public function get(string $key, callable $callback, float $beta = null) { - if (0 > $beta) { + return $this->doGet($this, $key, $callback, $beta); + } + + private function doGet(AdapterInterface $pool, string $key, callable $callback, ?float $beta) + { + if (0 > $beta = $beta ?? 1.0) { throw new InvalidArgumentException(sprintf('Argument "$beta" provided to "%s::get()" must be a positive number, %f given.', \get_class($this), $beta)); } - return $this->doGet($this, $key, $callback, $beta ?? 1.0); - } - - private function doGet(CacheItemPoolInterface $pool, string $key, callable $callback, float $beta) - { - retry: $t = 0; $item = $pool->getItem($key); $recompute = !$item->isHit() || INF === $beta; - if ($item instanceof CacheItem && 0 < $beta) { + if (0 < $beta) { if ($recompute) { $t = microtime(true); } else { $metadata = $item->getMetadata(); - $expiry = $metadata[CacheItem::METADATA_EXPIRY] ?? false; - $ctime = $metadata[CacheItem::METADATA_CTIME] ?? false; + $expiry = $metadata[ItemInterface::METADATA_EXPIRY] ?? false; + $ctime = $metadata[ItemInterface::METADATA_CTIME] ?? false; if ($ctime && $expiry) { $t = microtime(true); @@ -69,11 +83,32 @@ trait GetTrait return $item->get(); } - if (!LockRegistry::save($key, $pool, $item, $callback, $t, $value)) { - $beta = 0; - goto retry; - } + static $save; - return $value; + $save = $save ?? \Closure::bind( + function (AdapterInterface $pool, ItemInterface $item, $value, float $startTime) { + if ($startTime && $item->expiry > $endTime = microtime(true)) { + $item->newMetadata[ItemInterface::METADATA_EXPIRY] = $item->expiry; + $item->newMetadata[ItemInterface::METADATA_CTIME] = 1000 * (int) ($endTime - $startTime); + } + $pool->save($item->set($value)); + + return $value; + }, + null, + CacheItem::class + ); + + // don't wrap nor save recursive calls + if (null === $callbackWrapper = $this->callbackWrapper) { + return $callback($item); + } + $this->callbackWrapper = null; + + try { + return $save($pool, $item, $callbackWrapper($item, $callback, $pool), $t); + } finally { + $this->callbackWrapper = $callbackWrapper; + } } }