Skip to content

Commit 1f65e78

Browse files
nicolas-grekasnicolas-grekas
committed
minor symfony#40317 [Cache] boost perf by wrapping keys validity checks with assert() (nicolas-grekas)
This PR was merged into the 5.3-dev branch. Discussion ---------- [Cache] boost perf by wrapping keys validity checks with `assert()` | Q | A | ------------- | --- | Branch? | 5.x | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - PSR-6 has one perf hog: checking the validity of keys. But in practice, an invalid key should never happen in production: encoding/cleanup is a must-have, and it's a step that should be identified *during dev*. That's why I think we're safe wrapping these checks with `assert()`. On an `ArrayAdapter`, this doubles the throughput of the pool when getting items. I didn't use `assert()` in constructors when not on the hot path. This PR also makes some callable properties static, as they should be from the beginning. Commits ------- 8f03a1f [Cache] boost perf by wrapping keys validity checks with `assert()`
2 parents 6dd2d7b + 8f03a1f commit 1f65e78

15 files changed

+145
-93
lines changed

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

+5-6
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,11 @@ abstract class AbstractAdapter implements AdapterInterface, CacheInterface, Logg
3939
protected function __construct(string $namespace = '', int $defaultLifetime = 0)
4040
{
4141
$this->namespace = '' === $namespace ? '' : CacheItem::validateKey($namespace).static::NS_SEPARATOR;
42+
$this->defaultLifetime = $defaultLifetime;
4243
if (null !== $this->maxIdLength && \strlen($namespace) > $this->maxIdLength - 24) {
4344
throw new InvalidArgumentException(sprintf('Namespace must be %d chars max, %d given ("%s").', $this->maxIdLength - 24, \strlen($namespace), $namespace));
4445
}
45-
$this->createCacheItem = \Closure::bind(
46+
self::$createCacheItem ?? self::$createCacheItem = \Closure::bind(
4647
static function ($key, $value, $isHit) {
4748
$item = new CacheItem();
4849
$item->key = $key;
@@ -63,9 +64,8 @@ static function ($key, $value, $isHit) {
6364
null,
6465
CacheItem::class
6566
);
66-
$getId = \Closure::fromCallable([$this, 'getId']);
67-
$this->mergeByLifetime = \Closure::bind(
68-
static function ($deferred, $namespace, &$expiredIds) use ($getId, $defaultLifetime) {
67+
self::$mergeByLifetime ?? self::$mergeByLifetime = \Closure::bind(
68+
static function ($deferred, $namespace, &$expiredIds, $getId, $defaultLifetime) {
6969
$byLifetime = [];
7070
$now = microtime(true);
7171
$expiredIds = [];
@@ -147,8 +147,7 @@ public static function createConnection(string $dsn, array $options = [])
147147
public function commit()
148148
{
149149
$ok = true;
150-
$byLifetime = $this->mergeByLifetime;
151-
$byLifetime = $byLifetime($this->deferred, $this->namespace, $expiredIds);
150+
$byLifetime = (self::$mergeByLifetime)($this->deferred, $this->namespace, $expiredIds, \Closure::fromCallable([$this, 'getId']), $this->defaultLifetime);
152151
$retry = $this->deferred = [];
153152

154153
if ($expiredIds) {

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

+5-7
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,11 @@ abstract class AbstractTagAwareAdapter implements TagAwareAdapterInterface, TagA
4040
protected function __construct(string $namespace = '', int $defaultLifetime = 0)
4141
{
4242
$this->namespace = '' === $namespace ? '' : CacheItem::validateKey($namespace).':';
43+
$this->defaultLifetime = $defaultLifetime;
4344
if (null !== $this->maxIdLength && \strlen($namespace) > $this->maxIdLength - 24) {
4445
throw new InvalidArgumentException(sprintf('Namespace must be %d chars max, %d given ("%s").', $this->maxIdLength - 24, \strlen($namespace), $namespace));
4546
}
46-
$this->createCacheItem = \Closure::bind(
47+
self::$createCacheItem ?? self::$createCacheItem = \Closure::bind(
4748
static function ($key, $value, $isHit) {
4849
$item = new CacheItem();
4950
$item->key = $key;
@@ -68,10 +69,8 @@ static function ($key, $value, $isHit) {
6869
null,
6970
CacheItem::class
7071
);
71-
$getId = \Closure::fromCallable([$this, 'getId']);
72-
$tagPrefix = self::TAGS_PREFIX;
73-
$this->mergeByLifetime = \Closure::bind(
74-
static function ($deferred, &$expiredIds) use ($getId, $tagPrefix, $defaultLifetime) {
72+
self::$mergeByLifetime ?? self::$mergeByLifetime = \Closure::bind(
73+
static function ($deferred, &$expiredIds, $getId, $tagPrefix, $defaultLifetime) {
7574
$byLifetime = [];
7675
$now = microtime(true);
7776
$expiredIds = [];
@@ -175,8 +174,7 @@ protected function doDeleteYieldTags(array $ids): iterable
175174
public function commit(): bool
176175
{
177176
$ok = true;
178-
$byLifetime = $this->mergeByLifetime;
179-
$byLifetime = $byLifetime($this->deferred, $expiredIds);
177+
$byLifetime = (self::$mergeByLifetime)($this->deferred, $expiredIds, \Closure::fromCallable([$this, 'getId']), self::TAGS_PREFIX, $this->defaultLifetime);
180178
$retry = $this->deferred = [];
181179

182180
if ($expiredIds) {

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

+19-14
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,12 @@ class ArrayAdapter implements AdapterInterface, CacheInterface, LoggerAwareInter
3333
private $storeSerialized;
3434
private $values = [];
3535
private $expiries = [];
36-
private $createCacheItem;
3736
private $defaultLifetime;
3837
private $maxLifetime;
3938
private $maxItems;
4039

40+
private static $createCacheItem;
41+
4142
/**
4243
* @param bool $storeSerialized Disabling serialization can lead to cache corruptions when storing mutable values but increases performance otherwise
4344
*/
@@ -55,7 +56,7 @@ public function __construct(int $defaultLifetime = 0, bool $storeSerialized = tr
5556
$this->storeSerialized = $storeSerialized;
5657
$this->maxLifetime = $maxLifetime;
5758
$this->maxItems = $maxItems;
58-
$this->createCacheItem = \Closure::bind(
59+
self::$createCacheItem ?? self::$createCacheItem = \Closure::bind(
5960
static function ($key, $value, $isHit) {
6061
$item = new CacheItem();
6162
$item->key = $key;
@@ -111,7 +112,7 @@ public function hasItem($key)
111112

112113
return true;
113114
}
114-
CacheItem::validateKey($key);
115+
\assert('' !== CacheItem::validateKey($key));
115116

116117
return isset($this->expiries[$key]) && !$this->deleteItem($key);
117118
}
@@ -131,23 +132,18 @@ public function getItem($key)
131132
} else {
132133
$value = $this->storeSerialized ? $this->unfreeze($key, $isHit) : $this->values[$key];
133134
}
134-
$f = $this->createCacheItem;
135135

136-
return $f($key, $value, $isHit);
136+
return (self::$createCacheItem)($key, $value, $isHit);
137137
}
138138

139139
/**
140140
* {@inheritdoc}
141141
*/
142142
public function getItems(array $keys = [])
143143
{
144-
foreach ($keys as $key) {
145-
if (!\is_string($key) || !isset($this->expiries[$key])) {
146-
CacheItem::validateKey($key);
147-
}
148-
}
144+
\assert(self::validateKeys($keys));
149145

150-
return $this->generateItems($keys, microtime(true), $this->createCacheItem);
146+
return $this->generateItems($keys, microtime(true), self::$createCacheItem);
151147
}
152148

153149
/**
@@ -157,9 +153,7 @@ public function getItems(array $keys = [])
157153
*/
158154
public function deleteItem($key)
159155
{
160-
if (!\is_string($key) || !isset($this->expiries[$key])) {
161-
CacheItem::validateKey($key);
162-
}
156+
\assert('' !== CacheItem::validateKey($key));
163157
unset($this->values[$key], $this->expiries[$key]);
164158

165159
return true;
@@ -395,4 +389,15 @@ private function unfreeze(string $key, bool &$isHit)
395389

396390
return $value;
397391
}
392+
393+
private function validateKeys(array $keys): bool
394+
{
395+
foreach ($keys as $key) {
396+
if (!\is_string($key) || !isset($this->expiries[$key])) {
397+
CacheItem::validateKey($key);
398+
}
399+
}
400+
401+
return true;
402+
}
398403
}

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

+11-8
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,9 @@ class ChainAdapter implements AdapterInterface, CacheInterface, PruneableInterfa
3535

3636
private $adapters = [];
3737
private $adapterCount;
38-
private $syncItem;
38+
private $defaultLifetime;
39+
40+
private static $syncItem;
3941

4042
/**
4143
* @param CacheItemPoolInterface[] $adapters The ordered list of adapters used to fetch cached items
@@ -62,9 +64,10 @@ public function __construct(array $adapters, int $defaultLifetime = 0)
6264
}
6365
}
6466
$this->adapterCount = \count($this->adapters);
67+
$this->defaultLifetime = $defaultLifetime;
6568

66-
$this->syncItem = \Closure::bind(
67-
static function ($sourceItem, $item, $sourceMetadata = null) use ($defaultLifetime) {
69+
self::$syncItem ?? self::$syncItem = \Closure::bind(
70+
static function ($sourceItem, $item, $defaultLifetime, $sourceMetadata = null) {
6871
$sourceItem->isTaggable = false;
6972
$sourceMetadata = $sourceMetadata ?? $sourceItem->metadata;
7073
unset($sourceMetadata[CacheItem::METADATA_TAGS]);
@@ -105,7 +108,7 @@ public function get(string $key, callable $callback, float $beta = null, array &
105108
$value = $this->doGet($adapter, $key, $callback, $beta, $metadata);
106109
}
107110
if (null !== $item) {
108-
($this->syncItem)($lastItem = $lastItem ?? $item, $item, $metadata);
111+
(self::$syncItem)($lastItem = $lastItem ?? $item, $item, $this->defaultLifetime, $metadata);
109112
}
110113

111114
return $value;
@@ -119,15 +122,15 @@ public function get(string $key, callable $callback, float $beta = null, array &
119122
*/
120123
public function getItem($key)
121124
{
122-
$syncItem = $this->syncItem;
125+
$syncItem = self::$syncItem;
123126
$misses = [];
124127

125128
foreach ($this->adapters as $i => $adapter) {
126129
$item = $adapter->getItem($key);
127130

128131
if ($item->isHit()) {
129132
while (0 <= --$i) {
130-
$this->adapters[$i]->save($syncItem($item, $misses[$i]));
133+
$this->adapters[$i]->save($syncItem($item, $misses[$i], $this->defaultLifetime));
131134
}
132135

133136
return $item;
@@ -164,13 +167,13 @@ private function generateItems(iterable $items, int $adapterIndex)
164167
}
165168

166169
if ($missing) {
167-
$syncItem = $this->syncItem;
170+
$syncItem = self::$syncItem;
168171
$adapter = $this->adapters[$adapterIndex];
169172
$items = $this->generateItems($nextAdapter->getItems($missing), $nextAdapterIndex);
170173

171174
foreach ($items as $k => $item) {
172175
if ($item->isHit()) {
173-
$adapter->save($syncItem($item, $misses[$k]));
176+
$adapter->save($syncItem($item, $misses[$k], $this->defaultLifetime));
174177
}
175178

176179
yield $k => $item;

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

+7-9
Original file line numberDiff line numberDiff line change
@@ -20,19 +20,19 @@
2020
*/
2121
class NullAdapter implements AdapterInterface, CacheInterface
2222
{
23-
private $createCacheItem;
23+
private static $createCacheItem;
2424

2525
public function __construct()
2626
{
27-
$this->createCacheItem = \Closure::bind(
28-
function ($key) {
27+
self::$createCacheItem ?? self::$createCacheItem = \Closure::bind(
28+
static function ($key) {
2929
$item = new CacheItem();
3030
$item->key = $key;
3131
$item->isHit = false;
3232

3333
return $item;
3434
},
35-
$this,
35+
null,
3636
CacheItem::class
3737
);
3838
}
@@ -44,17 +44,15 @@ public function get(string $key, callable $callback, float $beta = null, array &
4444
{
4545
$save = true;
4646

47-
return $callback(($this->createCacheItem)($key), $save);
47+
return $callback((self::$createCacheItem)($key), $save);
4848
}
4949

5050
/**
5151
* {@inheritdoc}
5252
*/
5353
public function getItem($key)
5454
{
55-
$f = $this->createCacheItem;
56-
57-
return $f($key);
55+
return (self::$createCacheItem)($key);
5856
}
5957

6058
/**
@@ -145,7 +143,7 @@ public function delete(string $key): bool
145143

146144
private function generateItems(array $keys)
147145
{
148-
$f = $this->createCacheItem;
146+
$f = self::$createCacheItem;
149147

150148
foreach ($keys as $key) {
151149
yield $key => $f($key);

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

+4-6
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@ class PhpArrayAdapter implements AdapterInterface, CacheInterface, PruneableInte
3737
private $file;
3838
private $keys;
3939
private $values;
40-
private $createCacheItem;
4140

41+
private static $createCacheItem;
4242
private static $valuesCache = [];
4343

4444
/**
@@ -49,7 +49,7 @@ public function __construct(string $file, AdapterInterface $fallbackPool)
4949
{
5050
$this->file = $file;
5151
$this->pool = $fallbackPool;
52-
$this->createCacheItem = \Closure::bind(
52+
self::$createCacheItem ?? self::$createCacheItem = \Closure::bind(
5353
static function ($key, $value, $isHit) {
5454
$item = new CacheItem();
5555
$item->key = $key;
@@ -142,9 +142,7 @@ public function getItem($key)
142142
}
143143
}
144144

145-
$f = $this->createCacheItem;
146-
147-
return $f($key, $value, $isHit);
145+
return (self::$createCacheItem)($key, $value, $isHit);
148146
}
149147

150148
/**
@@ -407,7 +405,7 @@ private function initialize()
407405

408406
private function generateItems(array $keys): \Generator
409407
{
410-
$f = $this->createCacheItem;
408+
$f = self::$createCacheItem;
411409
$fallbackKeys = [];
412410

413411
foreach ($keys as $key) {

0 commit comments

Comments
 (0)