Commit c333efe9 authored by Christian Kuhn's avatar Christian Kuhn Committed by Georg Ringer
Browse files

[TASK] Deferred reference index updating

Within DataHandler processing, updating the reference index
is an expensive task since it needs tons of queries to do
the job.

With DataHandler instances and it's sub instances created
for relations and localizations, the code tends to update the
reference index for the same table/uid combination multiple times:
As example, core test IRRE/ForeignField/Modify copyParentContent()
copies a content element with one hotel, one offer and one price
around. This leads to 3 update reference index calls for the
tt_content element, 4 for the hotel, 4 for the offer and 3 for
the price.
Of course, the situation is worse in workspaces.

There have been various attempts over the years to reduce the
query load, usually by adding runtime caches everywhere.

With proper sys_refindex functional test coverage in place, we
can however finally solve the root problem: The patch adds the
helper object ReferenceIndexUpdater to fill a registry with
to-update workspace/table/uid combinations. The object is carried
around within DataHandler and RelationHandler to DataHandler sub
instances. Only the outer most instance of a DataHandler then
finally executes the update() operations in one go and only
once per combination.

The patch tries to be rather conservative to allow a 10.4 backport.
For master, there should be further mess-reducing cleanup patches
to streamline related parts of the ReferenceIndex update process.

Result: The DataHandler query load is reduced significantly. It
heavily depends on the structure that is changed, to get an idea,
the above test goes down from 448 queries to 346 queries!

Change-Id: I49f5ed73114ca5d6e2cb75fa43846bde5ea72d26
Resolves: #92356
Related: #88134
Releases: master, 10.4
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/65796


Tested-by: Oliver Bartsch's avatarOliver Bartsch <bo@cedev.de>
Tested-by: default avatarTYPO3com <noreply@typo3.com>
Tested-by: Benni Mack's avatarBenni Mack <benni@typo3.org>
Tested-by: Georg Ringer's avatarGeorg Ringer <georg.ringer@gmail.com>
Reviewed-by: Oliver Bartsch's avatarOliver Bartsch <bo@cedev.de>
Reviewed-by: Benni Mack's avatarBenni Mack <benni@typo3.org>
Reviewed-by: Georg Ringer's avatarGeorg Ringer <georg.ringer@gmail.com>
parent 14c742b7
......@@ -42,7 +42,6 @@ use TYPO3\CMS\Core\Database\Query\Restriction\BackendWorkspaceRestriction;
use TYPO3\CMS\Core\Database\Query\Restriction\DeletedRestriction;
use TYPO3\CMS\Core\Database\Query\Restriction\QueryRestrictionContainerInterface;
use TYPO3\CMS\Core\Database\Query\Restriction\WorkspaceRestriction;
use TYPO3\CMS\Core\Database\ReferenceIndex;
use TYPO3\CMS\Core\Database\RelationHandler;
use TYPO3\CMS\Core\DataHandling\History\RecordHistoryStore;
use TYPO3\CMS\Core\DataHandling\Localization\DataMapProcessor;
......@@ -542,12 +541,15 @@ class DataHandler implements LoggerAwareInterface
protected $remapStackRefIndex = [];
/**
* Array used for additional calls to $this->updateRefIndex
* Registry object to gather reference index update requests and perform updates after
* main processing has been done. The first call to start() instantiates this object.
* Recursive sub instances receive this instance via __construct().
* The final update() call is done at the end of process_cmdmap() or process_datamap()
* in the outer most instance.
*
* @var array
* @internal
* @var ReferenceIndexUpdater
*/
public $updateRefIndexStack = [];
protected $referenceIndexUpdater;
/**
* Tells, that this DataHandler instance was called from \TYPO3\CMS\Impext\ImportExport.
......@@ -624,13 +626,21 @@ class DataHandler implements LoggerAwareInterface
/**
* Sets up the data handler cache and some additional options, the main logic is done in the start() method.
*
* @param ReferenceIndexUpdater|null $referenceIndexUpdater Hand over from outer most instance to sub instances
*/
public function __construct()
public function __construct(ReferenceIndexUpdater $referenceIndexUpdater = null)
{
$this->checkStoredRecords = (bool)$GLOBALS['TYPO3_CONF_VARS']['BE']['checkStoredRecords'];
$this->checkStoredRecords_loose = (bool)$GLOBALS['TYPO3_CONF_VARS']['BE']['checkStoredRecordsLoose'];
$this->runtimeCache = $this->getRuntimeCache();
$this->pagePermissionAssembler = GeneralUtility::makeInstance(PagePermissionAssembler::class, $GLOBALS['TYPO3_CONF_VARS']['BE']['defaultPermissions']);
if ($referenceIndexUpdater === null) {
// Create ReferenceIndexUpdater object. This should only happen on outer most instance,
// sub instances should receive the reference index updater from a parent.
$referenceIndexUpdater = GeneralUtility::makeInstance(ReferenceIndexUpdater::class);
}
$this->referenceIndexUpdater = $referenceIndexUpdater;
}
/**
......@@ -868,7 +878,7 @@ class DataHandler implements LoggerAwareInterface
}
// Pre-process data-map and synchronize localization states
$this->datamap = GeneralUtility::makeInstance(SlugEnricher::class)->enrichDataMap($this->datamap);
$this->datamap = DataMapProcessor::instance($this->datamap, $this->BE_USER)->process();
$this->datamap = DataMapProcessor::instance($this->datamap, $this->BE_USER, $this->referenceIndexUpdater)->process();
// Organize tables so that the pages-table is always processed first. This is required if you want to make sure that content pointing to a new page will be created.
$orderOfTables = [];
// Set pages first.
......@@ -1014,7 +1024,7 @@ class DataHandler implements LoggerAwareInterface
$this->pagetreeNeedsRefresh = true;
/** @var DataHandler $tce */
$tce = GeneralUtility::makeInstance(__CLASS__);
$tce = GeneralUtility::makeInstance(__CLASS__, $this->referenceIndexUpdater);
$tce->enableLogging = $this->enableLogging;
// Setting up command for creating a new version of the record:
$cmd = [];
......@@ -1191,7 +1201,9 @@ class DataHandler implements LoggerAwareInterface
$hookObj->processDatamap_afterAllOperations($this);
}
}
if ($this->isOuterMostInstance()) {
$this->referenceIndexUpdater->update();
$this->processClearCacheQueue();
$this->resetElementsToBeDeleted();
}
......@@ -3176,6 +3188,7 @@ class DataHandler implements LoggerAwareInterface
}
}
if ($this->isOuterMostInstance()) {
$this->referenceIndexUpdater->update();
$this->processClearCacheQueue();
$this->resetNestedElementCalls();
}
......@@ -4610,7 +4623,7 @@ class DataHandler implements LoggerAwareInterface
// Remove child records (if synchronization requested it):
if (is_array($removeArray) && !empty($removeArray)) {
/** @var DataHandler $tce */
$tce = GeneralUtility::makeInstance(__CLASS__);
$tce = GeneralUtility::makeInstance(__CLASS__, $this->referenceIndexUpdater);
$tce->enableLogging = $this->enableLogging;
$tce->start([], $removeArray, $this->BE_USER);
$tce->process_cmdmap();
......@@ -4850,25 +4863,6 @@ class DataHandler implements LoggerAwareInterface
// Update reference index:
$this->updateRefIndex($table, $uid);
// We track calls to update the reference index as to avoid calling it twice
// with the same arguments. This is done because reference indexing is quite
// costly and the update reference index stack usually contain duplicates.
// NB: also filled and checked in loop below. The initialisation prevents
// running the "root" record twice if it appears in the stack twice.
$updateReferenceIndexCalls = [[$table, $uid]];
// If there are entries in the updateRefIndexStack
if (is_array($this->updateRefIndexStack[$table]) && is_array($this->updateRefIndexStack[$table][$uid])) {
while ($args = array_pop($this->updateRefIndexStack[$table][$uid])) {
if (!in_array($args, $updateReferenceIndexCalls, true)) {
// $args[0]: table, $args[1]: uid
$this->updateRefIndex($args[0], $args[1]);
$updateReferenceIndexCalls[] = $args;
}
}
unset($this->updateRefIndexStack[$table][$uid]);
}
}
/**
......@@ -5031,7 +5025,7 @@ class DataHandler implements LoggerAwareInterface
]
];
/** @var DataHandler $dataHandler */
$dataHandler = GeneralUtility::makeInstance(__CLASS__);
$dataHandler = GeneralUtility::makeInstance(__CLASS__, $this->referenceIndexUpdater);
$dataHandler->enableLogging = $this->enableLogging;
$dataHandler->neverHideAtCopy = true;
$dataHandler->start([], $command, $this->BE_USER);
......@@ -5221,7 +5215,7 @@ class DataHandler implements LoggerAwareInterface
$dbAnalysis = $this->createRelationHandlerInstance();
$dbAnalysis->start($value, $allowedTables, $conf['MM'], $uid, $table, $conf);
foreach ($dbAnalysis->itemArray as $v) {
$this->updateRefIndexStack[$table][$uid][] = [$v['table'], $v['id']];
$this->updateRefIndex($v['table'], $v['id']);
}
}
}
......@@ -5790,7 +5784,7 @@ class DataHandler implements LoggerAwareInterface
*/
protected function getLocalTCE()
{
$copyTCE = GeneralUtility::makeInstance(DataHandler::class);
$copyTCE = GeneralUtility::makeInstance(DataHandler::class, $this->referenceIndexUpdater);
$copyTCE->copyTree = $this->copyTree;
$copyTCE->enableLogging = $this->enableLogging;
// Transformations should NOT be carried out during copy
......@@ -7239,22 +7233,16 @@ class DataHandler implements LoggerAwareInterface
}
/**
* Update Reference Index (sys_refindex) for a record
* Register a table/uid combination in current user workspace for reference updating.
* Should be called on almost any update to a record which could affect references inside the record.
*
* @param string $table Table name
* @param int $id Record UID
* @param int $uid Record UID
* @internal should only be used from within DataHandler
*/
public function updateRefIndex($table, $id)
public function updateRefIndex($table, $uid): void
{
/** @var ReferenceIndex $refIndexObj */
$refIndexObj = GeneralUtility::makeInstance(ReferenceIndex::class);
if (BackendUtility::isTableWorkspaceEnabled($table)) {
$refIndexObj->setWorkspaceId($this->BE_USER->workspace);
}
$refIndexObj->enableRuntimeCache();
$refIndexObj->updateRefIndexTable($table, $id);
$this->referenceIndexUpdater->registerForUpdate((string)$table, (int)$uid, (int)$this->BE_USER->workspace);
}
/*********************************************
......@@ -9132,6 +9120,7 @@ class DataHandler implements LoggerAwareInterface
$relationHandler->setWorkspaceId($this->BE_USER->workspace);
$relationHandler->setUseLiveReferenceIds($isWorkspacesLoaded);
$relationHandler->setUseLiveParentIds($isWorkspacesLoaded);
$relationHandler->setReferenceIndexUpdater($this->referenceIndexUpdater);
return $relationHandler;
}
......
......@@ -23,6 +23,7 @@ use TYPO3\CMS\Core\Database\Query\Restriction\BackendWorkspaceRestriction;
use TYPO3\CMS\Core\Database\Query\Restriction\DeletedRestriction;
use TYPO3\CMS\Core\Database\RelationHandler;
use TYPO3\CMS\Core\DataHandling\DataHandler;
use TYPO3\CMS\Core\DataHandling\ReferenceIndexUpdater;
use TYPO3\CMS\Core\Localization\LanguageService;
use TYPO3\CMS\Core\Utility\GeneralUtility;
use TYPO3\CMS\Core\Utility\MathUtility;
......@@ -44,6 +45,8 @@ use TYPO3\CMS\Core\Utility\StringUtility;
* Namings in this class:
* + forTableName, forId always refers to dependencies data is provided *for*
* + fromTableName, fromId always refers to ancestors data is retrieved *from*
*
* @internal should only be used by the TYPO3 Core
*/
class DataMapProcessor
{
......@@ -67,6 +70,11 @@ class DataMapProcessor
*/
protected $backendUser;
/**
* @var ReferenceIndexUpdater
*/
protected $referenceIndexUpdater;
/**
* @var DataMapItem[]
*/
......@@ -82,26 +90,39 @@ class DataMapProcessor
*
* @param array $dataMap The submitted data-map to be worked on
* @param BackendUserAuthentication $backendUser Forwarded backend-user scope
* @param ReferenceIndexUpdater|null $referenceIndexUpdater Forward reference index updater to sub DataHandler instances
* @return DataMapProcessor
*/
public static function instance(array $dataMap, BackendUserAuthentication $backendUser)
{
public static function instance(
array $dataMap,
BackendUserAuthentication $backendUser,
ReferenceIndexUpdater $referenceIndexUpdater = null
) {
return GeneralUtility::makeInstance(
static::class,
$dataMap,
$backendUser
$backendUser,
$referenceIndexUpdater
);
}
/**
* @param array $dataMap The submitted data-map to be worked on
* @param BackendUserAuthentication $backendUser Forwarded backend-user scope
* @param ReferenceIndexUpdater|null $referenceIndexUpdater Forward reference index updater to sub DataHandler instances
*/
public function __construct(array $dataMap, BackendUserAuthentication $backendUser)
{
public function __construct(
array $dataMap,
BackendUserAuthentication $backendUser,
ReferenceIndexUpdater $referenceIndexUpdater = null
) {
$this->allDataMap = $dataMap;
$this->modifiedDataMap = $dataMap;
$this->backendUser = $backendUser;
if ($referenceIndexUpdater === null) {
$referenceIndexUpdater = GeneralUtility::makeInstance(ReferenceIndexUpdater::class);
}
$this->referenceIndexUpdater = $referenceIndexUpdater;
}
/**
......@@ -614,7 +635,7 @@ class DataMapProcessor
}
// execute copy, localize and delete actions on persisted child records
if (!empty($localCommandMap)) {
$localDataHandler = GeneralUtility::makeInstance(DataHandler::class);
$localDataHandler = GeneralUtility::makeInstance(DataHandler::class, $this->referenceIndexUpdater);
$localDataHandler->start([], $localCommandMap, $this->backendUser);
$localDataHandler->process_cmdmap();
// update copied or localized ids
......
<?php
declare(strict_types=1);
/*
* This file is part of the TYPO3 CMS project.
*
* It is free software; you can redistribute it and/or modify it under
* the terms of the GNU General Public License, either version 2
* of the License, or any later version.
*
* For the full copyright and license information, please read the
* LICENSE.txt file that was distributed with this source code.
*
* The TYPO3 project - inspiring people to share!
*/
namespace TYPO3\CMS\Core\DataHandling;
use TYPO3\CMS\Backend\Utility\BackendUtility;
use TYPO3\CMS\Core\Database\ReferenceIndex;
use TYPO3\CMS\Core\Utility\GeneralUtility;
/**
* Helper class for DataHandler to gather requests for reference index updates
* and perform them in one go after other operations have been done.
* This is used to suppress multiple reference index update calls for the same
* workspace/table/uid combination within one DataHandler main call.
*
* @internal should only be used by the TYPO3 Core
*/
class ReferenceIndexUpdater
{
/**
* [ workspaceId => [ tableName => [ uid ] ] ]
*
* @var array<int, array<string, array<int, int>>>
*/
protected $updateRegistry = [];
/**
* Register a workspace/table/uid row for update
*
* @param string $table Table name
* @param int $uid Record uid
* @param int $workspace Workspace the record lives in
*/
public function registerForUpdate(string $table, int $uid, int $workspace): void
{
if ($workspace && !BackendUtility::isTableWorkspaceEnabled($table)) {
// If a user is in some workspace and changes relations of not workspace aware
// records, the reference index update needs to be performed as if the user
// is in live workspace. This is detected here and the update is registered for live.
$workspace = 0;
}
if (!isset($this->updateRegistry[$workspace][$table])) {
$this->updateRegistry[$workspace][$table] = [];
}
if (!in_array($uid, $this->updateRegistry[$workspace][$table], true)) {
$this->updateRegistry[$workspace][$table][] = $uid;
}
}
/**
* Perform the reference index update operations
*/
public function update(): void
{
$referenceIndex = GeneralUtility::makeInstance(ReferenceIndex::class);
$referenceIndex->enableRuntimeCache();
foreach ($this->updateRegistry as $workspace => $tableArray) {
$referenceIndex->setWorkspaceId($workspace);
foreach ($tableArray as $table => $uidArray) {
foreach ($uidArray as $uid) {
$referenceIndex->updateRefIndexTable($table, $uid);
}
}
}
$this->updateRegistry = [];
}
}
......@@ -21,6 +21,7 @@ use TYPO3\CMS\Core\Database\Query\QueryHelper;
use TYPO3\CMS\Core\Database\Query\Restriction\DeletedRestriction;
use TYPO3\CMS\Core\Database\Query\Restriction\WorkspaceRestriction;
use TYPO3\CMS\Core\DataHandling\PlainDataResolver;
use TYPO3\CMS\Core\DataHandling\ReferenceIndexUpdater;
use TYPO3\CMS\Core\Utility\GeneralUtility;
use TYPO3\CMS\Core\Utility\MathUtility;
use TYPO3\CMS\Core\Versioning\VersionState;
......@@ -182,10 +183,17 @@ class RelationHandler
protected $MM_oppositeUsage;
/**
* If false, reference index is not updated.
*
* @var bool
*/
protected $updateReferenceIndex = true;
/**
* @var ReferenceIndexUpdater|null
*/
protected $referenceIndexUpdater;
/**
* @var bool
*/
......@@ -197,7 +205,7 @@ class RelationHandler
protected $useLiveReferenceIds = true;
/**
* @var int
* @var int|null
*/
protected $workspaceId;
......@@ -218,7 +226,7 @@ class RelationHandler
*
* @return int
*/
public function getWorkspaceId()
public function getWorkspaceId(): int
{
if (!isset($this->workspaceId)) {
$this->workspaceId = (int)$GLOBALS['BE_USER']->workspace;
......@@ -231,11 +239,22 @@ class RelationHandler
*
* @param int $workspaceId
*/
public function setWorkspaceId($workspaceId)
public function setWorkspaceId($workspaceId): void
{
$this->workspaceId = (int)$workspaceId;
}
/**
* Setter to carry the 'deferred' reference index updater registry around.
*
* @param ReferenceIndexUpdater $updater
* @internal Used internally within DataHandler only
*/
public function setReferenceIndexUpdater(ReferenceIndexUpdater $updater): void
{
$this->referenceIndexUpdater = $updater;
}
/**
* Whether item array has been purged in this instance.
*
......@@ -353,6 +372,7 @@ class RelationHandler
* Sets whether the reference index shall be updated.
*
* @param bool $updateReferenceIndex Whether the reference index shall be updated
* @todo: Unused in core, should be removed, use ReferenceIndexUpdater instead
*/
public function setUpdateReferenceIndex($updateReferenceIndex)
{
......@@ -1282,25 +1302,32 @@ class RelationHandler
}
/**
* Update Reference Index (sys_refindex) for a record
* Update Reference Index (sys_refindex) for a record.
* Should be called any almost any update to a record which could affect references inside the record.
* (copied from DataHandler)
* If used from within DataHandler, only registers a row for update for later processing.
*
* @param string $table Table name
* @param int $id Record UID
* @return array Information concerning modifications delivered by \TYPO3\CMS\Core\Database\ReferenceIndex::updateRefIndexTable()
* @param int $uid Record uid
* @return array Result from ReferenceIndex->updateRefIndexTable() updated directly, else empty array
* @internal Should be protected
*/
public function updateRefIndex($table, $id)
public function updateRefIndex($table, $uid): array
{
$statisticsArray = [];
if ($this->updateReferenceIndex) {
/** @var \TYPO3\CMS\Core\Database\ReferenceIndex $refIndexObj */
$refIndexObj = GeneralUtility::makeInstance(ReferenceIndex::class);
if (!$this->updateReferenceIndex) {
return [];
}
if ($this->referenceIndexUpdater) {
// Add to update registry if given
$this->referenceIndexUpdater->registerForUpdate((string)$table, (int)$uid, $this->getWorkspaceId());
$statisticsArray = [];
} else {
// Update reference index directly if enabled
$referenceIndex = GeneralUtility::makeInstance(ReferenceIndex::class);
if (BackendUtility::isTableWorkspaceEnabled($table)) {
$refIndexObj->setWorkspaceId($this->getWorkspaceId());
$referenceIndex->setWorkspaceId($this->getWorkspaceId());
}
$refIndexObj->enableRuntimeCache();
$statisticsArray = $refIndexObj->updateRefIndexTable($table, $id);
$referenceIndex->enableRuntimeCache();
$statisticsArray = $referenceIndex->updateRefIndexTable($table, $uid);
}
return $statisticsArray;
}
......
.. include:: ../../Includes.txt
========================================================
Important: #92356 - DataHandler performance improvements
========================================================
See :issue:`92356`
Description
===========
The core :php:`DataHandler` is the central backend heart of the system to
persist state changes in the database whenever editors change elements.
Some changes have been applied to reduce the database query load performed
by the :php:`DataHandler` and its related classes. Latest changes especially
dropped a number of useless queries and php operations when relations are handled.
Depending on the handled structure, the :php:`DataHandler` executes up to 30% less
queries than before. More improvements continue to happen and will be ported
to v10 if possible.
.. index:: Backend, Database, ext:core
......@@ -429,79 +429,6 @@ class DataHandlerTest extends UnitTestCase
self::assertFalse($subject->process_datamap());
}
/**
* @test
*/
public function processDatamapWhenEditingRecordInWorkspaceCreatesNewRecordInWorkspace()
{
$GLOBALS['TCA'] = [
'pages' => [
'columns' => [],
],
];
/** @var $subject DataHandler|\PHPUnit\Framework\MockObject\MockObject */
$subject = $this->getMockBuilder(DataHandler::class)
->setMethods([
'newlog',
'checkModifyAccessList',
'tableReadOnly',
'checkRecordUpdateAccess',
'recordInfo',
'getCacheManager',
'registerElementsToBeDeleted',
'unsetElementsToBeDeleted',
'resetElementsToBeDeleted'
])
->disableOriginalConstructor()
->getMock();
$subject->bypassWorkspaceRestrictions = false;
$subject->datamap = [
'pages' => [
'1' => [
'header' => 'demo'
]
]
];
$cacheManagerMock = $this->getMockBuilder(CacheManager::class)
->setMethods(['flushCachesInGroupByTags'])
->getMock();
$cacheManagerMock->expects(self::once())->method('flushCachesInGroupByTags')->with('pages', []);
$subject->expects(self::once())->method('getCacheManager')->willReturn($cacheManagerMock);
$subject->expects(self::once())->method('recordInfo')->willReturn(null);
$subject->expects(self::once())->method('checkModifyAccessList')->with('pages')->willReturn(true);
$subject->expects(self::once())->method('tableReadOnly')->with('pages')->willReturn(false);
$subject->expects(self::once())->method('checkRecordUpdateAccess')->willReturn(true);
$subject->expects(self::once())->method('unsetElementsToBeDeleted')->willReturnArgument(0);
/** @var BackendUserAuthentication|\PHPUnit\Framework\MockObject\MockObject $backEndUser */
$backEndUser = $this->createMock(BackendUserAuthentication::class);
$backEndUser->workspace = 1;
$backEndUser->workspaceRec = ['freeze' => false];
$backEndUser->expects(self::once())->method('workspaceAllowAutoCreation')->willReturn(true);
$backEndUser->expects(self::once())->method('workspaceCannotEditRecord')->willReturn(true);
$backEndUser->expects(self::once())->method('recordEditAccessInternals')->with('pages', 1)->willReturn(true);
$subject->BE_USER = $backEndUser;
$createdDataHandler = $this->createMock(DataHandler::class);
$createdDataHandler->expects(self::once())->method('start')->with([], [
'pages' => [
1 => [
'version' => [
'action' => 'new',
'label' => 'Auto-created for WS #1'
]
]
]
]);
$createdDataHandler->expects(self::never())->method('process_datamap');
$createdDataHandler->expects(self::once())->method('process_cmdmap');
GeneralUtility::addInstance(DataHandler::class, $createdDataHandler);
$subject->process_datamap();
}
/**
* @test
*/
......
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