Bug #82408

Improve TCA cache

Added by Helmut Hummel about 2 years ago. Updated 12 months ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
-
Target version:
-
Start date:
2017-09-09
Due date:
% Done:

100%

TYPO3 Version:
9
PHP Version:
Tags:
Complexity:
Is Regression:
Sprint Focus:

Description

Currently TCA is cached as serialized string using
the core cache, which is in fact a PHP code cache.

This is done hacking around the fact that code cache
always starts with a "

This has two disadvantages. One is this hack is required,
despite other caches are configured with variable frontend
and file backend. Last but not least, an additional
unserialize is needed, where we could just require one
PHP file that returns an array, which can be pretty fast with
an opcode cache enabled

Associated revisions

Revision 4eceaf14 (diff)
Added by Helmut Hummel about 2 years ago

[TASK] Improve TCA cache

Currently TCA is cached as serialized string using
the core cache.

Simplify the cache retrieval by taking advantage of
the code cache by using requireOnce and previously
storing the cache as PHP file that returns an array.
This significantly improves performance
with opcode cache enabled.

The cache identifier is changed to avoid conflicts
with previously stored data.

Resolves: #82408
Releases: master, 8.7
Change-Id: I59210fa800d10c14d21aceb7416ea418988d6ca5
Reviewed-on: https://review.typo3.org/54062
Tested-by: TYPO3com <>
Reviewed-by: Claus Due <>
Reviewed-by: Christian Kuhn <>
Tested-by: Christian Kuhn <>
Reviewed-by: Anja Leichsenring <>
Tested-by: Anja Leichsenring <>
Reviewed-by: Helmut Hummel <>
Tested-by: Helmut Hummel <>

Revision c71d9480 (diff)
Added by Helmut Hummel about 2 years ago

[TASK] Improve TCA cache

Currently TCA is cached as serialized string using
the core cache.

Simplify the cache retrieval by taking advantage of
the code cache by using requireOnce and previously
storing the cache as PHP file that returns an array.
This significantly improves performance
with opcode cache enabled.

The cache identifier is changed to avoid conflicts
with previously stored data.

Resolves: #82408
Releases: master, 8.7
Change-Id: I59210fa800d10c14d21aceb7416ea418988d6ca5
Reviewed-on: https://review.typo3.org/54114
Tested-by: TYPO3com <>
Reviewed-by: Christian Kuhn <>
Tested-by: Christian Kuhn <>

Revision 3d81788b (diff)
Added by Oliver Hader almost 2 years ago

[FOLLOWUP][TASK] Improve TCA cache

Allowed classes in the options of unserialize invocation need to
be injected using the 'allowed_classes' array index - the current
implementation is casted to true which allows all classes.

Related: #82408
Releases: master, 8.7
Change-Id: I68fbd873a5a7057630a48586878c77547b532348
Reviewed-on: https://review.typo3.org/54154
Tested-by: TYPO3com <>
Reviewed-by: Nicole Cordes <>
Tested-by: Nicole Cordes <>
Reviewed-by: Helmut Hummel <>
Tested-by: Helmut Hummel <>

Revision dac827e4 (diff)
Added by Oliver Hader almost 2 years ago

[FOLLOWUP][TASK] Improve TCA cache

Allowed classes in the options of unserialize invocation need to
be injected using the 'allowed_classes' array index - the current
implementation is casted to true which allows all classes.

Related: #82408
Releases: master, 8.7
Change-Id: I68fbd873a5a7057630a48586878c77547b532348
Reviewed-on: https://review.typo3.org/54155
Tested-by: TYPO3com <>
Reviewed-by: Helmut Hummel <>
Tested-by: Helmut Hummel <>

Revision a398a717 (diff)
Added by Nicole Cordes almost 2 years ago

[BUGFIX] Allow multiple calls of ExtensionManagementUtility::loadBaseTca

Although the function itself is marked as private, it can be triggered
twice due to official API in EidUtility::initTCA. As requireOnce returns
true if called multiple times with the same file, this return state has
to be handled as well.

Resolves: #82539
Related: #82408
Releases: 8.7
Change-Id: I1acd519c342e525c06eda44289acdf86c508a898
Reviewed-on: https://review.typo3.org/54201
Tested-by: TYPO3com <>
Reviewed-by: Oliver Hader <>
Reviewed-by: Faton Haliti <>
Tested-by: Faton Haliti <>
Reviewed-by: Wouter Wolters <>
Reviewed-by: Helmut Hummel <>
Tested-by: Helmut Hummel <>

History

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

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

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

#4 Updated by Gerrit Code Review about 2 years ago

Patch set 1 for branch TYPO3_8-7 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/54114

#5 Updated by Helmut Hummel about 2 years ago

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

#6 Updated by Faton Haliti almost 2 years ago

Hii, after this commit I'm having trouble with codecption running all the cept test. If fails in my third Cept test PHP Fatal error: Uncaught TypeError: Argument 2 passed to TYPO3\CMS\Core\Utility\GeneralUtility::setSingletonInstance() must implement interface TYPO3\CMS\Core\SingletonInterface, boolean given, called in

I was wondering if this is happening to others too, or anyone has any idea why it fails after TCA cache improvement.

#7 Updated by Helmut Hummel almost 2 years ago

Faton Haliti wrote:

Hii, after this commit I'm having trouble with codecption running all the cept test. If fails in my third Cept test PHP Fatal error: Uncaught TypeError: Argument 2 passed to TYPO3\CMS\Core\Utility\GeneralUtility::setSingletonInstance() must implement interface TYPO3\CMS\Core\SingletonInterface, boolean given, called in

I was wondering if this is happening to others too, or anyone has any idea why it fails after TCA cache improvement.

What PHP version are you using? Can you try to remove all caches from typo3temp/var/Cache ?

#8 Updated by Faton Haliti almost 2 years ago

Im using PHP v7.1.7 I removed the cache but that did not help

#9 Updated by Helmut Hummel almost 2 years ago

Faton Haliti wrote:

Im using PHP v7.1.7 I removed the cache but that did not help

Can you remove the second argument (['allowed_classes' => [CategoryRegistry::class]]) from the unserialize call and see if this helps?

But honestly I have no clue why it does not work for you. I can't reproduce it.

#10 Updated by Tymoteusz Motylewski about 1 year ago

just FYI,
I did some benchmarks with PHP 7.0 and 7.2 and unserializing an array is much faster than requiring a php file (with opcache enabled).
so the performance argument is not there.

read serialized array from  php file: 2.08
include php file: 9.65

below links to benchmark code and results.
https://gist.github.com/tmotyl/68a3c61e86e595e6f540881ac7c71de3
https://gist.github.com/tmotyl/dccadfd99d80d3c9b02726292598eda5

#11 Updated by Helmut Hummel about 1 year ago

Tymoteusz Motylewski wrote:

just FYI,
I did some benchmarks with PHP 7.0 and 7.2 and unserializing an array is much faster than requiring a php file (with opcache enabled).
so the performance argument is not there.

[...]

below links to benchmark code and results.
https://gist.github.com/tmotyl/68a3c61e86e595e6f540881ac7c71de3
https://gist.github.com/tmotyl/dccadfd99d80d3c9b02726292598eda5

Thanks Tymek for your tests.

I can confirm the result of your tests, however I don't agree with your conclusion.

I stripped down your benchmark file to what I considered important:

Reading a cache file and populating an array from it. No benchmarking for writing the file or different forms of serializing.

What is important to understand is, that if you write a file within one request and then include it in the same request, you bypass opcache. it just never hits the opcache at all.
You also bypass opcache, when the file is updated with every request, as it will then always be recognized as changed (at least in my default config)

Therefor your test scenario does not match our real scenario.
We write the cache in one request.
A second request parses the php cache file and stores the result in (opcache)memory
Every subsequent request will directly pull out the file out of memory without any parsing.

I slightly changed your benchmark file to match our scenario and the result is very different from yours (PHP 7.1, FGI):
https://gist.github.com/helhum/9129ab496d96638dcc1988248b08de46#file-results-md

tldr: with opcache a require of a cached php file returning an array is 170 times faster than unserializing file content.
without opcache unserializing is 4 times faster

I find these results quite logical.
requiring a file with a large array means, php must parse this long file, which takes a while
parsing long php code takes longer than parsing a simple serialized string

however when the result of the parsing is stored in memory (opcache),
then loading the array from the parsed file is as fast a copying memory,
while the serialized string still needs to be parsed in runtime

HTH

#12 Updated by Helmut Hummel about 1 year ago

Helmut Hummel wrote:

Faton Haliti wrote:

Im using PHP v7.1.7 I removed the cache but that did not help

Can you remove the second argument (['allowed_classes' => [CategoryRegistry::class]]) from the unserialize call and see if this helps?

But honestly I have no clue why it does not work for you. I can't reproduce it.

I now ran into the same issue. Problem can occur, when you call the bootstrap code to include tca twice. the second time you will run into this error.
So my advice is to not call load tca twice within one request

#13 Updated by Tymoteusz Motylewski about 1 year ago

Thanks Helmut for the further investigation! That's really interesting to know.

#14 Updated by Benni Mack 12 months ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF