[TASK] Simplify PackageManager caching 82/58282/4
authorBenjamin Franzke <bfr@qbus.de>
Sat, 15 Sep 2018 07:31:32 +0000 (09:31 +0200)
committerSusanne Moog <susanne.moog@typo3.org>
Tue, 18 Sep 2018 06:44:17 +0000 (08:44 +0200)
commitbae520c6b2c791a9205deaa06e86c496f1cb91a6
treec5768628203f4ed8e0e47929a1a3a17b943ff86e
parent6a6fe299caa4ebb77711e4ca3565ffeb801910e5
[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 <no-reply@typo3.com>
Reviewed-by: Benni Mack <benni@typo3.org>
Tested-by: Benni Mack <benni@typo3.org>
Reviewed-by: Susanne Moog <susanne.moog@typo3.org>
Tested-by: Susanne Moog <susanne.moog@typo3.org>
typo3/sysext/core/Classes/Package/Package.php
typo3/sysext/core/Classes/Package/PackageManager.php