feature symfony#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

Author: nicolas-grekas

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

@@ -276,13 +276,6 @@ public function saveDeferred(CacheItemInterface $item)
         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;

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

@@ -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 @@ function ($key, $value, $isHit) use ($defaultLifetime) {
      */
     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 @@ public function save(CacheItemInterface $item)
         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 @@ private function generateItems(array $keys)
         $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);
         }
     }
 }

src/Symfony/Component/Cache/CacheItem.php

@@ -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 @@ public function expiresAfter($time)
      *
      * @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);