Project

General

Profile

Actions

Bug #63629

closed

Bug #63692: Memory consumption while bulk inserting

High memory consumption in BackendUtility::getPagesTSconfig while using the DataHandler

Added by Lukas Krieger about 10 years ago. Updated about 6 years ago.

Status:
Closed
Priority:
Must have
Assignee:
-
Category:
Performance
Target version:
Start date:
2014-12-06
Due date:
% Done:

100%

Estimated time:
TYPO3 Version:
6.2
PHP Version:
Tags:
Complexity:
Is Regression:
No
Sprint Focus:

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 2 (0 open2 closed)

Related to TYPO3 Core - Bug #53370: Cache getPagesTSconfig per ID and rootlineClosedJigal van Hemert2013-11-06

Actions
Related to TYPO3 Core - Task #82473: Make use of TYPO3s runtime caches instead of static in-method variablesClosed2017-09-13

Actions
Actions #1

Updated by Gerrit Code Review about 10 years 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

Actions #2

Updated by Gerrit Code Review about 10 years 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

Actions #3

Updated by Gerrit Code Review about 10 years 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

Actions #4

Updated by Gerrit Code Review about 10 years 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

Actions #5

Updated by Stephan Großberndt about 10 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100
Actions #6

Updated by Philipp Gampe about 10 years ago

  • Parent task set to #63692
Actions #7

Updated by Stephan Großberndt almost 10 years 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?

Actions #8

Updated by Alexander Opitz almost 10 years ago

Hi Stephan, thanks for all the work.

Yes, please create a new issue, add a patch and add Oliver Klee as Reviewer.

Actions #9

Updated by Mathias Schreiber almost 10 years ago

  • Parent task deleted (#63692)
Actions #10

Updated by Mathias Schreiber almost 10 years ago

  • Parent task set to #63692
Actions #11

Updated by Christian Kuhn almost 7 years ago

  • Related to Task #82473: Make use of TYPO3s runtime caches instead of static in-method variables added
Actions #12

Updated by Benni Mack about 6 years ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF