Skip to content

Commit

Permalink
[TASK] Streamline usage of caching framework
Browse files Browse the repository at this point in the history
Use code cache properly instead of storing strings into it,
because it is proper usage of our API, removes the need
for stripping PHP code strings and improves performance
when OPCache is in use (which is recommended on production systems).

Additionally all calls to ->has are removed, except one place where
boolean values are stored in a runtime cache and in tests,
because the API guarantees returning "false" when using ->get or ->require

Doing so reduces the amount of lookups on the backend storage (file_exists
for file backend, SQL queries for DB backend) and makes up more
straightforward code in most places (reduces cyclomatic complexity by removing obsolete "else" branching).

Releases: master
Resolves: #89895
Change-Id: I401512ad4ddffb622ce2f4a9d88274deb4b4e849
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/62558
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Achim Fritz <af@achimfritz.de>
Tested-by: Benni Mack <benni@typo3.org>
Reviewed-by: Achim Fritz <af@achimfritz.de>
Reviewed-by: Benni Mack <benni@typo3.org>
  • Loading branch information
bmack committed Feb 14, 2020
1 parent 0360ef3 commit 266bf67
Show file tree
Hide file tree
Showing 21 changed files with 102 additions and 212 deletions.
5 changes: 2 additions & 3 deletions typo3/sysext/backend/Classes/ServiceProvider.php
Expand Up @@ -96,9 +96,8 @@ public static function configureBackendRouter(ContainerInterface $container, Rou
$cache = $container->get('cache.core');

$cacheIdentifier = 'BackendRoutes_' . sha1((string)(new Typo3Version()) . Environment::getProjectPath() . 'BackendRoutes');
if ($cache->has($cacheIdentifier)) {
$routesFromPackages = $cache->require($cacheIdentifier);
} else {
$routesFromPackages = $cache->require($cacheIdentifier);
if ($routesFromPackages === false) {
$routesFromPackages = $container->get('backend.routes')->getArrayCopy();
$cache->set($cacheIdentifier, 'return ' . var_export($routesFromPackages, true) . ';');
}
Expand Down
5 changes: 3 additions & 2 deletions typo3/sysext/backend/Classes/Utility/BackendUtility.php
Expand Up @@ -685,8 +685,9 @@ public static function getPagesTSconfig($id)
$id = (int)$id;

$cache = self::getRuntimeCache();
if ($cache->has('pagesTsConfigIdToHash' . $id)) {
return $cache->get('pagesTsConfigHashToContent' . $cache->get('pagesTsConfigIdToHash' . $id));
$pagesTsConfigIdToHash = $cache->get('pagesTsConfigIdToHash' . $id);
if ($pagesTsConfigIdToHash !== false) {
return $cache->get('pagesTsConfigHashToContent' . $pagesTsConfigIdToHash);
}

$rootLine = self::BEgetRootLine($id, '', true);
Expand Down
4 changes: 4 additions & 0 deletions typo3/sysext/core/Classes/Cache/Backend/NullBackend.php
Expand Up @@ -111,17 +111,21 @@ public function collectGarbage()
* Does nothing
*
* @param string $identifier An identifier which describes the cache entry to load
* @return bool
*/
public function requireOnce($identifier)
{
return false;
}

/**
* Does nothing
*
* @param string $identifier An identifier which describes the cache entry to load
* @return bool
*/
public function require(string $identifier)
{
return false;
}
}
141 changes: 14 additions & 127 deletions typo3/sysext/core/Classes/Cache/Frontend/NullFrontend.php
@@ -1,5 +1,6 @@
<?php
declare(strict_types = 1);
namespace TYPO3\CMS\Core\Cache\Frontend;

/*
* This file is part of the TYPO3 CMS project.
Expand All @@ -13,144 +14,30 @@
*
* The TYPO3 project - inspiring people to share!
*/
namespace TYPO3\CMS\Core\Cache\Frontend;

use Psr\Log\NullLogger;
use TYPO3\CMS\Core\Cache\Backend\NullBackend;

/**
* Class TYPO3\CMS\Core\Cache\Frontend\NullFrontend
* This class only acts as shortcut to construct a cache frontend with a null backend.
* It extends PhpFrontend to be sure it can also be used for all types of caches (also the one requiring a PhpFrontend like "core").
* TODO: Instead a factory class should be introduced that replaces this class and \TYPO3\CMS\Core\Core\Bootstrap::createCache
*/
class NullFrontend implements FrontendInterface
class NullFrontend extends PhpFrontend
{
/**
* @var string
*/
private $identifier;

/**
* @param string $identifier
*/
public function __construct(string $identifier)
{
$this->identifier = $identifier;
}

/**
* Returns this cache's identifier
*
* @return string The identifier for this cache
*/
public function getIdentifier()
{
return $this->identifier;
}

/**
* Returns the backend used by this cache
*
* @return \TYPO3\CMS\Core\Cache\Backend\BackendInterface The backend used by this cache
*/
public function getBackend()
{
return new NullBackend('');
$backend = new NullBackend(
'',
[
'logger' => new NullLogger(),
]
);
parent::__construct($identifier, $backend);
}

/**
* Saves data in the cache.
*
* @param string $entryIdentifier Something which identifies the data - depends on concrete cache
* @param mixed $data The data to cache - also depends on the concrete cache implementation
* @param array $tags Tags to associate with this cache entry
* @param int $lifetime Lifetime of this cache entry in seconds. If NULL is specified, the default lifetime is used. "0" means unlimited lifetime.
*/
public function set($entryIdentifier, $data, array $tags = [], $lifetime = null)
{
}

/**
* Finds and returns data from the cache.
*
* @param string $entryIdentifier Something which identifies the cache entry - depends on concrete cache
* @return mixed
*/
public function get($entryIdentifier)
{
return null;
}

/**
* Checks if a cache entry with the specified identifier exists.
*
* @param string $entryIdentifier An identifier specifying the cache entry
* @return bool TRUE if such an entry exists, FALSE if not
*/
public function has($entryIdentifier)
{
return false;
}

/**
* Removes the given cache entry from the cache.
*
* @param string $entryIdentifier An identifier specifying the cache entry
* @return bool TRUE if such an entry exists, FALSE if not
*/
public function remove($entryIdentifier)
{
return false;
}

/**
* Removes all cache entries of this cache.
*/
public function flush()
{
}

/**
* Removes all cache entries of this cache which are tagged by the specified tag.
*
* @param string $tag The tag the entries must have
*/
public function flushByTag($tag)
{
}

/**
* Removes all cache entries of this cache which are tagged by any of the specified tags.
*
* @param string[] $tags List of tags
*/
public function flushByTags(array $tags)
{
}

/**
* Does garbage collection
*/
public function collectGarbage()
{
}

/**
* Checks the validity of an entry identifier. Returns TRUE if it's valid.
*
* @param string $identifier An identifier to be checked for validity
* @return bool
*/
public function isValidEntryIdentifier($identifier)
{
return false;
}

/**
* Checks the validity of a tag. Returns TRUE if it's valid.
*
* @param string $tag A tag to be checked for validity
* @return bool
*/
public function isValidTag($tag)
{
return false;
// Noop
}
}
51 changes: 23 additions & 28 deletions typo3/sysext/core/Classes/Configuration/SiteConfiguration.php
Expand Up @@ -19,7 +19,7 @@
use Symfony\Component\Finder\Finder;
use Symfony\Component\Yaml\Yaml;
use TYPO3\CMS\Core\Cache\CacheManager;
use TYPO3\CMS\Core\Cache\Frontend\FrontendInterface;
use TYPO3\CMS\Core\Cache\Frontend\PhpFrontend;
use TYPO3\CMS\Core\Configuration\Loader\YamlFileLoader;
use TYPO3\CMS\Core\Exception\SiteNotFoundException;
use TYPO3\CMS\Core\SingletonInterface;
Expand Down Expand Up @@ -55,7 +55,7 @@ class SiteConfiguration implements SingletonInterface
* @internal
* @var string
*/
protected $cacheIdentifier = 'site-configuration';
protected $cacheIdentifier = 'sites-configuration';

/**
* Cache stores all configuration as Site objects, as long as they haven't been changed.
Expand Down Expand Up @@ -149,33 +149,28 @@ public function resolveAllExistingSites(bool $useCache = true): array
*/
protected function getAllSiteConfigurationFromFiles(bool $useCache = true): array
{
$siteConfiguration = null;
// Check if the data is already cached
if ($useCache && $siteConfiguration = $this->getCache()->get($this->cacheIdentifier)) {
// Due to the nature of PhpFrontend, the `<?php` and `#` wraps have to be removed
$siteConfiguration = preg_replace('/^<\?php\s*|\s*#$/', '', $siteConfiguration);
$siteConfiguration = json_decode($siteConfiguration, true);
$siteConfiguration = $useCache ? $this->getCache()->require($this->cacheIdentifier) : false;
if ($siteConfiguration !== false) {
return $siteConfiguration;
}

// Nothing in the cache (or no site found)
if (empty($siteConfiguration)) {
$finder = new Finder();
try {
$finder->files()->depth(0)->name($this->configFileName)->in($this->configPath . '/*');
} catch (\InvalidArgumentException $e) {
// Directory $this->configPath does not exist yet
$finder = [];
}
$loader = GeneralUtility::makeInstance(YamlFileLoader::class);
$siteConfiguration = [];
foreach ($finder as $fileInfo) {
$configuration = $loader->load(GeneralUtility::fixWindowsFilePath((string)$fileInfo));
$identifier = basename($fileInfo->getPath());
$siteConfiguration[$identifier] = $configuration;
}
$this->getCache()->set($this->cacheIdentifier, json_encode($siteConfiguration));
$finder = new Finder();
try {
$finder->files()->depth(0)->name($this->configFileName)->in($this->configPath . '/*');
} catch (\InvalidArgumentException $e) {
// Directory $this->configPath does not exist yet
$finder = [];
}
return $siteConfiguration ?? [];
$loader = GeneralUtility::makeInstance(YamlFileLoader::class);
$siteConfiguration = [];
foreach ($finder as $fileInfo) {
$configuration = $loader->load(GeneralUtility::fixWindowsFilePath((string)$fileInfo));
$identifier = basename($fileInfo->getPath());
$siteConfiguration[$identifier] = $configuration;
}
$this->getCache()->set($this->cacheIdentifier, 'return ' . var_export($siteConfiguration, true) . ';');

return $siteConfiguration;
}

/**
Expand Down Expand Up @@ -272,10 +267,10 @@ public function delete(string $siteIdentifier): void
/**
* Short-hand function for the cache
*
* @return FrontendInterface
* @return PhpFrontend
* @throws \TYPO3\CMS\Core\Cache\Exception\NoSuchCacheException
*/
protected function getCache(): FrontendInterface
protected function getCache(): PhpFrontend
{
return GeneralUtility::makeInstance(CacheManager::class)->getCache('core');
}
Expand Down
14 changes: 7 additions & 7 deletions typo3/sysext/core/Classes/Database/ReferenceIndex.php
Expand Up @@ -216,11 +216,12 @@ public function updateRefIndexTable($tableName, $uid, $testOnly = false)

// Fetch tableRelationFields and save them in cache if not there yet
$cacheId = static::$cachePrefixTableRelationFields . $tableName;
if (!$this->useRuntimeCache || !$this->runtimeCache->has($cacheId)) {
$tableRelationFields = $this->useRuntimeCache ? $this->runtimeCache->get($cacheId) : false;
if ($tableRelationFields === false) {
$tableRelationFields = $this->fetchTableRelationFields($tableName);
$this->runtimeCache->set($cacheId, $tableRelationFields);
} else {
$tableRelationFields = $this->runtimeCache->get($cacheId);
if ($this->useRuntimeCache) {
$this->runtimeCache->set($cacheId, $tableRelationFields);
}
}

$connectionPool = GeneralUtility::makeInstance(ConnectionPool::class);
Expand Down Expand Up @@ -1300,13 +1301,12 @@ protected function getRecordRawCached(string $tableName, int $uid)

// Fetch fields of the table which might contain relations
$cacheId = static::$cachePrefixTableRelationFields . $tableName;
if (!$this->useRuntimeCache || !$this->runtimeCache->has($cacheId)) {
$tableRelationFields = $this->useRuntimeCache ? $this->runtimeCache->get($cacheId) : false;
if ($tableRelationFields === false) {
$tableRelationFields = $this->fetchTableRelationFields($tableName);
if ($this->useRuntimeCache) {
$this->runtimeCache->set($cacheId, $tableRelationFields);
}
} else {
$tableRelationFields = $this->runtimeCache->get($cacheId);
}

// Return if there are no fields which could contain relations
Expand Down
Expand Up @@ -73,9 +73,8 @@ public function createDependencyInjectionContainer(PackageManager $packageManage
$cacheIdentifier = $this->getCacheIdentifier();
$containerClassName = $cacheIdentifier;

if ($cache->has($cacheIdentifier)) {
$cache->requireOnce($cacheIdentifier);
} else {
$hasCache = $cache->requireOnce($cacheIdentifier) !== false;
if (!$hasCache) {
$containerBuilder = $this->buildContainer($packageManager, $serviceProviderRegistry);
$code = $this->dumpContainer($containerBuilder, $cache);

Expand All @@ -85,9 +84,8 @@ public function createDependencyInjectionContainer(PackageManager $packageManage
// Once we remove support for singletons configured in ext_localconf.php
// and $GLOBALS['TYPO_CONF_VARS']['SYS']['Objects'], we can remove this,
// and use `$container = $containerBuilder` directly
if ($cache->has($cacheIdentifier)) {
$cache->requireOnce($cacheIdentifier);
} else {
$hasCache = $cache->requireOnce($cacheIdentifier) !== false;
if (!$hasCache) {
// $cacheIdentifier may be unavailable if the 'core' cache iis configured to
// use the NullBackend
eval($code);
Expand Down
Expand Up @@ -24,9 +24,10 @@ public function getExpressionLanguageProviders(): array
$packageManager = GeneralUtility::makeInstance(PackageManager::class);
$cache = GeneralUtility::makeInstance(CacheManager::class)->getCache('core');

if ($cache->has($this->cacheIdentifier)) {
/** @noinspection PhpUndefinedMethodInspection the method require() will be added to the interface in TYPO3 v10.0 */
return $cache->require($this->cacheIdentifier);
/** @noinspection PhpUndefinedMethodInspection the method require() will be added to the interface in TYPO3 v10.0 */
$providers = $cache->require($this->cacheIdentifier);
if ($providers !== false) {
return $providers;
}

$packages = $packageManager->getActivePackages();
Expand Down
4 changes: 2 additions & 2 deletions typo3/sysext/core/Classes/Package/PackageManager.php
Expand Up @@ -169,7 +169,7 @@ protected function getCacheEntryIdentifier()
protected function saveToPackageCache()
{
$cacheEntryIdentifier = $this->getCacheEntryIdentifier();
if ($cacheEntryIdentifier !== null && !$this->coreCache->has($cacheEntryIdentifier)) {
if ($cacheEntryIdentifier !== null) {
// Build cache file
$packageCache = [
'packageStatesConfiguration' => $this->packageStatesConfiguration,
Expand All @@ -192,7 +192,7 @@ protected function saveToPackageCache()
protected function loadPackageManagerStatesFromCache()
{
$cacheEntryIdentifier = $this->getCacheEntryIdentifier();
if ($cacheEntryIdentifier === null || !$this->coreCache->has($cacheEntryIdentifier) || !($packageCache = $this->coreCache->require($cacheEntryIdentifier))) {
if ($cacheEntryIdentifier === null || ($packageCache = $this->coreCache->require($cacheEntryIdentifier)) === false) {
throw new Exception\PackageManagerCacheUnavailableException('The package state cache could not be loaded.', 1393883342);
}
$this->packageStatesConfiguration = $packageCache['packageStatesConfiguration'];
Expand Down

0 comments on commit 266bf67

Please sign in to comment.