Bug #88033
closedMassive performance degration since symfony/property-info
100%
Description
Offending commit: bcde6c0177aba345b0bd363c4519892a132ef279 for https://review.typo3.org/#/c/Packages/TYPO3.CMS/+/59454/ https://forge.typo3.org/issues/87457
Core master has a massive performance issues since #87457, rendering uncached backend close to unusable, especially if xdebug is loaded at the same time.
This is obvious if calling backend after clearing caches, and if using the install tool that runs uncached all the time.
with xdebug:- install tool first request without patch: 318ms
- install tool first request with patch: 1990ms (factor >6)
- extension manager with empty caches, without patch: 2800ms
- extension manager with empty caches, with patch: 21270ms (thats 20 seconds, factor >7 !!!)
- install tool first request without patch: 41ms
- install tool first request with patch: 123ms (factor ~2.5)
- extension manager with empty caches, without patch: 550ms
- extension manager with empty caches, with patch: 1400ms (factor ~2.5)
Updated by Christian Kuhn over 5 years ago
- Related to Feature #87457: Use symfony/property-info to gather doc block information added
Updated by Alexander Schnitzler over 5 years ago
A few things to mention here:
1) Yes, first hit is slower, especially since we reverted a patch (don't know which one exactly right now), since all class properties are analyzed again and not just a subset. This can be fixed with more work in the validation area.
2) There is a plan to create a cache warmup command. It's part of the DI-patch.
3) I can't take care right now and not until middle of may. Let's discuss this in a few weeks, please.
Updated by Alexander Schnitzler over 5 years ago
Just a few words on what happened here:
symfony/property-info had been introduced as it offers a quite slim approach to detect types of properties, even non-fully-qualified ones. It's a slim approach compared to other approaches I tried in the months before, namely nikic/php-parser and phpdocumentor/reflection-docblock.
symfony/property-info also uses the extractor of phpdocumentor/reflection-docblock, but spares a lot of cpu cycles through caching and other things. The details are not relevant at this point.
However, in the first approach, not all properties of classes had been analysed but only those that need the support for the detection of non-fully-qualified class names.
Those properties are:
- Injection properties
- Relation properties
Due to that circumstance we enabled the type extraction only for models and properties of classes that had an inject annotation. Unfortunately this broke the validation resolver which needs to know the type of properties for recursive validation. So, we changed the ClassSchema to analyse all properties of all classes.
To solve this I propose the following changes:
- Reduce the lookup of types in ClassSchema on the mentioned properties (injection, relation)
- Do a separate reflection of classes in the ValidatorResolver that determines the types of properties.
I would still like to find a proper solution that enables us to do a proper reflection on everything with a small footprint but I don't see this happening at this moment.
As validation doesn't take place that offen compared to other requests and as I think that validation may take a bit longer than other requests, I think doing a separate reflection in the ValidatorResolver is acceptable. This contradicts my approach of having all reflection in one place but it would be a sane thing to do imo.
Updated by Gerrit Code Review over 5 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 https://review.typo3.org/c/Packages/TYPO3.CMS/+/61076
Updated by Gerrit Code Review over 5 years ago
Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/61076
Updated by Gerrit Code Review over 5 years ago
Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/61076
Updated by Gerrit Code Review over 5 years ago
Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/61076
Updated by Alexander Schnitzler over 5 years ago
btw: I created an issue in symfony/symfony that describes that the behaviour we experience is a bug: https://github.com/symfony/symfony/issues/32073
The bug is being dealt with in this PR: https://github.com/symfony/symfony/pull/32188
Updated by Anonymous over 5 years ago
- Status changed from Under Review to Resolved
- % Done changed from 0 to 100
Applied in changeset e4cc43a4c5ec21404bfc2eb69da8c640ff71cca8.
Updated by Benni Mack almost 5 years ago
- Status changed from Resolved to Closed