-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[DependencyInjection] Leverage native lazy objects for lazy services #59913
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| if (!$manager instanceof LazyLoadingInterface) { | ||
| throw new \LogicException(\sprintf('Resetting a non-lazy manager service is not supported. Declare the "%s" service as lazy.', $name)); | ||
| } | ||
| trigger_deprecation('symfony/doctrine-bridge', '7.3', 'Support for proxy-manager is deprecated.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This case should never happen (who would use proxy-manager directly instead of Symfony's lazy services for the Doctrine EM!?), still added for completeness.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't this deprecation always triggered on PHP 8.3 and older ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's guarded by if (!$manager instanceof LazyLoadingInterface) { (note the !) so only when ppl use proxy-manager, which we don't anymore thanks to var-exporter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no. This deprecation is triggered after that if, when we haven't thrown an exception (and so in the case where we will actually reset the object)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, I missed that the case of lazy objects using var-exporter are handled before the check for PHP 8.4, so this is not the code path we reach in modern projects.
855b264 to
02958b9
Compare
02958b9 to
ed695a6
Compare
|
Nice! Could this PR possibly also improve performance? Any idea on how to benchmark that? |
…ve lazy objects (HypeMC) This PR was merged into the 7.3 branch. Discussion ---------- [DoctrineBridge] Fix resetting the manager when using native lazy objects | Q | A | ------------- | --- | Branch? | 7.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | - | License | MIT PR #59913 doesn't seem to work as expected. For example, a simple test like this: ```php class DoctrineTest extends KernelTestCase { public function testManagerRegistryReset(): void { $entityManager = self::getContainer()->get(EntityManagerInterface::class); $entityManager->close(); self::assertFalse($entityManager->isOpen()); $managerRegistry = self::getContainer()->get(ManagerRegistry::class); $managerRegistry->resetManager(); self::assertTrue($entityManager->isOpen()); } } ``` fails with: > There was 1 error: > > 1) App\Tests\DoctrineTest::testManagerRegistryReset > Error: Call to a member function __construct() on false > > /project/var/cache/test/ContainerTVZSKa5/App_KernelTestDebugContainer.php:709 > /project/vendor/symfony/doctrine-bridge/ManagerRegistry.php:91 > /project/vendor/doctrine/orm/src/EntityManager.php:534 > /project/tests/DoctrineTest.php:21 Here's the dumped container for reference: ```php protected static function getDoctrine_Orm_DefaultEntityManagerService($container, $lazyLoad = true) { if (true === $lazyLoad) { return $container->services['doctrine.orm.default_entity_manager'] = new \ReflectionClass('Doctrine\ORM\EntityManager')->newLazyGhost(static function ($proxy) use ($container) { self::getDoctrine_Orm_DefaultEntityManagerService($container, $proxy); }); } include_once \dirname(__DIR__, 4).'/vendor/doctrine/orm/src/Proxy/Autoloader.php'; include_once \dirname(__DIR__, 4).'/vendor/doctrine/persistence/src/Persistence/ObjectManager.php'; include_once \dirname(__DIR__, 4).'/vendor/doctrine/orm/src/EntityManagerInterface.php'; include_once \dirname(__DIR__, 4).'/vendor/doctrine/orm/src/EntityManager.php'; $instance = ($lazyLoad->__construct(($container->services['doctrine.dbal.default_connection'] ?? self::getDoctrine_Dbal_DefaultConnectionService($container)), ($container->privates['doctrine.orm.default_configuration'] ?? self::getDoctrine_Orm_DefaultConfigurationService($container)), ($container->privates['doctrine.dbal.default_connection.event_manager'] ?? self::getDoctrine_Dbal_DefaultConnection_EventManagerService($container))) && false ?: $lazyLoad); ($container->privates['doctrine.orm.default_manager_configurator'] ??= new \Doctrine\Bundle\DoctrineBundle\ManagerConfigurator([], []))->configure($instance); return $instance; } ``` cc `@nicolas`-grekas Commits ------- 5972d98 [DoctrineBridge] Fix resetting the manager when using native lazy objects
Symfony >7.3 generates optimized DI caches for PHP >=8.4 by using `ReflectionClass::newLazyProxy` (and friends) if available [1]. That means a cache that is generated with PHP 8.4 (e.g. CLI context) can not be used in a PHP 8.2 process (e.g. Web Context) and therefore we ensure that the major and minor versions match when reusing a cached DI container. This has a drawback that DI caches can not be warmed using `bin/typo3 cache:warmup` on CLI, when CLI and WEB PHP versions do not share the same major and minor version, but it's better than a broken instance. [1] symfony/symfony#59913 Resolves: #107649 Releases: main, 13.4 Change-Id: Ib27b0f015847fbce81fc42685278c72e8a707aa0 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/90988 Tested-by: Garvin Hicking <garvin@hick.ing> Tested-by: Benni Mack <benni@typo3.org> Reviewed-by: Garvin Hicking <garvin@hick.ing> Tested-by: Nikita Hovratov <nikita.h@live.de> Reviewed-by: Oliver Hader <oliver.hader@typo3.org> Tested-by: core-ci <typo3@b13.com> Reviewed-by: Benjamin Franzke <ben@bnf.dev> Reviewed-by: Benni Mack <benni@typo3.org> Reviewed-by: Nikita Hovratov <nikita.h@live.de> Tested-by: Christian Kuhn <lolli@schwarzbu.ch> Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch> Tested-by: Benjamin Franzke <ben@bnf.dev>
Symfony >7.3 generates optimized DI caches for PHP >=8.4 by using `ReflectionClass::newLazyProxy` (and friends) if available [1]. That means a cache that is generated with PHP 8.4 (e.g. CLI context) can not be used in a PHP 8.2 process (e.g. Web Context) and therefore we ensure that the major and minor versions match when reusing a cached DI container. This has a drawback that DI caches can not be warmed using `bin/typo3 cache:warmup` on CLI, when CLI and WEB PHP versions do not share the same major and minor version, but it's better than a broken instance. [1] symfony/symfony#59913 Resolves: #107649 Releases: main, 13.4 Change-Id: Ib27b0f015847fbce81fc42685278c72e8a707aa0 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/90999 Tested-by: core-ci <typo3@b13.com> Tested-by: Benjamin Franzke <ben@bnf.dev> Reviewed-by: Benjamin Franzke <ben@bnf.dev>
Symfony >7.3 generates optimized DI caches for PHP >=8.4 by using `ReflectionClass::newLazyProxy` (and friends) if available [1]. That means a cache that is generated with PHP 8.4 (e.g. CLI context) can not be used in a PHP 8.2 process (e.g. Web Context) and therefore we ensure that the major and minor versions match when reusing a cached DI container. This has a drawback that DI caches can not be warmed using `bin/typo3 cache:warmup` on CLI, when CLI and WEB PHP versions do not share the same major and minor version, but it's better than a broken instance. [1] symfony/symfony#59913 Resolves: #107649 Releases: main, 13.4 Change-Id: Ib27b0f015847fbce81fc42685278c72e8a707aa0 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/90988 Tested-by: Garvin Hicking <garvin@hick.ing> Tested-by: Benni Mack <benni@typo3.org> Reviewed-by: Garvin Hicking <garvin@hick.ing> Tested-by: Nikita Hovratov <nikita.h@live.de> Reviewed-by: Oliver Hader <oliver.hader@typo3.org> Tested-by: core-ci <typo3@b13.com> Reviewed-by: Benjamin Franzke <ben@bnf.dev> Reviewed-by: Benni Mack <benni@typo3.org> Reviewed-by: Nikita Hovratov <nikita.h@live.de> Tested-by: Christian Kuhn <lolli@schwarzbu.ch> Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch> Tested-by: Benjamin Franzke <ben@bnf.dev>
Symfony >7.3 generates optimized DI caches for PHP >=8.4 by using `ReflectionClass::newLazyProxy` (and friends) if available [1]. That means a cache that is generated with PHP 8.4 (e.g. CLI context) can not be used in a PHP 8.2 process (e.g. Web Context) and therefore we ensure that the major and minor versions match when reusing a cached DI container. This has a drawback that DI caches can not be warmed using `bin/typo3 cache:warmup` on CLI, when CLI and WEB PHP versions do not share the same major and minor version, but it's better than a broken instance. [1] symfony/symfony#59913 Resolves: #107649 Releases: main, 13.4 Change-Id: Ib27b0f015847fbce81fc42685278c72e8a707aa0 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/90999 Tested-by: core-ci <typo3@b13.com> Tested-by: Benjamin Franzke <ben@bnf.dev> Reviewed-by: Benjamin Franzke <ben@bnf.dev>
When a service is lazy, we can leverage native lazy objects on PHP 8.4.
(Note that we still need var-exporter to generate lazy proxies when using the
proxytag - aka when proxying "by interface".)