Bug #53962
Epic #55070: Workpackages
Epic #55065: WP: Overall System Performance (Backend and Frontend)
Bug #52949: Speed decrease since 4.5
Class loader does not cache non existing classes
| Status: | Resolved | Start date: | 2013-11-25 | |
|---|---|---|---|---|
| Priority: | Should have | Due date: | ||
| Assigned To: | - | % Done: | 100% |
|
| Category: | Performance | Spent time: | - | |
| Target version: | 6.2.0 | |||
| TYPO3 Version: | 6.2 | Is Regression: | No | |
| PHP Version: | 5.4 | Sprint Focus: | ||
| Complexity: |
Description
Just found this one while testing https://review.typo3.org/25673/:
ValidatorResolver->buildBaseValidatorConjunction()
called by
ActionController->initializeActionMethodValidators()
will always trigger the class loader because it calls:
class_exists('string')
which of course is not a class and does not exist.
Therefore it will not be stored in the cache and the whole class loader process starts all over again.
This is done by every request to the page module (by the SysNote hook).
Maybe it would make sense to store information about non existing classes in the cache?
Related issues
Associated revisions
[TASK] Cache non existing classes
To prevent multiple retries when loading information about non existing
classes (e.g. caused by class_exists() calls) the cache is filled with
an empty string when a non existing class is detected.
With this information the class loader can return early and does not
need to run all expensive checks for determining the class file.
Releases: 6.2
Resolves: #53962
Change-Id: I3c7e750bcce8846aaee79ac2d04527be2e31fc16
Reviewed-on: https://review.typo3.org/25679
Reviewed-by: Helmut Hummel
Tested-by: Helmut Hummel
History
#1 Updated by Markus Klein over 1 year ago
- Parent task set to #52949
#2 Updated by Gerrit Code Review over 1 year 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/25679
#3 Updated by Philipp Gampe over 1 year ago
We should of course cache negative requests, but this sounds more like a bug in extbase?!?
#4 Updated by Alexander Stehlik over 1 year ago
I guess you could consider this as a bug. I opened #53970 for this :)
#5 Updated by Thomas Maroschik over 1 year ago
I am strongly against the caching of non existent classes. By doing so you break the class loader chain, if there are more than one class loader present.
Rather class_exists calls to simple types should be first checked via the TypeHandlingUtility.
#6 Updated by Alexander Stehlik over 1 year ago
Which class loader chain do you mean?
Do you mean the native PHP chain when multiple autoloaders are registered? If so I can not see a problem because if the TYPO3 class loader fails the next one in the chain should take over, right?
Or is there a class loading chain inside the TYPO3 class loader? If so it should be possible to cache non existing classes after the chain.
I agree that the TypeHandlingUtility needs to be fixed. And I can not really think of a use case when you would run a lot of class_exist calls of non existing classes. But this does not mean it will not happen.
Do you see other problems besides the loader chain?
#7 Updated by Gerrit Code Review over 1 year 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/25679
#8 Updated by Gerrit Code Review over 1 year 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/25679
#9 Updated by Alexander Stehlik over 1 year ago
- Status changed from Under Review to Resolved
- % Done changed from 0 to 100
Applied in changeset 67c4204dcb7131e1ed2b6aa02c2abdec2aa6ce93.