feature #27549 [Cache] Unconditionally use PhpFilesAdapter for system…

… pools (nicolas-grekas) This PR was merged into the 4.2-dev branch. Discussion ---------- [Cache] Unconditionally use PhpFilesAdapter for system pools | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Now that we're about to leverage OPCache shared memory even for objects (see #27543), there's no reason anymore to use APCu for system caches. Let's remove some complexity here. As a bonus, this makes system caches pruneable using the `cache:pool:prune` command. Commits ------- 51381e5 [Cache] Unconditionally use PhpFilesAdapter for system pools
symfony · Jun 11, 2018 · d4f5d46 · d4f5d46
2 parents 7e3b7b0 + 51381e5
commit d4f5d46
Show file tree

Hide file tree

Showing 15 changed files with 51 additions and 58 deletions.
diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
@@ -1538,7 +1538,6 @@ private function registerCacheConfiguration(array $config, ContainerBuilder $con
     {
         $version = new Parameter('container.build_id');
         $container->getDefinition('cache.adapter.apcu')->replaceArgument(2, $version);
-        $container->getDefinition('cache.adapter.system')->replaceArgument(2, $version);
         $container->getDefinition('cache.adapter.filesystem')->replaceArgument(2, $config['directory']);
 
         if (isset($config['prefix_seed'])) {

diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/cache.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/cache.xml
@@ -35,15 +35,15 @@
             <tag name="cache.pool" />
         </service>
 
-        <service id="cache.adapter.system" class="Symfony\Component\Cache\Adapter\AdapterInterface" abstract="true">
-            <factory class="Symfony\Component\Cache\Adapter\AbstractAdapter" method="createSystemCache" />
+        <service id="cache.adapter.system" class="Symfony\Component\Cache\Adapter\PhpFilesAdapter" abstract="true">
             <tag name="cache.pool" clearer="cache.system_clearer" />
             <tag name="monolog.logger" channel="cache" />
             <argument /> <!-- namespace -->
             <argument>0</argument> <!-- default lifetime -->
-            <argument /> <!-- version -->
             <argument>%kernel.cache_dir%/pools</argument>
-            <argument type="service" id="logger" on-invalid="ignore" />
+            <call method="setLogger">
+                <argument type="service" id="logger" on-invalid="ignore" />
+            </call>
         </service>
 
         <service id="cache.adapter.apcu" class="Symfony\Component\Cache\Adapter\ApcuAdapter" abstract="true">

diff --git a/src/Symfony/Component/Cache/Adapter/AbstractAdapter.php b/src/Symfony/Component/Cache/Adapter/AbstractAdapter.php
@@ -102,9 +102,13 @@ function ($deferred, $namespace, &$expiredIds) use ($getId) {
      * @param LoggerInterface|null $logger
      *
      * @return AdapterInterface
+     *
+     * @deprecated since Symfony 4.2
      */
     public static function createSystemCache($namespace, $defaultLifetime, $version, $directory, LoggerInterface $logger = null)
     {
+        @trigger_error(sprintf('The "%s()" method is deprecated since Symfony 4.2.', __CLASS__), E_USER_DEPRECATED);
+
         if (null === self::$apcuSupported) {
             self::$apcuSupported = ApcuAdapter::isSupported();
         }

diff --git a/src/Symfony/Component/Cache/Adapter/PhpArrayAdapter.php b/src/Symfony/Component/Cache/Adapter/PhpArrayAdapter.php
@@ -43,7 +43,6 @@ public function __construct(string $file, AdapterInterface $fallbackPool)
     {
         $this->file = $file;
         $this->pool = $fallbackPool;
-        $this->zendDetectUnicode = ini_get('zend.detect_unicode');
         $this->createCacheItem = \Closure::bind(
             function ($key, $value, $isHit) {
                 $item = new CacheItem();

diff --git a/src/Symfony/Component/Cache/Adapter/PhpFilesAdapter.php b/src/Symfony/Component/Cache/Adapter/PhpFilesAdapter.php
@@ -24,14 +24,11 @@ class PhpFilesAdapter extends AbstractAdapter implements PruneableInterface
      */
     public function __construct(string $namespace = '', int $defaultLifetime = 0, string $directory = null)
     {
-        if (!static::isSupported()) {
-            throw new CacheException('OPcache is not enabled');
-        }
+        self::$startTime = self::$startTime ?? $_SERVER['REQUEST_TIME'] ?? time();
         parent::__construct('', $defaultLifetime);
         $this->init($namespace, $directory);
 
         $e = new \Exception();
         $this->includeHandler = function () use ($e) { throw $e; };
-        $this->zendDetectUnicode = ini_get('zend.detect_unicode');
     }
 }
diff --git a/src/Symfony/Component/Cache/CHANGELOG.md b/src/Symfony/Component/Cache/CHANGELOG.md
@@ -7,6 +7,7 @@ CHANGELOG
  * added `CacheInterface`, which provides stampede protection via probabilistic early expiration and should become the preferred way to use a cache
  * throw `LogicException` when `CacheItem::tag()` is called on an item coming from a non tag-aware pool
  * deprecated `CacheItem::getPreviousTags()`, use `CacheItem::getMetadata()` instead
+ * deprecated the `AbstractAdapter::createSystemCache()` method
 
 3.4.0
 -----

diff --git a/src/Symfony/Component/Cache/Simple/PhpArrayCache.php b/src/Symfony/Component/Cache/Simple/PhpArrayCache.php
@@ -36,7 +36,6 @@ public function __construct(string $file, CacheInterface $fallbackPool)
     {
         $this->file = $file;
         $this->pool = $fallbackPool;
-        $this->zendDetectUnicode = ini_get('zend.detect_unicode');
     }
 
     /**

diff --git a/src/Symfony/Component/Cache/Simple/PhpFilesCache.php b/src/Symfony/Component/Cache/Simple/PhpFilesCache.php
@@ -24,14 +24,11 @@ class PhpFilesCache extends AbstractCache implements PruneableInterface
      */
     public function __construct(string $namespace = '', int $defaultLifetime = 0, string $directory = null)
     {
-        if (!static::isSupported()) {
-            throw new CacheException('OPcache is not enabled');
-        }
+        self::$startTime = self::$startTime ?? $_SERVER['REQUEST_TIME'] ?? time();
         parent::__construct('', $defaultLifetime);
         $this->init($namespace, $directory);
 
         $e = new \Exception();
         $this->includeHandler = function () use ($e) { throw $e; };
-        $this->zendDetectUnicode = ini_get('zend.detect_unicode');
     }
 }
diff --git a/src/Symfony/Component/Cache/Tests/Adapter/MaxIdLengthAdapterTest.php b/src/Symfony/Component/Cache/Tests/Adapter/MaxIdLengthAdapterTest.php
@@ -26,7 +26,7 @@ public function testLongKey()
         $cache->expects($this->exactly(2))
             ->method('doHave')
             ->withConsecutive(
-                array($this->equalTo('----------:0GTYWa9n4ed8vqNlOT2iEr:')),
+                array($this->equalTo('----------:nWfzGiCgLczv3SSUzXL3kg:')),
                 array($this->equalTo('----------:---------------------------------------'))
             );
 

diff --git a/src/Symfony/Component/Cache/Tests/Adapter/PhpFilesAdapterTest.php b/src/Symfony/Component/Cache/Tests/Adapter/PhpFilesAdapterTest.php
@@ -25,10 +25,6 @@ class PhpFilesAdapterTest extends AdapterTestCase
 
     public function createCachePool()
     {
-        if (!PhpFilesAdapter::isSupported()) {
-            $this->markTestSkipped('OPcache extension is not enabled.');
-        }
-
         return new PhpFilesAdapter('sf-cache');
     }
 

diff --git a/src/Symfony/Component/Cache/Tests/Simple/PhpFilesCacheTest.php b/src/Symfony/Component/Cache/Tests/Simple/PhpFilesCacheTest.php
@@ -25,10 +25,6 @@ class PhpFilesCacheTest extends CacheTestCase
 
     public function createSimpleCache()
     {
-        if (!PhpFilesCache::isSupported()) {
-            $this->markTestSkipped('OPcache extension is not enabled.');
-        }
-
         return new PhpFilesCache('sf-cache');
     }
 

diff --git a/src/Symfony/Component/Cache/Traits/AbstractTrait.php b/src/Symfony/Component/Cache/Traits/AbstractTrait.php
@@ -27,6 +27,7 @@ trait AbstractTrait
     private $namespaceVersion = '';
     private $versioningIsEnabled = false;
     private $deferred = array();
+    private $ids = array();
 
     /**
      * @var int|null The maximum length to enforce for identifiers or null when no limit applies
@@ -198,6 +199,7 @@ public function reset()
             $this->commit();
         }
         $this->namespaceVersion = '';
+        $this->ids = array();
     }
 
     /**
@@ -229,20 +231,26 @@ protected static function unserialize($value)
 
     private function getId($key)
     {
-        CacheItem::validateKey($key);
-
         if ($this->versioningIsEnabled && '' === $this->namespaceVersion) {
             $this->namespaceVersion = '1:';
             foreach ($this->doFetch(array('@'.$this->namespace)) as $v) {
                 $this->namespaceVersion = $v;
             }
         }
 
+        if (\is_string($key) && isset($this->ids[$key])) {
+            return $this->namespace.$this->namespaceVersion.$this->ids[$key];
+        }
+        CacheItem::validateKey($key);
+        $this->ids[$key] = $key;
+
         if (null === $this->maxIdLength) {
             return $this->namespace.$this->namespaceVersion.$key;
         }
         if (\strlen($id = $this->namespace.$this->namespaceVersion.$key) > $this->maxIdLength) {
-            $id = $this->namespace.$this->namespaceVersion.substr_replace(base64_encode(hash('sha256', $key, true)), ':', -22);
+            // Use MD5 to favor speed over security, which is not an issue here
+            $this->ids[$key] = $id = substr_replace(base64_encode(hash('md5', $key, true)), ':', -2);
+            $id = $this->namespace.$this->namespaceVersion.$id;
         }
 
         return $id;

diff --git a/src/Symfony/Component/Cache/Traits/FilesystemCommonTrait.php b/src/Symfony/Component/Cache/Traits/FilesystemCommonTrait.php
@@ -56,7 +56,7 @@ protected function doClear($namespace)
         $ok = true;
 
         foreach (new \RecursiveIteratorIterator(new \RecursiveDirectoryIterator($this->directory, \FilesystemIterator::SKIP_DOTS)) as $file) {
-            $ok = ($file->isDir() || @unlink($file) || !file_exists($file)) && $ok;
+            $ok = ($file->isDir() || $this->doUnlink($file) || !file_exists($file)) && $ok;
         }
 
         return $ok;
@@ -71,12 +71,17 @@ protected function doDelete(array $ids)
 
         foreach ($ids as $id) {
             $file = $this->getFile($id);
-            $ok = (!file_exists($file) || @unlink($file) || !file_exists($file)) && $ok;
+            $ok = (!file_exists($file) || $this->doUnlink($file) || !file_exists($file)) && $ok;
         }
 
         return $ok;
     }
 
+    protected function doUnlink($file)
+    {
+        return @unlink($file);
+    }
+
     private function write($file, $data, $expiresAt = null)
     {
         set_error_handler(__CLASS__.'::throwError');
@@ -98,7 +103,8 @@ private function write($file, $data, $expiresAt = null)
 
     private function getFile($id, $mkdir = false)
     {
-        $hash = str_replace('/', '-', base64_encode(hash('sha256', static::class.$id, true)));
+        // Use MD5 to favor speed over security, which is not an issue here
+        $hash = str_replace('/', '-', base64_encode(hash('md5', static::class.$id, true)));
         $dir = $this->directory.strtoupper($hash[0].DIRECTORY_SEPARATOR.$hash[1].DIRECTORY_SEPARATOR);
 
         if ($mkdir && !file_exists($dir)) {

diff --git a/src/Symfony/Component/Cache/Traits/PhpArrayTrait.php b/src/Symfony/Component/Cache/Traits/PhpArrayTrait.php
@@ -26,7 +26,6 @@ trait PhpArrayTrait
 
     private $file;
     private $values;
-    private $zendDetectUnicode;
 
     /**
      * Store an array of cached values.
@@ -98,7 +97,6 @@ public function warmUp(array $values)
         }
 
         $dump .= "\n);\n";
-        $dump = str_replace("' . \"\\0\" . '", "\0", $dump);
 
         $tmpFile = uniqid($this->file, true);
 
@@ -128,15 +126,6 @@ public function clear()
      */
     private function initialize()
     {
-        if ($this->zendDetectUnicode) {
-            $zmb = ini_set('zend.detect_unicode', 0);
-        }
-        try {
-            $this->values = file_exists($this->file) ? (include $this->file ?: array()) : array();
-        } finally {
-            if ($this->zendDetectUnicode) {
-                ini_set('zend.detect_unicode', $zmb);
-            }
-        }
+        $this->values = file_exists($this->file) ? (include $this->file ?: array()) : array();
     }
 }
diff --git a/src/Symfony/Component/Cache/Traits/PhpFilesTrait.php b/src/Symfony/Component/Cache/Traits/PhpFilesTrait.php
@@ -26,11 +26,14 @@ trait PhpFilesTrait
     use FilesystemCommonTrait;
 
     private $includeHandler;
-    private $zendDetectUnicode;
+
+    private static $startTime;
 
     public static function isSupported()
     {
-        return function_exists('opcache_invalidate') && ini_get('opcache.enable');
+        self::$startTime = self::$startTime ?? $_SERVER['REQUEST_TIME'] ?? time();
+
+        return \function_exists('opcache_invalidate') && ini_get('opcache.enable') && ('cli' !== \PHP_SAPI || ini_get('opcache.enable_cli'));
     }
 
     /**
@@ -40,19 +43,14 @@ public function prune()
     {
         $time = time();
         $pruned = true;
-        $allowCompile = 'cli' !== PHP_SAPI || ini_get('opcache.enable_cli');
 
         set_error_handler($this->includeHandler);
         try {
             foreach (new \RecursiveIteratorIterator(new \RecursiveDirectoryIterator($this->directory, \FilesystemIterator::SKIP_DOTS), \RecursiveIteratorIterator::LEAVES_ONLY) as $file) {
                 list($expiresAt) = include $file;
 
                 if ($time >= $expiresAt) {
-                    $pruned = @unlink($file) && !file_exists($file) && $pruned;
-
-                    if ($allowCompile) {
-                        @opcache_invalidate($file, true);
-                    }
+                    $pruned = $this->doUnlink($file) && !file_exists($file) && $pruned;
                 }
             }
         } finally {
@@ -70,9 +68,6 @@ protected function doFetch(array $ids)
         $values = array();
         $now = time();
 
-        if ($this->zendDetectUnicode) {
-            $zmb = ini_set('zend.detect_unicode', 0);
-        }
         set_error_handler($this->includeHandler);
         try {
             foreach ($ids as $id) {
@@ -88,9 +83,6 @@ protected function doFetch(array $ids)
             }
         } finally {
             restore_error_handler();
-            if ($this->zendDetectUnicode) {
-                ini_set('zend.detect_unicode', $zmb);
-            }
         }
 
         foreach ($values as $id => $value) {
@@ -119,7 +111,7 @@ protected function doSave(array $values, $lifetime)
     {
         $ok = true;
         $data = array($lifetime ? time() + $lifetime : PHP_INT_MAX, '');
-        $allowCompile = 'cli' !== PHP_SAPI || ini_get('opcache.enable_cli');
+        $allowCompile = self::isSupported();
 
         foreach ($values as $key => $value) {
             if (null === $value || \is_object($value)) {
@@ -142,7 +134,8 @@ protected function doSave(array $values, $lifetime)
 
             $data[1] = $value;
             $file = $this->getFile($key, true);
-            $ok = $this->write($file, '<?php return '.var_export($data, true).';') && $ok;
+            // Since OPcache only compiles files older than the script execution start, set the file's mtime in the past
+            $ok = $this->write($file, '<?php return '.var_export($data, true).';', self::$startTime - 10) && $ok;
 
             if ($allowCompile) {
                 @opcache_invalidate($file, true);
@@ -155,4 +148,13 @@ protected function doSave(array $values, $lifetime)
 
         return $ok;
     }
+
+    protected function doUnlink($file)
+    {
+        if (self::isSupported()) {
+            @opcache_invalidate($file, true);
+        }
+
+        return @unlink($file);
+    }
 }