Skip to content

Commit

Permalink
!!!TASK: Reduce complexity of ReflectionService
Browse files Browse the repository at this point in the history 
This change tackles some problems within the reflection service that
stem from historically increasing complexity due to various caching
mechanisms depending on application context and compile time status.

The aim was to cut down on this complexity, while ensuring that all
existing use-cases continue working as intended.

This ultimately also fixes issue neos#3402 by providing the same
reflection data across all possible contexts.

A few features and caches got deprecated with this change and
could be breaking in the rare case you used the freeze package
api in your code:

The entire concept of freezing a package is deprecated

What remains are the commands in the package controller, which are now
all no-ops and deprecated to be removed with 9.0. This is to ensure
deployment pipelines possibly calling freeze commands do not break
with the 8.4 update.

Additionally the single method `PackageManager::isPackageFrozen`
remains, while the rest was removed. None of the methods was ever api
and it seems unlikely that someone used them in user-land code.
`isPackageFrozen` however is at the very least used in Framework and
Neos code and therefore remains until 9.0, but will now return false
for every package.

Caches deprecated and unused

With the simplification two caches are no longer needed, both are
still declared so that possibly existing cache configuration in user
projects doesn't error, but both

`Flow_Reflection_Status`

and

`Flow_Reflection_CompiletimeData`

will no longer be used and any content can be removed.

The only reflection cache is now `Flow_Reflection_RuntimeData`, which
makes the name somewhat deceptive as it is also used in compile time.
To avoid backwards compatibility issues however it makes sense to keep
the name for the foreseeable future.

Quick performance comparisons suggest that especially the initial
compile from empty cache benefits from this change. Reflection updates
in Development context afterwards seem to be on par with the existing
code base.
  • Loading branch information
@kitsunet
kitsunet committed Feb 7, 2025
1 parent 1bdf165 commit 10b5103
Show file tree
Hide file tree
Showing 14 changed files with 103 additions and 664 deletions.
4 changes: 1 addition & 3 deletions Neos.Flow/Classes/Aop/Builder/ProxyClassBuilder.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -441,10 +441,8 @@ public function buildProxyClass(string $targetClassName, array $aspectContainers
* @param ClassNameIndex $targetClassNameCandidates target class names for advices
* @param ClassNameIndex $treatedSubClasses Already treated (sub) classes to avoid duplication
* @return ClassNameIndex The new collection of already treated classes
* @throws ClassLoadingForReflectionFailedException
* @throws \ReflectionException
* @throws InvalidClassException
* @throws CannotBuildObjectException
* @throws \ReflectionException
*/
protected function proxySubClassesOfClassToEnsureAdvices(string $className, ClassNameIndex $targetClassNameCandidates, ClassNameIndex $treatedSubClasses): ClassNameIndex
{
Expand Down
5 changes: 3 additions & 2 deletions Neos.Flow/Classes/Cache/CacheManager.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -374,9 +374,10 @@ protected function flushClassCachesByChangedFiles(array $changedFiles): void
$flushDoctrineProxyCache = false;
$flushPolicyCache = false;
if (count($modifiedClassNamesWithUnderscores) > 0) {
$reflectionStatusCache = $this->getCache('Flow_Reflection_Status');
$reflectionDataRuntimeCache = $this->getCache('Flow_Reflection_RuntimeData');
foreach (array_keys($modifiedClassNamesWithUnderscores) as $classNameWithUnderscores) {
$reflectionStatusCache->remove($classNameWithUnderscores);
$this->logger->debug('File change detected, removing reflection for ' . $classNameWithUnderscores);
$reflectionDataRuntimeCache->remove($classNameWithUnderscores);
if ($flushDoctrineProxyCache === false && preg_match('/_Domain_Model_(.+)/', $classNameWithUnderscores) === 1) {
$flushDoctrineProxyCache = true;
}
Expand Down
20 changes: 0 additions & 20 deletions Neos.Flow/Classes/Command/CacheCommandController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,6 @@ public function injectEnvironment(Environment $environment)
* from running, the removal of any temporary data can be forced by specifying
* the option <b>--force</b>.
*
* This command does not remove the precompiled data provided by frozen
* packages unless the <b>--force</b> option is used.
*
* @param boolean $force Force flushing of any temporary data
* @return void
* @see neos.flow:cache:warmup
Expand All @@ -157,23 +154,6 @@ public function flushCommand(bool $force = false)
$this->lockManager->unlockSite();
}

$frozenPackages = [];
foreach (array_keys($this->packageManager->getAvailablePackages()) as $packageKey) {
if ($this->packageManager->isPackageFrozen($packageKey)) {
$frozenPackages[] = $packageKey;
}
}
if ($frozenPackages !== []) {
$this->outputFormatted(PHP_EOL . 'Please note that the following package' . (count($frozenPackages) === 1 ? ' is' : 's are') . ' currently frozen: ' . PHP_EOL);
$this->outputFormatted(implode(PHP_EOL, $frozenPackages) . PHP_EOL, [], 2);

$message = 'As code and configuration changes in these packages are not detected, the application may respond ';
$message .= 'unexpectedly if modifications were done anyway or the remaining code relies on these changes.' . PHP_EOL . PHP_EOL;
$message .= 'You may call <b>package:refreeze all</b> in order to refresh frozen packages or use the <b>--force</b> ';
$message .= 'option of this <b>cache:flush</b> command to flush caches if Flow becomes unresponsive.' . PHP_EOL;
$this->outputFormatted($message, [$frozenPackages]);
}

$this->sendAndExit(0);
}

Expand Down
164 changes: 7 additions & 157 deletions Neos.Flow/Classes/Command/PackageCommandController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,25 +32,6 @@ class PackageCommandController extends CommandController
*/
protected $packageManager;

/**
* @var array
*/
protected $settings;

/**
* @var Bootstrap
*/
protected $bootstrap;

/**
* @param array $settings The Flow settings
* @return void
*/
public function injectSettings(array $settings)
{
$this->settings = $settings;
}

/**
* @param PackageManager $packageManager
* @return void
Expand All @@ -60,15 +41,6 @@ public function injectPackageManager(PackageManager $packageManager)
$this->packageManager = $packageManager;
}

/**
* @param Bootstrap $bootstrap
* @return void
*/
public function injectBootstrap(Bootstrap $bootstrap)
{
$this->bootstrap = $bootstrap;
}

/**
* Create a new package
*
Expand Down Expand Up @@ -112,9 +84,7 @@ public function createCommand(string $packageKey, string $packageType = PackageI
public function listCommand(bool $loadingOrder = false)
{
$availablePackages = [];
$frozenPackages = [];
$longestPackageKey = 0;
$freezeSupported = $this->bootstrap->getContext()->isDevelopment();

foreach ($this->packageManager->getAvailablePackages() as $packageKey => $package) {
if (strlen($packageKey) > $longestPackageKey) {
Expand All @@ -123,9 +93,6 @@ public function listCommand(bool $loadingOrder = false)

$availablePackages[$packageKey] = $package;

if ($this->packageManager->isPackageFrozen($packageKey)) {
$frozenPackages[$packageKey] = $package;
}
}

if ($loadingOrder === false) {
Expand All @@ -135,13 +102,7 @@ public function listCommand(bool $loadingOrder = false)
$this->outputLine('PACKAGES:');
/** @var PackageInterface|PackageKeyAwareInterface $package */
foreach ($availablePackages as $package) {
$frozenState = ($freezeSupported && isset($frozenPackages[$package->getPackageKey()]) ? '* ' : ' ');
$this->outputLine(' ' . str_pad($package->getPackageKey(), $longestPackageKey + 3) . $frozenState . str_pad($package->getInstalledVersion(), 15));
}

if (count($frozenPackages) > 0 && $freezeSupported) {
$this->outputLine();
$this->outputLine(' * frozen package');
$this->outputLine(' ' . str_pad($package->getPackageKey(), $longestPackageKey + 3) . str_pad($package->getInstalledVersion(), 15));
}
}

Expand All @@ -165,47 +126,11 @@ public function listCommand(bool $loadingOrder = false)
* @return void
* @see neos.flow:package:unfreeze
* @see neos.flow:package:refreeze
* @deprecated since 8.4
*/
public function freezeCommand(string $packageKey = 'all')
{
if (!$this->bootstrap->getContext()->isDevelopment()) {
$this->outputLine('Package freezing is only supported in Development context.');
$this->quit(3);
}

$packagesToFreeze = [];

if ($packageKey === 'all') {
foreach (array_keys($this->packageManager->getAvailablePackages()) as $packageKey) {
if (!$this->packageManager->isPackageFrozen($packageKey)) {
$packagesToFreeze[] = $packageKey;
}
}
if ($packagesToFreeze === []) {
$this->outputLine('Nothing to do, all packages were already frozen.');
$this->quit(0);
}
} elseif ($packageKey === 'blackberry') {
$this->outputLine('http://bit.ly/freeze-blackberry');
$this->quit(42);
} else {
if (!$this->packageManager->isPackageAvailable($packageKey)) {
$this->outputLine('Package "%s" is not available.', [$packageKey]);
$this->quit(2);
}

if ($this->packageManager->isPackageFrozen($packageKey)) {
$this->outputLine('Package "%s" was already frozen.', [$packageKey]);
$this->quit(0);
}

$packagesToFreeze = [$packageKey];
}

foreach ($packagesToFreeze as $packageKey) {
$this->packageManager->freezePackage($packageKey);
$this->outputLine('Froze package "%s".', [$packageKey]);
}
$this->outputLine('Package freezing is no longer supported, this command is deprecated and will be removed with 9.0.');
}

/**
Expand All @@ -222,47 +147,11 @@ public function freezeCommand(string $packageKey = 'all')
* @return void
* @see neos.flow:package:freeze
* @see neos.flow:cache:flush
* @deprecated since 8.4
*/
public function unfreezeCommand(string $packageKey = 'all')
{
if (!$this->bootstrap->getContext()->isDevelopment()) {
$this->outputLine('Package freezing is only supported in Development context.');
$this->quit(3);
}

$packagesToUnfreeze = [];

if ($packageKey === 'all') {
foreach (array_keys($this->packageManager->getAvailablePackages()) as $packageKey) {
if ($this->packageManager->isPackageFrozen($packageKey)) {
$packagesToUnfreeze[] = $packageKey;
}
}
if ($packagesToUnfreeze === []) {
$this->outputLine('Nothing to do, no packages were frozen.');
$this->quit(0);
}
} else {
if ($packageKey === null) {
$this->outputLine('You must specify a package to unfreeze.');
$this->quit(1);
}

if (!$this->packageManager->isPackageAvailable($packageKey)) {
$this->outputLine('Package "%s" is not available.', [$packageKey]);
$this->quit(2);
}
if (!$this->packageManager->isPackageFrozen($packageKey)) {
$this->outputLine('Package "%s" was not frozen.', [$packageKey]);
$this->quit(0);
}
$packagesToUnfreeze = [$packageKey];
}

foreach ($packagesToUnfreeze as $packageKey) {
$this->packageManager->unfreezePackage($packageKey);
$this->outputLine('Unfroze package "%s".', [$packageKey]);
}
$this->outputLine('Package freezing is no longer supported, this command is deprecated and will be removed with 9.0.');
}

/**
Expand All @@ -280,50 +169,11 @@ public function unfreezeCommand(string $packageKey = 'all')
* @return void
* @see neos.flow:package:freeze
* @see neos.flow:cache:flush
* @deprecated since 8.4
*/
public function refreezeCommand(string $packageKey = 'all')
{
if (!$this->bootstrap->getContext()->isDevelopment()) {
$this->outputLine('Package freezing is only supported in Development context.');
$this->quit(3);
}

$packagesToRefreeze = [];

if ($packageKey === 'all') {
foreach (array_keys($this->packageManager->getAvailablePackages()) as $packageKey) {
if ($this->packageManager->isPackageFrozen($packageKey)) {
$packagesToRefreeze[] = $packageKey;
}
}
if ($packagesToRefreeze === []) {
$this->outputLine('Nothing to do, no packages were frozen.');
$this->quit(0);
}
} else {
if ($packageKey === null) {
$this->outputLine('You must specify a package to refreeze.');
$this->quit(1);
}

if (!$this->packageManager->isPackageAvailable($packageKey)) {
$this->outputLine('Package "%s" is not available.', [$packageKey]);
$this->quit(2);
}
if (!$this->packageManager->isPackageFrozen($packageKey)) {
$this->outputLine('Package "%s" was not frozen.', [$packageKey]);
$this->quit(0);
}
$packagesToRefreeze = [$packageKey];
}

foreach ($packagesToRefreeze as $packageKey) {
$this->packageManager->refreezePackage($packageKey);
$this->outputLine('Refroze package "%s".', [$packageKey]);
}

Scripts::executeCommand('neos.flow:cache:flush', $this->settings, false);
$this->sendAndExit(0);
$this->outputLine('Package freezing is no longer supported, this command is deprecated and will be removed with 9.0.');
}

/**
Expand Down
6 changes: 1 addition & 5 deletions Neos.Flow/Classes/Core/Booting/Scripts.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ public static function initializeSystemLogger(Bootstrap $bootstrap): void
$throwableStorage = self::initializeExceptionStorage($bootstrap, $settings);
$bootstrap->setEarlyInstance(ThrowableStorageInterface::class, $throwableStorage);

/** @var PsrLoggerFactoryInterface $psrLoggerFactoryName */
/** @var class-string $psrLoggerFactoryName */
$psrLoggerFactoryName = $settings['log']['psr3']['loggerFactory'];
$psrLogConfigurations = $settings['log']['psr3'][$psrLoggerFactoryName] ?? [];
$psrLogFactory = $psrLoggerFactoryName::create($psrLogConfigurations);
Expand Down Expand Up @@ -578,10 +578,6 @@ public static function initializeSystemFileMonitor(Bootstrap $bootstrap)

/** @var FlowPackageInterface $package */
foreach ($packageManager->getFlowPackages() as $packageKey => $package) {
if ($packageManager->isPackageFrozen($packageKey)) {
continue;
}

self::monitorDirectoryIfItExists($fileMonitors['Flow_ConfigurationFiles'], $package->getConfigurationPath(), '\.y(a)?ml$');
self::monitorDirectoryIfItExists($fileMonitors['Flow_TranslationFiles'], $package->getResourcesPath() . 'Private/Translations/', '\.xlf');

Expand Down
6 changes: 2 additions & 4 deletions Neos.Flow/Classes/Package.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
use Neos\Flow\ObjectManagement\Proxy;
use Neos\Flow\Package\Package as BasePackage;
use Neos\Flow\Package\PackageManager;
use Neos\Flow\Reflection\ReflectionService;
use Neos\Flow\ResourceManagement\ResourceManager;
use Neos\Flow\ResourceManagement\ResourceRepository;
use Neos\Flow\Security\Authentication\AuthenticationProviderManager;
Expand Down Expand Up @@ -88,10 +89,6 @@ public function boot(Core\Bootstrap $bootstrap)
/** @var PackageManager $packageManager */
$packageManager = $bootstrap->getEarlyInstance(Package\PackageManager::class);
foreach ($packageManager->getFlowPackages() as $packageKey => $package) {
if ($packageManager->isPackageFrozen($packageKey)) {
continue;
}

$publicResourcesPath = $package->getResourcesPath() . 'Public/';
if (is_dir($publicResourcesPath)) {
$publicResourcesFileMonitor->monitorDirectory($publicResourcesPath);
Expand Down Expand Up @@ -122,6 +119,7 @@ public function boot(Core\Bootstrap $bootstrap)
$dispatcher->connect(Core\Bootstrap::class, 'bootstrapShuttingDown', ObjectManagement\ObjectManagerInterface::class, 'shutdown');
$dispatcher->connect(Core\Bootstrap::class, 'bootstrapShuttingDown', Configuration\ConfigurationManager::class, 'shutdown');

/** @see ReflectionService::saveToCache() */
$dispatcher->connect(Core\Bootstrap::class, 'bootstrapShuttingDown', Reflection\ReflectionService::class, 'saveToCache');

$dispatcher->connect(Command\CoreCommandController::class, 'finishedCompilationRun', Security\Authorization\Privilege\Method\MethodPrivilegePointcutFilter::class, 'savePolicyCache');
Expand Down
Loading

0 comments on commit 10b5103

Please sign in to comment.