[TYPO3-core] RFC 17978 -.... 30% performance drop without discussion

[Helmut Hummel]

On 05.02.13 17:18, Tolleiv Nietsch wrote:

Hi!

> Jigal van Hemert schrieb:
>>
>> Note: the RM agreed with this patch and it's only a single step of a
>> larger path.
>>
>
> It's step back actually and a significant one and I wonder why this has
> to be pushed through before actual improvements with the cleaner
> approach can be shown... that's what we have sandbox-repos for.

Let me start with explaining what is wrong with the current situation, 
before I will argue, why I would take a slightly different approach to 
improve it.

1. ext_tables.php is a PHP file.

Because of that, extension authors put all sorts of plain PHP code in 
there to "solve" their problems. The problem with that is, that it 
impossible to add a caching layer for that.

2. ext_tables.php does not only to add/ modify TCA

Unlike the name of file suggests, the core itself uses ext_tables.php 
for all kind of stuff not related to TCA but also to add/ modify al 
sorts of global arrays (TBE_STYLES, PAGES_TYPES, TBE_MODULES, TCA_DESCR, 
FILEICONS, and probably more).

3. TCA and especially loadTCA is not designed to work in the frontend

TCA is mainly designed and required for the backend to show correct 
input forms for editors. But since TCA also defines relations and 
validation, it can be useful for frontend extensions as well. In fact 
extbase requires TCA for it's ORM.

One major problem is the way the full TCA for one table is compiled.
It consists of two parts, the control section (TCA[table][ctrl]) and the 
columns section (TCA[table][columns])

By default only the control section is defined in ext_tables.php, but in 
this section one property (TCA[table][ctrl][dynamicConfigFile]) defines 
in which file the colums section is located.

This means, if you want to modify or add colum configuration, in your 
extension ext_tables.php, you must first load the default columns 
definitions (loadTCA(table)) into memory to not end up with corrupted TCA.

This works quite OK in the backend, because all ext_tables.php are 
included for every request.

It does not work in FE because the TCA control section is cached and 
once the cache entry is found ext_tables.php files are not included any 
more. The consequence is, that even by calling loadTCA(table) you will 
not get the TCA column changes or additions from extensions any more, 
but only the ones found in the "dynamicConfigFile"

This leads to the next problem:

4. "compressed" TCA in frontend context

If a frontend request does not find a cached TCA, it includes all 
ext_tables.php files, leading to a complete TCA columns section in this 
request, making every extension relying on it work fine.

Unfortunatly this full TCA is "compressed", which means, that (almost) 
the complete colmuns section is thrown away and only the remaining 
control section is cached. This is done to "save space"

In the next request to this page, compressed TCA is loaded from cache 
making it impossible for (uncached) extensions to get the full TCA. If 
this is needed for the extension to work, it will fail during the second 
request.

Since it is not realy visible when a cache entry is fetched and when 
not, you will end up in debugging hell if you are not aware of this problem.


OK, so far the story of what is wrong. Now what has been done in the 
merged change to tackle these problems.

1. Get rid of *all* loadTCA calls.

Forgetting such calls will lead to corrupted TCA, having the call does 
not gurantee to have a full TCA (in FE).

Removing them altogether and always have a complete TCA is a good thing!


2. Always load the TCA columns sections after including every ext_tables.php

This adds a performance penalty for every backend request, as now *all* 
files with the TCA columns secions are included, even if they are "not 
needed" for particular requests.

The problem I have with that is not the performance penalty, but not 
deprecating "dynamicConfigFile" at the same time, as this never worked 
as expected, is now in fact obsolete but still part of the specification.

Doing so in a second step, is quite easy but we should do this as soon 
as possible.


3. Always load ext_tables.php in BE *and* FE

This is what causes the big performance penalty for cached frontend 
pages, because even in full cached context *all* ext_tables.php files 
(or the one big combined one) are included.

That is what makes me worry the most.
Performance is only one thing that gets worse with this change.

As mentioned above, a lot of things are set up in ext_tables.php (even 
by the core) which is not required at all in FE context at all.

Additionally we even encourage developers to put PHP code in there, as 
it will now will be reliably executed in backend *and* frontend context.

While this is more consistent, it goes into a wrong direction. We need 
to restrict these files to configuration or at least to clearly defined 
API calls not loosen up restrictions. If we would release a TYPO3 
versions in this state, we would have even more troubles to maintain 
backwards compatibility when trying to tighten things up again.

What I would do as followup here, is to cache the *full* TCA after a 
first request which included all ext_tables.php and not include 
ext_tables.php any more after that in frontend context.

The performance impact will be gone again, we will have a full 
consistent TCA in FE and no need for loadTCA.

Of course this is a bit inconsistent as the behaviour differs in BE and 
FE context, but this is outweight by the benefits to do so.


Summary, next steps:
====================

Overall the change is good, if we do not leave it like it is.

We need to:

1. Deprecate dynamicConfigFile and remove the usage in the core
2. Cache full TCA in FE
3. Seperate TCA configuration from ext_tables.php and deprecate defining 
TCA there
4. Cache full TCA in all contexts
5. Move from PHP to a configuration notation for TCA (with a good 
concept of extending/ overriding this configuration)
6. Move all configuration from ext_tables.php to a configuration 
notation and deprecate ext_tables.php altogether


I think 1. - 2. are mandatory for TYPO3 6.1 and 3. - 4. absolutely doable.

5. - 6. Should be left for future versions


Kind regards,
Helmut

-- 
Helmut Hummel
Release Manager TYPO3 6.0
TYPO3 Core Developer, TYPO3 Security Team Member

TYPO3 .... inspiring people to share!
Get involved: typo3.org