feature #36535 [DI] skip preloading dependencies of non-preloaded services (nicolas-grekas)

This PR was merged into the 5.1-dev branch. Discussion ---------- [DI] skip preloading dependencies of non-preloaded services | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - Suggested by @stof on Slack: this improves preloading by propagating the `container.no_preload` tag to services that are referenced only by not-preloaded services. The benefit is double: 1. this fixes potential over-preloading 2. this requires less work from the community: no need to add the tag anymore most of the time As a corollary, listeners of console events are tagged with `container.no_preload` automatically now. Commits ------- add867020a [DI] skip preloading dependencies of non-preloaded services
2020-04-24 10:23:03 +02:00 · 2020-04-24 10:23:03 +02:00 · ac3bd146a3
commit ac3bd146a3
parent 87c9ab4cbe add867020a
17 changed files with 220 additions and 16 deletions
--- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
+++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
@ -436,8 +436,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)
--- 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);
--- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/annotations.xml
+++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/annotations.xml
@ -34,7 +34,6 @@
        </service>

        <service id="annotations.cache_warmer" class="Symfony\Bundle\FrameworkBundle\CacheWarmer\AnnotationsCacheWarmer">
-            <tag name="container.no_preload" />
            <argument type="service" id="annotations.reader" />
            <argument>%kernel.cache_dir%/annotations.php</argument>
            <argument>#^Symfony\\(?:Component\\HttpKernel\\|Bundle\\FrameworkBundle\\Controller\\(?!.*Controller$))#</argument>
--- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/cache_debug.xml
+++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/cache_debug.xml
@ -20,7 +20,6 @@
                <argument>cache.serializer</argument>
            </argument>
            <tag name="kernel.cache_warmer" priority="64" />
-            <tag name="container.no_preload" />
        </service>
    </services>
 </container>
--- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/console.xml
+++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/console.xml
@ -11,12 +11,10 @@
            <argument type="service" id="logger" on-invalid="null" />
            <tag name="kernel.event_subscriber" />
            <tag name="monolog.logger" channel="console" />
-            <tag name="container.no_preload" />
        </service>

        <service id="console.suggest_missing_package_subscriber" class="Symfony\Bundle\FrameworkBundle\EventListener\SuggestMissingPackageSubscriber">
            <tag name="kernel.event_subscriber" />
-            <tag name="container.no_preload" />
        </service>

        <service id="console.command.about" class="Symfony\Bundle\FrameworkBundle\Command\AboutCommand">
--- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/routing.xml
+++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/routing.xml
@ -101,7 +101,6 @@
        <service id="router.cache_warmer" class="Symfony\Bundle\FrameworkBundle\CacheWarmer\RouterCacheWarmer">
            <tag name="container.service_subscriber" id="router" />
            <tag name="kernel.cache_warmer" />
-            <tag name="container.no_preload" />
            <argument type="service" id="Psr\Container\ContainerInterface" />
        </service>

--- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/serializer.xml
+++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/serializer.xml
@ -113,7 +113,6 @@
            <argument type="collection" /><!-- Loaders injected by the extension -->
            <argument>%serializer.mapping.cache.file%</argument>
            <tag name="kernel.cache_warmer" />
-            <tag name="container.no_preload" />
        </service>

        <service id="serializer.mapping.cache.symfony" class="Psr\Cache\CacheItemPoolInterface">
--- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/translation.xml
+++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/translation.xml
@ -139,7 +139,6 @@
        <service id="translation.warmer" class="Symfony\Bundle\FrameworkBundle\CacheWarmer\TranslationsCacheWarmer">
            <tag name="container.service_subscriber" id="translator" />
            <tag name="kernel.cache_warmer" />
-            <tag name="container.no_preload" />
            <argument type="service" id="Psr\Container\ContainerInterface" />
        </service>
    </services>
--- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/validator.xml
+++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/validator.xml
@ -36,7 +36,6 @@
            <argument type="service" id="validator.builder" />
            <argument>%validator.mapping.cache.file%</argument>
            <tag name="kernel.cache_warmer" />
-            <tag name="container.no_preload" />
        </service>

        <service id="validator.mapping.cache.adapter" class="Symfony\Component\Cache\Adapter\PhpArrayAdapter">
--- 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",
--- a/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml
+++ b/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml
@ -213,7 +213,6 @@
        <!-- Cache Warmers -->
        <service id="security.cache_warmer.expression" class="Symfony\Bundle\SecurityBundle\CacheWarmer\ExpressionCacheWarmer">
            <tag name="kernel.cache_warmer" />
-            <tag name="container.no_preload" />
            <argument type="collection" /> <!-- expressions -->
            <argument type="service" id="security.expression_language" />
        </service>
--- a/src/Symfony/Bundle/TwigBundle/Resources/config/twig.xml
+++ b/src/Symfony/Bundle/TwigBundle/Resources/config/twig.xml
@ -45,7 +45,6 @@

        <service id="twig.template_cache_warmer" class="Symfony\Bundle\TwigBundle\CacheWarmer\TemplateCacheWarmer">
            <tag name="kernel.cache_warmer" />
-            <tag name="container.no_preload" />
            <tag name="container.service_subscriber" id="twig" />
            <argument type="service" id="Psr\Container\ContainerInterface" />
            <argument type="service" id="twig.template_iterator" />
--- 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(),
        ]];
    }

--- a/src/Symfony/Component/DependencyInjection/Compiler/ResolveNoPreloadPass.php
+++ b/src/Symfony/Component/DependencyInjection/Compiler/ResolveNoPreloadPass.php
@ -0,0 +1,100 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * 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 <p@tchwork.com>
+ */
+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);
+    }
+}
--- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveNoPreloadPassTest.php
+++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveNoPreloadPassTest.php
@ -0,0 +1,53 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * 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'));
+    }
+}
--- 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 = [];
        }
--- 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');