[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 <bfr@qbus.de>
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Daniel Goerz <daniel.goerz@posteo.de>
Tested-by: Benni Mack <benni@typo3.org>
Reviewed-by: Benjamin Franzke <bfr@qbus.de>
Reviewed-by: Daniel Goerz <daniel.goerz@posteo.de>
Reviewed-by: Benni Mack <benni@typo3.org>

diff --git a/typo3/sysext/backend/Classes/Form/FormDataProvider/SiteDatabaseEditRow.php b/typo3/sysext/backend/Classes/Form/FormDataProvider/SiteDatabaseEditRow.php
index 6f12603..834d1a5 100644 (file)
--- a/typo3/sysext/backend/Classes/Form/FormDataProvider/SiteDatabaseEditRow.php
+++ b/typo3/sysext/backend/Classes/Form/FormDataProvider/SiteDatabaseEditRow.php
@@ -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());
     }
 }

diff --git a/typo3/sysext/backend/Tests/Unit/Form/FormDataProvider/SiteDatabaseEditRowTest.php b/typo3/sysext/backend/Tests/Unit/Form/FormDataProvider/SiteDatabaseEditRowTest.php
index 942e212..7c6af3e 100644 (file)
--- a/typo3/sysext/backend/Tests/Unit/Form/FormDataProvider/SiteDatabaseEditRowTest.php
+++ b/typo3/sysext/backend/Tests/Unit/Form/FormDataProvider/SiteDatabaseEditRowTest.php
@@ -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));
     }
 }

diff --git a/typo3/sysext/core/Classes/Configuration/SiteConfiguration.php b/typo3/sysext/core/Classes/Configuration/SiteConfiguration.php
index 76a97e5..a3df395 100644 (file)
--- a/typo3/sysext/core/Classes/Configuration/SiteConfiguration.php
+++ b/typo3/sysext/core/Classes/Configuration/SiteConfiguration.php
@@ -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;
     }
 
     /**

diff --git a/typo3/sysext/core/Classes/Site/SiteFinder.php b/typo3/sysext/core/Classes/Site/SiteFinder.php
index 6924899..316b288 100644 (file)
--- a/typo3/sysext/core/Classes/Site/SiteFinder.php
+++ b/typo3/sysext/core/Classes/Site/SiteFinder.php
@@ -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;