Skip to content

Commit 3cd411a

Browse files
fabpotfabpot
committed
feature symfony#28588 [Cache] add "setCallbackWrapper()" on adapters implementing CacheInterface for more flexibility (nicolas-grekas)
This PR was merged into the 4.2-dev branch. Discussion ---------- [Cache] add "setCallbackWrapper()" on adapters implementing CacheInterface for more flexibility | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | symfony#27730 | License | MIT | Doc PR | - Preparing my talk at SymfonyLive London, see you all there :) This allows wrapping the callback passed to `->get($item, $callback, $beta)` in a callable that should at least return `$callback($item)`, but can do something around the call. The default wrapper is locking the key to provide lock-based stampede protection. That was already the case before this PR, but in a much dirtier way at the design level. Fixes a few issues found meanwhile. Commits ------- 8cf3625 [Cache] add "setCallbackWrapper()" on adapters implementing CacheInterface for more flexibility
2 parents e6deb09 + 8cf3625 commit 3cd411a

File tree

5 files changed

+64
-49
lines changed

5 files changed

+64
-49
lines changed

‎src/Symfony/Component/Cache/Adapter/ChainAdapter.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ public function get(string $key, callable $callback, float $beta = null)
100100
if ($adapter instanceof CacheInterface) {
101101
$value = $adapter->get($key, $callback, $beta);
102102
} else {
103-
$value = $this->doGet($adapter, $key, $callback, $beta ?? 1.0);
103+
$value = $this->doGet($adapter, $key, $callback, $beta);
104104
}
105105
if (null !== $item) {
106106
($this->syncItem)($lastItem = $lastItem ?? $item, $item);

‎src/Symfony/Component/Cache/Adapter/PhpArrayAdapter.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ public function get(string $key, callable $callback, float $beta = null)
9393
return $this->pool->get($key, $callback, $beta);
9494
}
9595

96-
return $this->doGet($this->pool, $key, $callback, $beta ?? 1.0);
96+
return $this->doGet($this->pool, $key, $callback, $beta);
9797
}
9898
$value = $this->values[$this->keys[$key]];
9999

‎src/Symfony/Component/Cache/Adapter/ProxyAdapter.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ function (CacheItemInterface $innerItem, array $item) {
9494
public function get(string $key, callable $callback, float $beta = null)
9595
{
9696
if (!$this->pool instanceof CacheInterface) {
97-
return $this->doGet($this, $key, $callback, $beta ?? 1.0);
97+
return $this->doGet($this, $key, $callback, $beta);
9898
}
9999

100100
return $this->pool->get($this->getId($key), function ($innerItem) use ($key, $callback) {

‎src/Symfony/Component/Cache/LockRegistry.php‎

Lines changed: 7 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@
1111

1212
namespace Symfony\Component\Cache;
1313

14-
use Psr\Cache\CacheItemInterface;
15-
use Psr\Cache\CacheItemPoolInterface;
14+
use Symfony\Contracts\Cache\CacheInterface;
15+
use Symfony\Contracts\Cache\ItemInterface;
1616

1717
/**
1818
* LockRegistry is used internally by existing adapters to protect against cache stampede.
@@ -75,40 +75,20 @@ public static function setFiles(array $files): array
7575
return $previousFiles;
7676
}
7777

78-
/**
79-
* @internal
80-
*/
81-
public static function save(string $key, CacheItemPoolInterface $pool, CacheItemInterface $item, callable $callback, float $startTime, &$value): bool
78+
public static function compute(ItemInterface $item, callable $callback, CacheInterface $pool)
8279
{
83-
self::$save = self::$save ?? \Closure::bind(
84-
function (CacheItemPoolInterface $pool, CacheItemInterface $item, $value, float $startTime) {
85-
if ($item instanceof CacheItem && $startTime && $item->expiry > $endTime = microtime(true)) {
86-
$item->newMetadata[CacheItem::METADATA_EXPIRY] = $item->expiry;
87-
$item->newMetadata[CacheItem::METADATA_CTIME] = 1000 * (int) ($endTime - $startTime);
88-
}
89-
$pool->save($item->set($value));
90-
91-
return $value;
92-
},
93-
null,
94-
CacheItem::class
95-
);
96-
97-
$key = self::$files ? crc32($key) % \count(self::$files) : -1;
80+
$key = self::$files ? crc32($item->getKey()) % \count(self::$files) : -1;
9881

9982
if ($key < 0 || (self::$lockedFiles[$key] ?? false) || !$lock = self::open($key)) {
100-
$value = (self::$save)($pool, $item, $callback($item), $startTime);
101-
102-
return true;
83+
return $callback($item);
10384
}
10485

10586
try {
10687
// race to get the lock in non-blocking mode
10788
if (flock($lock, LOCK_EX | LOCK_NB)) {
10889
self::$lockedFiles[$key] = true;
109-
$value = (self::$save)($pool, $item, $callback($item), $startTime);
11090

111-
return true;
91+
return $callback($item);
11292
}
11393
// if we failed the race, retry locking in blocking mode to wait for the winner
11494
flock($lock, LOCK_SH);
@@ -117,7 +97,7 @@ function (CacheItemPoolInterface $pool, CacheItemInterface $item, $value, float
11797
unset(self::$lockedFiles[$key]);
11898
}
11999

120-
return false;
100+
return $pool->get($item->getKey(), $callback, 0);
121101
}
122102

123103
private static function open(int $key)

‎src/Symfony/Component/Cache/Traits/GetTrait.php‎

Lines changed: 54 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -11,48 +11,62 @@
1111

1212
namespace Symfony\Component\Cache\Traits;
1313

14-
use Psr\Cache\CacheItemPoolInterface;
14+
use Symfony\Component\Cache\Adapter\AdapterInterface;
1515
use Symfony\Component\Cache\CacheItem;
1616
use Symfony\Component\Cache\Exception\InvalidArgumentException;
1717
use Symfony\Component\Cache\LockRegistry;
18+
use Symfony\Contracts\Cache\CacheInterface;
19+
use Symfony\Contracts\Cache\ItemInterface;
1820

1921
/**
20-
* An implementation for CacheInterface that provides stampede protection via probabilistic early expiration.
21-
*
22-
* @see https://en.wikipedia.org/wiki/Cache_stampede
23-
*
2422
* @author Nicolas Grekas <p@tchwork.com>
2523
*
2624
* @internal
2725
*/
2826
trait GetTrait
2927
{
28+
private $callbackWrapper = array(LockRegistry::class, 'compute');
29+
30+
/**
31+
* Wraps the callback passed to ->get() in a callable.
32+
*
33+
* @param callable(ItemInterface, callable, CacheInterface):mixed $callbackWrapper
34+
*
35+
* @return callable the previous callback wrapper
36+
*/
37+
public function setCallbackWrapper(callable $callbackWrapper): callable
38+
{
39+
$previousWrapper = $this->callbackWrapper;
40+
$this->callbackWrapper = $callbackWrapper;
41+
42+
return $previousWrapper;
43+
}
44+
3045
/**
3146
* {@inheritdoc}
3247
*/
3348
public function get(string $key, callable $callback, float $beta = null)
3449
{
35-
if (0 > $beta) {
36-
throw new InvalidArgumentException(sprintf('Argument "$beta" provided to "%s::get()" must be a positive number, %f given.', \get_class($this), $beta));
37-
}
38-
39-
return $this->doGet($this, $key, $callback, $beta ?? 1.0);
50+
return $this->doGet($this, $key, $callback, $beta);
4051
}
4152

42-
private function doGet(CacheItemPoolInterface $pool, string $key, callable $callback, float $beta)
53+
private function doGet(AdapterInterface $pool, string $key, callable $callback, ?float $beta)
4354
{
44-
retry:
55+
if (0 > $beta = $beta ?? 1.0) {
56+
throw new InvalidArgumentException(sprintf('Argument "$beta" provided to "%s::get()" must be a positive number, %f given.', \get_class($this), $beta));
57+
}
58+
4559
$t = 0;
4660
$item = $pool->getItem($key);
4761
$recompute = !$item->isHit() || INF === $beta;
4862

49-
if ($item instanceof CacheItem && 0 < $beta) {
63+
if (0 < $beta) {
5064
if ($recompute) {
5165
$t = microtime(true);
5266
} else {
5367
$metadata = $item->getMetadata();
54-
$expiry = $metadata[CacheItem::METADATA_EXPIRY] ?? false;
55-
$ctime = $metadata[CacheItem::METADATA_CTIME] ?? false;
68+
$expiry = $metadata[ItemInterface::METADATA_EXPIRY] ?? false;
69+
$ctime = $metadata[ItemInterface::METADATA_CTIME] ?? false;
5670

5771
if ($ctime && $expiry) {
5872
$t = microtime(true);
@@ -69,11 +83,32 @@ private function doGet(CacheItemPoolInterface $pool, string $key, callable $call
6983
return $item->get();
7084
}
7185

72-
if (!LockRegistry::save($key, $pool, $item, $callback, $t, $value)) {
73-
$beta = 0;
74-
goto retry;
86+
static $save;
87+
88+
$save = $save ?? \Closure::bind(
89+
function (AdapterInterface $pool, ItemInterface $item, $value, float $startTime) {
90+
if ($startTime && $item->expiry > $endTime = microtime(true)) {
91+
$item->newMetadata[ItemInterface::METADATA_EXPIRY] = $item->expiry;
92+
$item->newMetadata[ItemInterface::METADATA_CTIME] = 1000 * (int) ($endTime - $startTime);
93+
}
94+
$pool->save($item->set($value));
95+
96+
return $value;
97+
},
98+
null,
99+
CacheItem::class
100+
);
101+
102+
// don't wrap nor save recursive calls
103+
if (null === $callbackWrapper = $this->callbackWrapper) {
104+
return $callback($item);
75105
}
106+
$this->callbackWrapper = null;
76107

77-
return $value;
108+
try {
109+
return $save($pool, $item, $callbackWrapper($item, $callback, $pool), $t);
110+
} finally {
111+
$this->callbackWrapper = $callbackWrapper;
112+
}
78113
}
79114
}

0 commit comments

Comments
 (0)