From add867020a39a651b4a3d6636f1d2c137505a732 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Wed, 22 Apr 2020 18:13:47 +0200 Subject: [PATCH] [DI] skip preloading dependencies of non-preloaded services --- .../FrameworkExtension.php | 3 +- .../FrameworkBundle/FrameworkBundle.php | 15 ++- .../Resources/config/annotations.xml | 1 - .../Resources/config/cache_debug.xml | 1 - .../Resources/config/console.xml | 2 - .../Resources/config/routing.xml | 1 - .../Resources/config/serializer.xml | 1 - .../Resources/config/translation.xml | 1 - .../Resources/config/validator.xml | 1 - .../Bundle/FrameworkBundle/composer.json | 1 + .../Resources/config/security.xml | 1 - .../TwigBundle/Resources/config/twig.xml | 1 - .../Compiler/PassConfig.php | 1 + .../Compiler/ResolveNoPreloadPass.php | 100 ++++++++++++++++++ .../Compiler/ResolveNoPreloadPassTest.php | 53 ++++++++++ .../RegisterListenersPass.php | 32 +++++- .../RegisterListenersPassTest.php | 21 ++++ 17 files changed, 220 insertions(+), 16 deletions(-) create mode 100644 src/Symfony/Component/DependencyInjection/Compiler/ResolveNoPreloadPass.php create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveNoPreloadPassTest.php diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index cef82f8ce1..fbbbff86b9 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -435,8 +435,7 @@ class FrameworkExtension extends Extension $container->registerForAutoconfiguration(CacheClearerInterface::class) ->addTag('kernel.cache_clearer'); $container->registerForAutoconfiguration(CacheWarmerInterface::class) - ->addTag('kernel.cache_warmer') - ->addTag('container.no_preload'); + ->addTag('kernel.cache_warmer'); $container->registerForAutoconfiguration(EventSubscriberInterface::class) ->addTag('kernel.event_subscriber'); $container->registerForAutoconfiguration(LocaleAwareInterface::class) diff --git a/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php b/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php index 973c7c6028..8bfe72af54 100644 --- a/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php +++ b/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php @@ -34,6 +34,7 @@ use Symfony\Component\Cache\DependencyInjection\CachePoolClearerPass; use Symfony\Component\Cache\DependencyInjection\CachePoolPass; use Symfony\Component\Cache\DependencyInjection\CachePoolPrunerPass; use Symfony\Component\Config\Resource\ClassExistenceResource; +use Symfony\Component\Console\ConsoleEvents; use Symfony\Component\Console\DependencyInjection\AddConsoleCommandPass; use Symfony\Component\DependencyInjection\Compiler\PassConfig; use Symfony\Component\DependencyInjection\Compiler\RegisterReverseContainerPass; @@ -103,13 +104,21 @@ class FrameworkBundle extends Bundle { parent::build($container); - $hotPathEvents = [ + $registerListenersPass = new RegisterListenersPass(); + $registerListenersPass->setHotPathEvents([ KernelEvents::REQUEST, KernelEvents::CONTROLLER, KernelEvents::CONTROLLER_ARGUMENTS, KernelEvents::RESPONSE, KernelEvents::FINISH_REQUEST, - ]; + ]); + if (class_exists(ConsoleEvents::class)) { + $registerListenersPass->setNoPreloadEvents([ + ConsoleEvents::COMMAND, + ConsoleEvents::TERMINATE, + ConsoleEvents::ERROR, + ]); + } $container->addCompilerPass(new LoggerPass(), PassConfig::TYPE_BEFORE_OPTIMIZATION, -32); $container->addCompilerPass(new RegisterControllerArgumentLocatorsPass()); @@ -118,7 +127,7 @@ class FrameworkBundle extends Bundle $container->addCompilerPass(new ProfilerPass()); // must be registered before removing private services as some might be listeners/subscribers // but as late as possible to get resolved parameters - $container->addCompilerPass((new RegisterListenersPass())->setHotPathEvents($hotPathEvents), PassConfig::TYPE_BEFORE_REMOVING); + $container->addCompilerPass($registerListenersPass, PassConfig::TYPE_BEFORE_REMOVING); $this->addCompilerPassIfExists($container, AddConstraintValidatorsPass::class); $container->addCompilerPass(new AddAnnotationsCachedReaderPass(), PassConfig::TYPE_AFTER_REMOVING, -255); $this->addCompilerPassIfExists($container, AddValidatorInitializersPass::class); diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/annotations.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/annotations.xml index 7eac708e83..0ce6bf6594 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/annotations.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/annotations.xml @@ -34,7 +34,6 @@ - %kernel.cache_dir%/annotations.php #^Symfony\\(?:Component\\HttpKernel\\|Bundle\\FrameworkBundle\\Controller\\(?!.*Controller$))# diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/cache_debug.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/cache_debug.xml index d5a099d7b2..d4a7396c60 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/cache_debug.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/cache_debug.xml @@ -20,7 +20,6 @@ cache.serializer - diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/console.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/console.xml index 3ef3108b45..cbd43ac7a6 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/console.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/console.xml @@ -11,12 +11,10 @@ - - diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/routing.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/routing.xml index e4105a59f4..9c9eec1e15 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/routing.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/routing.xml @@ -101,7 +101,6 @@ - diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/serializer.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/serializer.xml index fefb86a014..ef5ed701ad 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/serializer.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/serializer.xml @@ -113,7 +113,6 @@ %serializer.mapping.cache.file% - diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/translation.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/translation.xml index 4d056a01a3..3c158abb02 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/translation.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/translation.xml @@ -139,7 +139,6 @@ - diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/validator.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/validator.xml index 01c8b36de8..070908f3db 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/validator.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/validator.xml @@ -36,7 +36,6 @@ %validator.mapping.cache.file% - diff --git a/src/Symfony/Bundle/FrameworkBundle/composer.json b/src/Symfony/Bundle/FrameworkBundle/composer.json index 9aca3a8432..c39184c8ce 100644 --- a/src/Symfony/Bundle/FrameworkBundle/composer.json +++ b/src/Symfony/Bundle/FrameworkBundle/composer.json @@ -21,6 +21,7 @@ "symfony/cache": "^4.4|^5.0", "symfony/config": "^5.0", "symfony/dependency-injection": "^5.1", + "symfony/event-dispatcher": "^5.1", "symfony/error-handler": "^4.4.1|^5.0.1", "symfony/http-foundation": "^4.4|^5.0", "symfony/http-kernel": "^5.0", diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml b/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml index 7219210597..28dceee7de 100644 --- a/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml +++ b/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml @@ -222,7 +222,6 @@ - diff --git a/src/Symfony/Bundle/TwigBundle/Resources/config/twig.xml b/src/Symfony/Bundle/TwigBundle/Resources/config/twig.xml index 47603bc4fd..cb30219365 100644 --- a/src/Symfony/Bundle/TwigBundle/Resources/config/twig.xml +++ b/src/Symfony/Bundle/TwigBundle/Resources/config/twig.xml @@ -45,7 +45,6 @@ - diff --git a/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php b/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php index b6d475c770..4586355055 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php @@ -92,6 +92,7 @@ class PassConfig $this->afterRemovingPasses = [[ new CheckExceptionOnInvalidReferenceBehaviorPass(), new ResolveHotPathPass(), + new ResolveNoPreloadPass(), ]]; } diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ResolveNoPreloadPass.php b/src/Symfony/Component/DependencyInjection/Compiler/ResolveNoPreloadPass.php new file mode 100644 index 0000000000..00e17fdd8b --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Compiler/ResolveNoPreloadPass.php @@ -0,0 +1,100 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\DependencyInjection\Compiler; + +use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Definition; +use Symfony\Component\DependencyInjection\Reference; + +/** + * Propagate the "container.no_preload" tag. + * + * @author Nicolas Grekas + */ +class ResolveNoPreloadPass extends AbstractRecursivePass +{ + private const DO_PRELOAD_TAG = '.container.do_preload'; + + private $tagName; + private $resolvedIds = []; + + public function __construct(string $tagName = 'container.no_preload') + { + $this->tagName = $tagName; + } + + /** + * {@inheritdoc} + */ + public function process(ContainerBuilder $container) + { + $this->container = $container; + + try { + foreach ($container->getDefinitions() as $id => $definition) { + if ($definition->isPublic() && !$definition->isPrivate() && !isset($this->resolvedIds[$id])) { + $this->resolvedIds[$id] = true; + $this->processValue($definition, true); + } + } + + foreach ($container->getAliases() as $alias) { + if ($alias->isPublic() && !$alias->isPrivate() && !isset($this->resolvedIds[$id = (string) $alias]) && $container->has($id)) { + $this->resolvedIds[$id] = true; + $this->processValue($container->getDefinition($id), true); + } + } + } finally { + $this->resolvedIds = []; + $this->container = null; + } + + foreach ($container->getDefinitions() as $definition) { + if ($definition->hasTag(self::DO_PRELOAD_TAG)) { + $definition->clearTag(self::DO_PRELOAD_TAG); + } elseif (!$definition->isDeprecated() && !$definition->hasErrors()) { + $definition->addTag($this->tagName); + } + } + } + + /** + * {@inheritdoc} + */ + protected function processValue($value, bool $isRoot = false) + { + if ($value instanceof Reference && ContainerBuilder::IGNORE_ON_UNINITIALIZED_REFERENCE !== $value->getInvalidBehavior() && $this->container->has($id = (string) $value)) { + $definition = $this->container->findDefinition($id); + + if (!isset($this->resolvedIds[$id])) { + $this->resolvedIds[$id] = true; + $this->processValue($definition, true); + } + + return $value; + } + + if (!$value instanceof Definition) { + return parent::processValue($value, $isRoot); + } + + if ($value->hasTag($this->tagName) || $value->isDeprecated() || $value->hasErrors()) { + return $value; + } + + if ($isRoot) { + $value->addTag(self::DO_PRELOAD_TAG); + } + + return parent::processValue($value, $isRoot); + } +} diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveNoPreloadPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveNoPreloadPassTest.php new file mode 100644 index 0000000000..7dbdbafd5a --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveNoPreloadPassTest.php @@ -0,0 +1,53 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\DependencyInjection\Tests\Compiler; + +use PHPUnit\Framework\TestCase; +use Symfony\Component\DependencyInjection\Compiler\ResolveNoPreloadPass; +use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Reference; + +class ResolveNoPreloadPassTest extends TestCase +{ + public function testProcess() + { + $container = new ContainerBuilder(); + + $container->register('entry_point') + ->setPublic(true) + ->addArgument(new Reference('preloaded')) + ->addArgument(new Reference('not_preloaded')); + + $container->register('preloaded') + ->addArgument(new Reference('preloaded_dep')) + ->addArgument(new Reference('common_dep')); + + $container->register('not_preloaded') + ->setPublic(true) + ->addTag('container.no_preload') + ->addArgument(new Reference('not_preloaded_dep')) + ->addArgument(new Reference('common_dep')); + + $container->register('preloaded_dep'); + $container->register('not_preloaded_dep'); + $container->register('common_dep'); + + (new ResolveNoPreloadPass())->process($container); + + $this->assertFalse($container->getDefinition('entry_point')->hasTag('container.no_preload')); + $this->assertFalse($container->getDefinition('preloaded')->hasTag('container.no_preload')); + $this->assertFalse($container->getDefinition('preloaded_dep')->hasTag('container.no_preload')); + $this->assertFalse($container->getDefinition('common_dep')->hasTag('container.no_preload')); + $this->assertTrue($container->getDefinition('not_preloaded')->hasTag('container.no_preload')); + $this->assertTrue($container->getDefinition('not_preloaded_dep')->hasTag('container.no_preload')); + } +} diff --git a/src/Symfony/Component/EventDispatcher/DependencyInjection/RegisterListenersPass.php b/src/Symfony/Component/EventDispatcher/DependencyInjection/RegisterListenersPass.php index 02ca7caa13..5147e573ec 100644 --- a/src/Symfony/Component/EventDispatcher/DependencyInjection/RegisterListenersPass.php +++ b/src/Symfony/Component/EventDispatcher/DependencyInjection/RegisterListenersPass.php @@ -32,6 +32,8 @@ class RegisterListenersPass implements CompilerPassInterface private $hotPathEvents = []; private $hotPathTagName; + private $noPreloadEvents = []; + private $noPreloadTagName; public function __construct(string $dispatcherService = 'event_dispatcher', string $listenerTag = 'kernel.event_listener', string $subscriberTag = 'kernel.event_subscriber', string $eventAliasesParameter = 'event_dispatcher.event_aliases') { @@ -41,7 +43,10 @@ class RegisterListenersPass implements CompilerPassInterface $this->eventAliasesParameter = $eventAliasesParameter; } - public function setHotPathEvents(array $hotPathEvents, $tagName = 'container.hot_path') + /** + * @return $this + */ + public function setHotPathEvents(array $hotPathEvents, string $tagName = 'container.hot_path') { $this->hotPathEvents = array_flip($hotPathEvents); $this->hotPathTagName = $tagName; @@ -49,6 +54,17 @@ class RegisterListenersPass implements CompilerPassInterface return $this; } + /** + * @return $this + */ + public function setNoPreloadEvents(array $noPreloadEvents, string $tagName = 'container.no_preload'): self + { + $this->noPreloadEvents = array_flip($noPreloadEvents); + $this->noPreloadTagName = $tagName; + + return $this; + } + public function process(ContainerBuilder $container) { if (!$container->hasDefinition($this->dispatcherService) && !$container->hasAlias($this->dispatcherService)) { @@ -64,6 +80,8 @@ class RegisterListenersPass implements CompilerPassInterface $globalDispatcherDefinition = $container->findDefinition($this->dispatcherService); foreach ($container->findTaggedServiceIds($this->listenerTag, true) as $id => $events) { + $noPreload = 0; + foreach ($events as $event) { $priority = isset($event['priority']) ? $event['priority'] : 0; @@ -99,8 +117,14 @@ class RegisterListenersPass implements CompilerPassInterface if (isset($this->hotPathEvents[$event['event']])) { $container->getDefinition($id)->addTag($this->hotPathTagName); + } elseif (isset($this->noPreloadEvents[$event['event']])) { + ++$noPreload; } } + + if ($noPreload && \count($events) === $noPreload) { + $container->getDefinition($id)->addTag($this->noPreloadTagName); + } } $extractingDispatcher = new ExtractingEventDispatcher(); @@ -132,6 +156,7 @@ class RegisterListenersPass implements CompilerPassInterface $dispatcherDefinitions = [$globalDispatcherDefinition]; } + $noPreload = 0; ExtractingEventDispatcher::$aliases = $aliases; ExtractingEventDispatcher::$subscriber = $class; $extractingDispatcher->addSubscriber($extractingDispatcher); @@ -143,8 +168,13 @@ class RegisterListenersPass implements CompilerPassInterface if (isset($this->hotPathEvents[$args[0]])) { $container->getDefinition($id)->addTag($this->hotPathTagName); + } elseif (isset($this->noPreloadEvents[$args[0]])) { + ++$noPreload; } } + if ($noPreload && \count($extractingDispatcher->listeners) === $noPreload) { + $container->getDefinition($id)->addTag($this->noPreloadTagName); + } $extractingDispatcher->listeners = []; ExtractingEventDispatcher::$aliases = []; } diff --git a/src/Symfony/Component/EventDispatcher/Tests/DependencyInjection/RegisterListenersPassTest.php b/src/Symfony/Component/EventDispatcher/Tests/DependencyInjection/RegisterListenersPassTest.php index 5252664a9f..16aade0bc0 100644 --- a/src/Symfony/Component/EventDispatcher/Tests/DependencyInjection/RegisterListenersPassTest.php +++ b/src/Symfony/Component/EventDispatcher/Tests/DependencyInjection/RegisterListenersPassTest.php @@ -157,6 +157,27 @@ class RegisterListenersPassTest extends TestCase $this->assertTrue($container->getDefinition('foo')->hasTag('container.hot_path')); } + public function testNoPreloadEvents() + { + $container = new ContainerBuilder(); + + $container->register('foo', SubscriberService::class)->addTag('kernel.event_subscriber', []); + $container->register('bar')->addTag('kernel.event_listener', ['event' => 'cold_event']); + $container->register('baz') + ->addTag('kernel.event_listener', ['event' => 'event']) + ->addTag('kernel.event_listener', ['event' => 'cold_event']); + $container->register('event_dispatcher', 'stdClass'); + + (new RegisterListenersPass()) + ->setHotPathEvents(['event']) + ->setNoPreloadEvents(['cold_event']) + ->process($container); + + $this->assertFalse($container->getDefinition('foo')->hasTag('container.no_preload')); + $this->assertTrue($container->getDefinition('bar')->hasTag('container.no_preload')); + $this->assertFalse($container->getDefinition('baz')->hasTag('container.no_preload')); + } + public function testEventSubscriberUnresolvableClassName() { $this->expectException('InvalidArgumentException');