feature #17654 [Cache] Don't clone, serialize (nicolas-grekas)
This PR was merged into the 3.1-dev branch.
Discussion
----------
[Cache] Don't clone, serialize
| Q | A
| ------------- | ---
| Bug fix? | no
| New feature? | yes
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | -
| License | MIT
| Doc PR | -
Cloning is not required by PSR-6, adds overhead and doesn't prevent any mistake for nested objects. Let's simplify.
ArrayCache in turn should store serialized by default, so that data is deep-cloned. But since this can cause a perf issue for some, I propose to add a constructor flag to disable serializing. WDYT?
Commits
-------
8605417
[Cache] Don't clone, serialize
This commit is contained in:
commit
b33eb05b7c
@ -276,13 +276,6 @@ abstract class AbstractAdapter implements CacheItemPoolInterface, LoggerAwareInt
|
||||
if (!$item instanceof CacheItem) {
|
||||
return false;
|
||||
}
|
||||
try {
|
||||
$item = clone $item;
|
||||
} catch (\Exception $e) {
|
||||
$value = $item->get();
|
||||
$type = is_object($value) ? get_class($value) : gettype($value);
|
||||
CacheItem::log($this->logger, 'Failed to clone key "{key}" ({type})', array('key' => $key, 'type' => $type, 'exception' => $e));
|
||||
}
|
||||
$this->deferred[$item->getKey()] = $item;
|
||||
|
||||
return true;
|
||||
|
@ -25,12 +25,18 @@ class ArrayAdapter implements CacheItemPoolInterface, LoggerAwareInterface
|
||||
{
|
||||
use LoggerAwareTrait;
|
||||
|
||||
private $storeSerialized;
|
||||
private $values = array();
|
||||
private $expiries = array();
|
||||
private $createCacheItem;
|
||||
|
||||
public function __construct($defaultLifetime = 0)
|
||||
/**
|
||||
* @param int $defaultLifetime
|
||||
* @param bool $storeSerialized Disabling serialization can lead to cache corruptions when storing mutable values but increases performance otherwise.
|
||||
*/
|
||||
public function __construct($defaultLifetime = 0, $storeSerialized = true)
|
||||
{
|
||||
$this->storeSerialized = $storeSerialized;
|
||||
$this->createCacheItem = \Closure::bind(
|
||||
function ($key, $value, $isHit) use ($defaultLifetime) {
|
||||
$item = new CacheItem();
|
||||
@ -51,10 +57,16 @@ class ArrayAdapter implements CacheItemPoolInterface, LoggerAwareInterface
|
||||
*/
|
||||
public function getItem($key)
|
||||
{
|
||||
if (!$isHit = $this->hasItem($key)) {
|
||||
$value = null;
|
||||
} elseif ($this->storeSerialized) {
|
||||
$value = unserialize($this->values[$key]);
|
||||
} else {
|
||||
$value = $this->values[$key];
|
||||
}
|
||||
$f = $this->createCacheItem;
|
||||
$isHit = $this->hasItem($key);
|
||||
|
||||
return $f($key, $isHit ? $this->values[$key] : null, $isHit);
|
||||
return $f($key, $value, $isHit);
|
||||
}
|
||||
|
||||
/**
|
||||
@ -125,13 +137,12 @@ class ArrayAdapter implements CacheItemPoolInterface, LoggerAwareInterface
|
||||
if (0 > $lifetime) {
|
||||
return true;
|
||||
}
|
||||
|
||||
if (is_object($value)) {
|
||||
if ($this->storeSerialized) {
|
||||
try {
|
||||
$value = clone $value;
|
||||
$value = serialize($value);
|
||||
} catch (\Exception $e) {
|
||||
$type = is_object($value) ? get_class($value) : gettype($value);
|
||||
CacheItem::log($this->logger, 'Failed to clone key "{key}" ({type})', array('key' => $key, 'type' => $type, 'exception' => $e));
|
||||
CacheItem::log($this->logger, 'Failed to save key "{key}" ({type})', array('key' => $key, 'type' => $type, 'exception' => $e));
|
||||
|
||||
return false;
|
||||
}
|
||||
@ -179,9 +190,15 @@ class ArrayAdapter implements CacheItemPoolInterface, LoggerAwareInterface
|
||||
$f = $this->createCacheItem;
|
||||
|
||||
foreach ($keys as $key) {
|
||||
$isHit = isset($this->expiries[$key]) && ($this->expiries[$key] >= time() || !$this->deleteItem($key));
|
||||
if (!$isHit = isset($this->expiries[$key]) && ($this->expiries[$key] >= time() || !$this->deleteItem($key))) {
|
||||
$value = null;
|
||||
} elseif ($this->storeSerialized) {
|
||||
$value = unserialize($this->values[$key]);
|
||||
} else {
|
||||
$value = $this->values[$key];
|
||||
}
|
||||
|
||||
yield $key => $f($key, $isHit ? $this->values[$key] : null, $isHit);
|
||||
yield $key => $f($key, $value, $isHit);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -31,13 +31,6 @@ final class CacheItem implements CacheItemInterface
|
||||
private $lifetime;
|
||||
private $defaultLifetime;
|
||||
|
||||
public function __clone()
|
||||
{
|
||||
if (is_object($this->value)) {
|
||||
$this->value = clone $this->value;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* {@inheritdoc}
|
||||
*/
|
||||
@ -112,7 +105,7 @@ final class CacheItem implements CacheItemInterface
|
||||
*
|
||||
* @internal
|
||||
*/
|
||||
public function log(LoggerInterface $logger = null, $message, $context = array())
|
||||
public static function log(LoggerInterface $logger = null, $message, $context = array())
|
||||
{
|
||||
if ($logger) {
|
||||
$logger->warning($message, $context);
|
||||
|
Reference in New Issue
Block a user