Task #86261

Epic #83968: PSR-11 Initiative

PackageManager caching is over complicated

Added by Benjamin Franzke about 1 year ago. Updated 4 months ago.

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

100%

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

Description

The PackageManager caches all Package objects.
As a Package object currently requires a reference to the PackageManager, the PackageManager temporarily provides a reference to itself in $GLOBALS['TYPO3_currentPackageManager'] while using unserialize to re-create caches Package objects.

The PackageManager reference should just be dropped.


Related issues

Related to TYPO3 Core - Bug #87175: activating SEO or Redirects core extension breaks the system Closed 2018-12-16

Associated revisions

Revision bae520c6 (diff)
Added by Benjamin Franzke about 1 year ago

[TASK] Simplify PackageManager caching

We do not want to pollute $GLOBALS to be able to inject PackageManager
as $GLOBALS['TYPO3_currentPackageManager']. to cache-restored Package
objects (during unserialize).
Therefore this patch drops the PackageManager dependency from the Package
class.

The global injected PackageManager was used to calculate the
package metadata on demand. Package metadata is a small array
that doesn't harm to be created during Package object construction,
(it's cached afterwards anyway).
This allows us to drop the PackageManager reference, which
simplifies serialize/unserialize logic (we can drop __sleep and __wakeup).

We also combine the PackageManger and PackageObjects cache
into one cache file instead of two. There is no advantage
in having to read two files from the filesystem. The prior argument
(introduced with the initial PackageManager patch in 2013) "so PHP does
not have to parse the serialized string" is actually invalid:
The serialized string has to be parsed anyway.
PHP did not need to parse the var_export'd variant, true.
BUT: The var_export'd variant is theoretically php opcache'able, while
reading a file and passing the file contents to serialize() is not.
That means the previous solution actually hampered native optimizations.

Ideally we could drop the serialize/unserialize thing and just use
var_export for Package objects, but for that to work the Package class
(and all of it's properties) would need to implement the magic
_set_state() method (which is used by var_export to create objects).
That's currently not possible, because the composerManifest (which
is read from json_decode) is a stdClass and does not provide a
_set_state() method. We'd need to rewrite all that code to an array
or so. Maybe something for a later patch.

As a drive-by fix we now hash the TYPO3_version value into the
cacheEntryIdentifier like other core caches do.

Change-Id: I764dc92c64165ede24c8020c44cd2360b3faa00c
Releases: master
Resolves: #86261
Reviewed-on: https://review.typo3.org/58282
Tested-by: TYPO3com <>
Reviewed-by: Benni Mack <>
Tested-by: Benni Mack <>
Reviewed-by: Susanne Moog <>
Tested-by: Susanne Moog <>

History

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

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

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

#4 Updated by Benjamin Franzke about 1 year ago

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

#5 Updated by Benni Mack about 1 year ago

  • Status changed from Resolved to Closed

#6 Updated by Helmut Hummel 10 months ago

  • Related to Bug #87175: activating SEO or Redirects core extension breaks the system added

#7 Updated by Benjamin Franzke 4 months ago

  • Parent task set to #83968

Also available in: Atom PDF