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

[Michael Stucki]

I agree with Tolleiv.

It is probably fine to keep it now that it was merged, and more
improvements are on the way, but without these improvements, the change
is incomplete.

Besides this, I agree that the rush was not neccessary.

- michael

Am 06.02.2013 20:31, schrieb Tolleiv Nietsch:
> Thanks for your answer - this doesn't answer my question. Why does this
> have to be merged without proper measurements done by multiple people
> and why don't we use a  dedicated branch for these changes until we gain
> back the "original" performance? What's so urgent here?
> 
> I don't criticize that we want to improve the TCA handling - all your
> points are right and valid - but why on earth do we have such a rush in
> this action?
> 
> Cheers,
> Tolleiv
> 
> 
> 
> 
> Helmut Hummel schrieb:
>> 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
>>
> 


-- 
Michael Stucki
TYPO3 Core Team member

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