[HttpKernel][DI] allow bundles to declare classes that should be preloaded #33689
Conversation
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
Show resolved
Hide resolved
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
Show resolved
Hide resolved
src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php
Show resolved
Hide resolved
src/Symfony/Component/HttpKernel/DependencyInjection/ClassMatchingTrait.php
Show resolved
Hide resolved
61acb29 to
c539899
Compare
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
Show resolved
Hide resolved
c539899 to
90ba34d
Compare
|
The |
There was a problem hiding this comment.
We also need a way to force some classes to stay out of preloading IMO.
We had this issue in the past with Swiftmailer doing some magic during autoloading to load the swiftmailer initialization code, where hot-loading the file i the compiled container would break things by skipping the autoloader. SwiftmailerBundle could fix that by forcing to remove the hot tag on all its services after the DI component marked services as hot (as marking services as hot is separate from performing optimization for the). But here, I don't see an opt-out system.
| 'Symfony\Component\Cache\Adapter\ApcuAdapter', | ||
| 'Symfony\Component\Cache\Adapter\ArrayAdapter', | ||
| 'Symfony\Component\Cache\Adapter\PhpArrayAdapter', | ||
| 'Symfony\Component\Cache\Adapter\PhpFilesAdapter', |
There was a problem hiding this comment.
Aren't these discovered thanks to services already ?
| 'Symfony\Component\HttpFoundation\ServerBag', | ||
| 'Symfony\Component\HttpKernel\Event\ControllerArgumentsEvent', | ||
| 'Symfony\Component\HttpKernel\Event\ControllerEvent', | ||
| 'Symfony\Component\HttpKernel\Event\TerminateEvent', |
There was a problem hiding this comment.
what about RequestEvent and ResponseEvent ?
| @@ -170,6 +170,18 @@ public function load(array $configs, ContainerBuilder $container) | |||
| $container->removeDefinition('twig.cache_warmer'); | |||
| $container->removeDefinition('twig.template_cache_warmer'); | |||
| } | |||
|
|
|||
| $this->addClassesToPreload([ | |||
| 'Symfony\Component\Stopwatch\Section', | |||
There was a problem hiding this comment.
shouldn't it be added by FrameworkBundle when defining the stopwatch service instead ? Thus, I don't think we use it in prod code.
|
Note that preloading hot services rather than all services would already provide such opt-out. |
|
As preloading might have some overlap with the already existing Will compiled classes get preloaded as well? |
|
I'm closing there because I found a better approach in #36103: inline We could add support for |
This adds new methods to allow apps and bundles to declare classes that should be preloaded (when using PHP 7.4's
opcache.preload).Since #32032, the DI component is already able to generate a preloading script, based on services with the
container.hot_pathtag. Yet, based on my local experiments, not all classes can be discovered using this strategy. Also, it appears that while preloading all classes invendor/is said to be a bad idea by Dmitry itself, not preloading a class has a measurable impact.Thus we should seek to preload all actually used classes. A bit more than that amount is better than a bit less.
As such, this PR now preloads all service classes and adds the infrastructure needed to list more classes when appropriate.
Listing more classes is done either via:
Kernel::getClassesToPreload(), which by default loads everything in the namespace of the app's kernel (usually theApp\namespace) + all bundle classesExtension::addClassesToPreload(), so that each bundle's DI extension can add to the list.FrameworkBundle,SecurityBundle, andTwigBundlealready contain the core they need.This works nice on a skeleton app but segfaults on a website-skeleton for now, so I won't give numbers.
This is nonetheless ready (merging would actually help php-internal to reproduce the segfault.)
PS: this requires dumping the autoloader, i.e.
composer install -o