Bug #63629
Bug #63692: Memory consumption while bulk inserting
High memory consumption in BackendUtility::getPagesTSconfig while using the DataHandler
Status: | Resolved | Start date: | 2014-12-06 | |
---|---|---|---|---|
Priority: | Must have | Due date: | ||
Assigned To: | - | % Done: | 100% |
|
Category: | Performance | Spent time: | - | |
Target version: | next-patchlevel | |||
TYPO3 Version: | 6.2 | Is Regression: | No | |
PHP Version: | Sprint Focus: | |||
Complexity: |
Description
There is a memory consumption problem in
TYPO3\CMS\Backend\Utility::getPagesTSconfig
In the Typo3 Forum there is this thread, describing the investigation, a solution for a patch and also testing the patch compared to the original function.
http://forum.typo3.org/index.php/t/206887/
Executive Summary:
If you import pages with the DataHandler into Typo3, the function mentioned above is creating a huge array saving all TSconfigs for every page (http://typo3.org/api/typo3cms/backend_2_classes_2_utility_2_backend_utility_8php_source.html#l01153 line 1197)
if ($useCacheForCurrentPageId) { > $pagesTSconfig_cache[$id] = $TSconfig; }
In my case, there are a lot of different keys in that array but only one unique page TS.
http://forum.typo3.org/index.php?t=tree&th=206887&goto=722134&#msg_722134
On a normal Backend call, the function is caching the pageTS config for only one page. What makes sense because the function is called multiple times BUT only for
one single page!
Therefore the array has everytime a count of 1.
1 pageID => 1 TSconfig
The main problem is that multiple pages can have the same TSconfig, like in my DataHandler import.
Therefore it would be nice to have a n:n relation for caching the TSconfig in this function.
The static cache array
$pagesTSconfig_cache
is in the very first request always empty.
Here is the description of a possible patch:
http://forum.typo3.org/index.php?t=tree&th=206887&goto=722198&#msg_722198
What the patch currently does is following:
1. add a second static array for caching unique TSconfig
2. use the pagesTSconfig_cache as a reference to the unique TSconfig stored in the other array
Here you can find some tests:
http://forum.typo3.org/index.php?t=tree&th=206887&goto=722232&#msg_722232
http://forum.typo3.org/index.php?t=tree&th=206887&goto=722311&#msg_722311 (comparing the original function with the patched one and adding conditions in the backend)
An improvement would be maybe to use the CacheManager for storing the caches globally.
Because the parsed page TSconfig is already cached by "TYPO3\\CMS\\Backend\\Configuration\\TsConfigParser" on line 1186
// Parsing the page TS-Config $pageTS = implode(LF . '[GLOBAL]' . LF, $TSdataArray); /* @var $parseObj \TYPO3\CMS\Backend\Configuration\TsConfigParser */ $parseObj = GeneralUtility::makeInstance('TYPO3\\CMS\\Backend\\Configuration\\TsConfigParser'); $res = $parseObj->parseTSconfig($pageTS, 'PAGES', $id, $rootLine);
Some result:
Task: modifying 280 pages Original Function: The whole process needs 19 seconds and is using 85MB memory Modified Function: The whole process needs also 19 seconds but is only using 14MB memory Task: modifying 150 pages Original Function: 9 seconds, 51.1 MB memory Modified Function: 8.7 seconds, 11MB memory Task: modifying 118 pages Original Function: 7.2 seconds, 41.7 MB memory Modified Function: 6.8 seconds, 10 MB memory Task: modifying 54 pages Original Function: 3.6 seconds , 23MB Modified Function: 3.3 seconds, 8.5 MB memory
The patch will follow on gerrit.
ps.: there are also some mass editing functions in the normal typo3 backend available. This patch could improve the performance if the backend mass editing is also calling this function.
Related issues
Associated revisions
[BUGFIX] Better caching of TSconfig in BackendUtility for bulk inserts
Changes the cache of pageTSconfig from creating a cached entry for each
page to creating a cached entry for each different TSconfig (including
userTSconfig) and a hash-cache for each page instead.
Resolves: #63629
Releases: master, 6.2
Change-Id: I3e4c70cd46cec26dda8574c058d828ec2e22a385
Reviewed-on: http://review.typo3.org/35122
Reviewed-by: Philipp Gampe <philipp.gampe@typo3.org>
Tested-by: Philipp Gampe <philipp.gampe@typo3.org>
Reviewed-by: Stephan Großberndt <stephan@grossberndt.de>
Reviewed-by: Lukas Krieger <lukas.krieger@me.com>
Tested-by: Lukas Krieger <lukas.krieger@me.com>
Reviewed-by: Markus Klein <klein.t3@reelworx.at>
Tested-by: Markus Klein <klein.t3@reelworx.at>
[BUGFIX] Better caching of TSconfig in BackendUtility for bulk inserts
Changes the cache of pageTSconfig from creating a cached entry for each
page to creating a cached entry for each different TSconfig (including
userTSconfig) and a hash-cache for each page instead.
Resolves: #63629
Releases: master, 6.2
Change-Id: I3e4c70cd46cec26dda8574c058d828ec2e22a385
Reviewed-on: http://review.typo3.org/35145
Reviewed-by: Markus Klein <klein.t3@reelworx.at>
Tested-by: Markus Klein <klein.t3@reelworx.at>
History
#1 Updated by Gerrit Code Review 8 months ago
- Status changed from New to Under Review
Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/35122
#2 Updated by Gerrit Code Review 8 months ago
Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/35122
#3 Updated by Gerrit Code Review 8 months ago
Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/35122
#4 Updated by Gerrit Code Review 8 months ago
Patch set 1 for branch TYPO3_6-2 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/35145
#5 Updated by Stephan Großberndt 8 months ago
- Status changed from Under Review to Resolved
- % Done changed from 0 to 100
Applied in changeset d5c8272727d016b51ca916abf575c5aff2bd732d.
#6 Updated by Philipp Gampe 8 months ago
- Parent task set to #63692
#7 Updated by Stephan Großberndt 8 months ago
As Philipp Gampe pointed out in https://review.typo3.org/#/c/35186/1/typo3/sysext/core/Classes/DataHandling/DataHandler.php@1615
"Please let's use a (static?) member here instead of the static local variable. Static local variables are impossible to mock."
That applies to these caches as well. Create another issue since this is merged?
#8 Updated by Alexander Opitz 8 months ago
Hi Stephan, thanks for all the work.
Yes, please create a new issue, add a patch and add Oliver Klee as Reviewer.
#9 Updated by Mathias Schreiber 8 months ago
- Parent task deleted (
#63692)
#10 Updated by Mathias Schreiber 8 months ago
- Parent task set to #63692