Commit b5539e3d authored by Benni Mack's avatar Benni Mack
Browse files

[BUGFIX] Speed up SiteConfiguration loading

Profiling shows that when linking to 100
pages, SiteFinder (which instantiates SiteConfiguration)
is instantiated 100 times. Although SiteFinder information
might change during one request, the SiteConfiguration
does not (except when updating the Configuration via API).

So, a first-level-cache can be used to avoid calls
to "cache_core" multiple times during one request,
and SiteConfiguration can become a Singleton instance.

Resolves: #88577
Releases: master, 9.5
Change-Id: I3d9167da9442d684d32a73d6cf2003c91bdf4d68
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/61078

Tested-by: Benjamin Franzke's avatarBenjamin Franzke <bfr@qbus.de>
Tested-by: default avatarTYPO3com <noreply@typo3.com>
Tested-by: Daniel Goerz's avatarDaniel Goerz <daniel.goerz@posteo.de>
Tested-by: Benni Mack's avatarBenni Mack <benni@typo3.org>
Reviewed-by: Benjamin Franzke's avatarBenjamin Franzke <bfr@qbus.de>
Reviewed-by: Daniel Goerz's avatarDaniel Goerz <daniel.goerz@posteo.de>
Reviewed-by: Benni Mack's avatarBenni Mack <benni@typo3.org>
parent 4ec169f2
......@@ -28,6 +28,22 @@ use TYPO3\CMS\Core\Utility\GeneralUtility;
*/
class SiteDatabaseEditRow implements FormDataProviderInterface
{
/**
* @var SiteConfiguration
*/
protected $siteConfiguration;
/**
* @param SiteConfiguration $siteConfiguration
*/
public function __construct(SiteConfiguration $siteConfiguration = null)
{
$this->siteConfiguration = $siteConfiguration ?? GeneralUtility::makeInstance(
SiteConfiguration::class,
Environment::getConfigPath() . '/sites'
);
}
/**
* First level of ['customData']['siteData'] to ['databaseRow']
*
......@@ -42,7 +58,7 @@ class SiteDatabaseEditRow implements FormDataProviderInterface
}
$tableName = $result['tableName'];
$siteFinder = GeneralUtility::makeInstance(SiteFinder::class);
$siteFinder = GeneralUtility::makeInstance(SiteFinder::class, $this->siteConfiguration);
if ($tableName === 'site') {
$rootPageId = (int)$result['vanillaUid'];
$rowData = $this->getRawConfigurationForSiteWithRootPageId($siteFinder, $rootPageId);
......@@ -80,11 +96,7 @@ class SiteDatabaseEditRow implements FormDataProviderInterface
protected function getRawConfigurationForSiteWithRootPageId(SiteFinder $siteFinder, int $rootPageId): array
{
$site = $siteFinder->getSiteByRootPageId($rootPageId);
$siteConfiguration = GeneralUtility::makeInstance(
SiteConfiguration::class,
Environment::getConfigPath() . '/sites'
);
// load config as it is stored on disk (without replacements)
return $siteConfiguration->load($site->getIdentifier());
return $this->siteConfiguration->load($site->getIdentifier());
}
}
......@@ -55,7 +55,8 @@ class SiteDatabaseEditRowTest extends UnitTestCase
'command' => 'new',
'foo' => 'bar',
];
$this->assertSame($input, (new SiteDatabaseEditRow())->addData($input));
$siteConfigurationProphecy = $this->prophesize(SiteConfiguration::class);
$this->assertSame($input, (new SiteDatabaseEditRow($siteConfigurationProphecy->reveal()))->addData($input));
}
/**
......@@ -69,7 +70,8 @@ class SiteDatabaseEditRowTest extends UnitTestCase
'foo' => 'bar',
]
];
$this->assertSame($input, (new SiteDatabaseEditRow())->addData($input));
$siteConfigurationProphecy = $this->prophesize(SiteConfiguration::class);
$this->assertSame($input, (new SiteDatabaseEditRow($siteConfigurationProphecy->reveal()))->addData($input));
}
/**
......@@ -84,8 +86,9 @@ class SiteDatabaseEditRowTest extends UnitTestCase
$this->expectException(\RuntimeException::class);
$this->expectExceptionCode(1520886234);
$siteFinderProphecy = $this->prophesize(SiteFinder::class);
$siteConfigurationProphecy = $this->prophesize(SiteConfiguration::class);
GeneralUtility::addInstance(SiteFinder::class, $siteFinderProphecy->reveal());
(new SiteDatabaseEditRow())->addData($input);
(new SiteDatabaseEditRow($siteConfigurationProphecy->reveal()))->addData($input);
}
/**
......@@ -115,7 +118,6 @@ class SiteDatabaseEditRowTest extends UnitTestCase
$siteProphecy->getIdentifier()->willReturn('testident');
$siteConfiguration = $this->prophesize(SiteConfiguration::class);
$siteConfiguration->load('testident')->willReturn($rowData);
GeneralUtility::addInstance(SiteConfiguration::class, $siteConfiguration->reveal());
$expected = $input;
$expected['databaseRow'] = [
......@@ -126,7 +128,7 @@ class SiteDatabaseEditRowTest extends UnitTestCase
'foo' => 'bar',
];
$this->assertEquals($expected, (new SiteDatabaseEditRow())->addData($input));
$this->assertEquals($expected, (new SiteDatabaseEditRow($siteConfiguration->reveal()))->addData($input));
}
/**
......@@ -151,11 +153,10 @@ class SiteDatabaseEditRowTest extends UnitTestCase
$siteProphecy->getIdentifier()->willReturn('testident');
$siteConfiguration = $this->prophesize(SiteConfiguration::class);
$siteConfiguration->load('testident')->willReturn($rowData);
GeneralUtility::addInstance(SiteConfiguration::class, $siteConfiguration->reveal());
$this->expectException(\RuntimeException::class);
$this->expectExceptionCode(1520886092);
(new SiteDatabaseEditRow())->addData($input);
(new SiteDatabaseEditRow($siteConfiguration->reveal()))->addData($input);
}
/**
......@@ -180,11 +181,10 @@ class SiteDatabaseEditRowTest extends UnitTestCase
$siteProphecy->getIdentifier()->willReturn('testident');
$siteConfiguration = $this->prophesize(SiteConfiguration::class);
$siteConfiguration->load('testident')->willReturn($rowData);
GeneralUtility::addInstance(SiteConfiguration::class, $siteConfiguration->reveal());
$this->expectException(\RuntimeException::class);
$this->expectExceptionCode(1520886092);
(new SiteDatabaseEditRow())->addData($input);
(new SiteDatabaseEditRow($siteConfiguration->reveal()))->addData($input);
}
/**
......@@ -213,7 +213,6 @@ class SiteDatabaseEditRowTest extends UnitTestCase
$siteProphecy->getIdentifier()->willReturn('testident');
$siteConfiguration = $this->prophesize(SiteConfiguration::class);
$siteConfiguration->load('testident')->willReturn($rowData);
GeneralUtility::addInstance(SiteConfiguration::class, $siteConfiguration->reveal());
$expected = $input;
$expected['databaseRow'] = [
......@@ -222,6 +221,6 @@ class SiteDatabaseEditRowTest extends UnitTestCase
'pid' => 0,
];
$this->assertEquals($expected, (new SiteDatabaseEditRow())->addData($input));
$this->assertEquals($expected, (new SiteDatabaseEditRow($siteConfiguration->reveal()))->addData($input));
}
}
......@@ -22,6 +22,7 @@ use TYPO3\CMS\Core\Cache\CacheManager;
use TYPO3\CMS\Core\Cache\Frontend\FrontendInterface;
use TYPO3\CMS\Core\Configuration\Loader\YamlFileLoader;
use TYPO3\CMS\Core\Exception\SiteNotFoundException;
use TYPO3\CMS\Core\SingletonInterface;
use TYPO3\CMS\Core\Site\Entity\Site;
use TYPO3\CMS\Core\Utility\GeneralUtility;
......@@ -32,7 +33,7 @@ use TYPO3\CMS\Core\Utility\GeneralUtility;
*
* @internal
*/
class SiteConfiguration
class SiteConfiguration implements SingletonInterface
{
/**
* @var string
......@@ -55,6 +56,14 @@ class SiteConfiguration
*/
protected $cacheIdentifier = 'site-configuration';
/**
* Cache stores all configuration as Site objects, as long as they haven't been changed.
* This drastically improves performance as SiteFinder utilizes SiteConfiguration heavily
*
* @var array|null
*/
protected $firstLevelCache;
/**
* @param string $configPath
*/
......@@ -68,6 +77,16 @@ class SiteConfiguration
*
* @return Site[]
*/
public function getAllExistingSites(): array
{
return $this->firstLevelCache ?? $this->resolveAllExistingSites();
}
/**
* Resolve all site objects which have been found in the filesystem.
*
* @return Site[]
*/
public function resolveAllExistingSites(): array
{
$sites = [];
......@@ -78,6 +97,7 @@ class SiteConfiguration
$sites[$identifier] = GeneralUtility::makeInstance(Site::class, $identifier, $rootPageId, $configuration);
}
}
$this->firstLevelCache = $sites;
return $sites;
}
......@@ -149,6 +169,7 @@ class SiteConfiguration
}
$yamlFileContents = Yaml::dump($configuration, 99, 2);
GeneralUtility::writeFile($fileName, $yamlFileContents);
$this->firstLevelCache = null;
$this->getCache()->remove($this->cacheIdentifier);
$this->getCache()->remove('pseudo-sites');
}
......@@ -167,6 +188,7 @@ class SiteConfiguration
throw new \RuntimeException('Unable to rename folder sites/' . $currentIdentifier, 1522491300);
}
$this->getCache()->remove($this->cacheIdentifier);
$this->firstLevelCache = null;
}
/**
......@@ -178,7 +200,7 @@ class SiteConfiguration
*/
public function delete(string $siteIdentifier): void
{
$sites = $this->resolveAllExistingSites();
$sites = $this->getAllExistingSites();
if (!isset($sites[$siteIdentifier])) {
throw new SiteNotFoundException('Site configuration named ' . $siteIdentifier . ' not found.', 1522866183);
}
......@@ -189,6 +211,7 @@ class SiteConfiguration
@unlink($fileName);
$this->getCache()->remove($this->cacheIdentifier);
$this->getCache()->remove('pseudo-sites');
$this->firstLevelCache = null;
}
/**
......
......@@ -43,11 +43,16 @@ class SiteFinder
/**
* Fetches all existing configurations as Site objects
*
* @param SiteConfiguration $siteConfiguration
*/
public function __construct()
public function __construct(SiteConfiguration $siteConfiguration = null)
{
$reader = GeneralUtility::makeInstance(SiteConfiguration::class, Environment::getConfigPath() . '/sites');
$sites = $reader->resolveAllExistingSites();
$siteConfiguration = $siteConfiguration ?? GeneralUtility::makeInstance(
SiteConfiguration::class,
Environment::getConfigPath() . '/sites'
);
$sites = $siteConfiguration->getAllExistingSites();
foreach ($sites as $identifier => $site) {
$this->sites[$identifier] = $site;
$this->mappingRootPageIdToIdentifier[$site->getRootPageId()] = $identifier;
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment