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

Added by Alexander Stehlik about 6 years ago. Updated about 2 years ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
Performance
Target version:
Start date:
2013-11-25
Due date:
% Done:

100%

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

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

Related to TYPO3 Core - Bug #53970: getBaseValidatorConjunction is called for simple types Closed 2013-11-26
Related to TYPO3 Core - Task #56086: Impexp functional tests do not fail on wrong assertion Closed 2014-02-18

Associated revisions

Revision 67c4204d (diff)
Added by Alexander Stehlik almost 6 years ago

[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 about 6 years ago

  • Parent task set to #52949

#2 Updated by Gerrit Code Review about 6 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/25679

#3 Updated by Philipp Gampe about 6 years ago

We should of course cache negative requests, but this sounds more like a bug in extbase?!?

#4 Updated by Alexander Stehlik about 6 years ago

I guess you could consider this as a bug. I opened #53970 for this :)

#5 Updated by Thomas Maroschik almost 6 years 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 almost 6 years 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 almost 6 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/25679

#8 Updated by Gerrit Code Review almost 6 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/25679

#9 Updated by Alexander Stehlik almost 6 years ago

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

#10 Updated by Riccardo De Contardi about 2 years ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF