From 75e2ee17b0f87fcf085f982df321cc5b7a6f6579 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Sat, 27 Jun 2020 12:33:05 +0200 Subject: [PATCH 1/2] [DI] fix minor perf regression when creating non-shared services --- .../DependencyInjection/Dumper/PhpDumper.php | 26 ++++++++++++++----- .../Tests/Fixtures/php/services9_as_files.txt | 12 +++++++-- .../Tests/Fixtures/php/services9_compiled.php | 6 ++++- .../php/services9_inlined_factories.txt | 12 +++++++-- .../php/services_almost_circular_public.php | 10 ++++--- .../php/services_errored_definition.php | 6 ++++- .../Fixtures/php/services_non_shared_lazy.php | 8 ++++-- .../php/services_non_shared_lazy_as_files.txt | 6 ++++- .../php/services_service_locator_argument.php | 6 ++++- 9 files changed, 73 insertions(+), 19 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php index 91c0ff5c9e..cda9a49acf 100644 --- a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php +++ b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php @@ -864,13 +864,23 @@ EOF; } if ($this->getProxyDumper()->isProxyCandidate($definition)) { - $factoryCode = $asFile ? "\$this->load('%s', false)" : '$this->%s(false)'; - $factoryCode = $this->getProxyDumper()->getProxyFactoryCode($definition, $id, sprintf($factoryCode, $methodName)); + $factoryCode = $definition->isShared() ? ($asFile ? "\$this->load('%s', false)" : '$this->%s(false)') : '$this->factories[%2$s](false)'; + $factoryCode = $this->getProxyDumper()->getProxyFactoryCode($definition, $id, sprintf($factoryCode, $methodName, $this->doExport($id))); $code .= $asFile ? preg_replace('/function \(([^)]*+)\) {/', 'function (\1) use ($container) {', $factoryCode) : $factoryCode; } $code .= $this->addServiceInclude($id, $definition); - $code .= $this->addInlineService($id, $definition); + $c = $this->addInlineService($id, $definition); + + if (!$definition->isShared()) { + $c = implode("\n", array_map(function ($line) { return $line ? ' '.$line : $line; }, explode("\n", $c))); + $factory = sprintf('$this->factories%s[%s]', $definition->isPublic() ? '' : "['service_container']", $this->doExport($id)); + $lazyloadInitialization = $definition->isLazy() ? '$lazyLoad = true' : ''; + + $c = sprintf(" %s = function (%s) {\n%s };\n\n return %1\$s();\n", $factory, $lazyloadInitialization, $c); + } + + $code .= $c; } if ($asFile) { @@ -1880,10 +1890,14 @@ EOF; $code = sprintf('$this->%s[%s] = %s', $definition->isPublic() ? 'services' : 'privates', $this->doExport($id), $code); } $code = "($code)"; - } elseif ($this->asFiles && !$this->inlineFactories && !$this->isHotPath($definition)) { - $code = sprintf("\$this->load('%s')", $this->generateMethodName($id)); } else { - $code = sprintf('$this->%s()', $this->generateMethodName($id)); + $code = $this->asFiles && !$this->inlineFactories && !$this->isHotPath($definition) ? "\$this->load('%s')" : '$this->%s()'; + $code = sprintf($code, $this->generateMethodName($id)); + + if (!$definition->isShared()) { + $factory = sprintf('$this->factories%s[%s]', $definition->isPublic() ? '' : "['service_container']", $this->doExport($id)); + $code = sprintf('(isset(%s) ? %1$s() : %s)', $factory, $code); + } } if ($definition->isShared() && !isset($this->singleUsePrivateIds[$id])) { $code = sprintf('($this->%s[%s] ?? %s)', $definition->isPublic() ? 'services' : 'privates', $this->doExport($id), $code); diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_as_files.txt b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_as_files.txt index 7818c091a0..c7291046ca 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_as_files.txt +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_as_files.txt @@ -402,7 +402,11 @@ class getFooBarService extends ProjectServiceContainer */ public static function do($container, $lazyLoad = true) { - return new \Bar\FooClass(($container->services['deprecated_service'] ?? $container->load('getDeprecatedServiceService'))); + $container->factories['foo_bar'] = function () use ($container) { + return new \Bar\FooClass(($container->services['deprecated_service'] ?? $container->load('getDeprecatedServiceService'))); + }; + + return $container->factories['foo_bar'](); } } @@ -574,7 +578,11 @@ class getNonSharedFooService extends ProjectServiceContainer { include_once $container->targetDir.''.'/Fixtures/includes/foo.php'; - return new \Bar\FooClass(); + $container->factories['non_shared_foo'] = function () use ($container) { + return new \Bar\FooClass(); + }; + + return $container->factories['non_shared_foo'](); } } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php index 277da470b5..671a1b11b3 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php @@ -275,7 +275,11 @@ class ProjectServiceContainer extends Container */ protected function getFooBarService() { - return new \Bar\FooClass(($this->services['deprecated_service'] ?? $this->getDeprecatedServiceService())); + $this->factories['foo_bar'] = function () { + return new \Bar\FooClass(($this->services['deprecated_service'] ?? $this->getDeprecatedServiceService())); + }; + + return $this->factories['foo_bar'](); } /** diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_inlined_factories.txt b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_inlined_factories.txt index 6a3af3fbb9..cf0543d4ee 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_inlined_factories.txt +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_inlined_factories.txt @@ -300,7 +300,11 @@ class ProjectServiceContainer extends Container */ protected function getFooBarService() { - return new \Bar\FooClass(($this->services['deprecated_service'] ?? $this->getDeprecatedServiceService())); + $this->factories['foo_bar'] = function () { + return new \Bar\FooClass(($this->services['deprecated_service'] ?? $this->getDeprecatedServiceService())); + }; + + return $this->factories['foo_bar'](); } /** @@ -398,7 +402,11 @@ class ProjectServiceContainer extends Container { include_once $this->targetDir.''.'/Fixtures/includes/foo.php'; - return new \Bar\FooClass(); + $this->factories['non_shared_foo'] = function () { + return new \Bar\FooClass(); + }; + + return $this->factories['non_shared_foo'](); } /** diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_almost_circular_public.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_almost_circular_public.php index 1554588ac3..4905ade2dd 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_almost_circular_public.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_almost_circular_public.php @@ -287,11 +287,15 @@ class Symfony_DI_PhpDumper_Test_Almost_Circular_Public extends Container */ protected function getFoo4Service() { - $instance = new \stdClass(); + $this->factories['foo4'] = function () { + $instance = new \stdClass(); - $instance->foobar = ($this->services['foobar4'] ?? $this->getFoobar4Service()); + $instance->foobar = ($this->services['foobar4'] ?? $this->getFoobar4Service()); - return $instance; + return $instance; + }; + + return $this->factories['foo4'](); } /** diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_errored_definition.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_errored_definition.php index 9f2bb02158..99595f6478 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_errored_definition.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_errored_definition.php @@ -275,7 +275,11 @@ class Symfony_DI_PhpDumper_Errored_Definition extends Container */ protected function getFooBarService() { - return new \Bar\FooClass(($this->services['deprecated_service'] ?? $this->getDeprecatedServiceService())); + $this->factories['foo_bar'] = function () { + return new \Bar\FooClass(($this->services['deprecated_service'] ?? $this->getDeprecatedServiceService())); + }; + + return $this->factories['foo_bar'](); } /** diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_non_shared_lazy.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_non_shared_lazy.php index 4295512b08..048e4fdf59 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_non_shared_lazy.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_non_shared_lazy.php @@ -57,7 +57,7 @@ class ProjectServiceContainer extends Container */ protected function getBarService() { - return $this->services['bar'] = new \stdClass($this->getFooService()); + return $this->services['bar'] = new \stdClass((isset($this->factories['service_container']['foo']) ? $this->factories['service_container']['foo']() : $this->getFooService())); } /** @@ -69,7 +69,11 @@ class ProjectServiceContainer extends Container { // lazy factory for stdClass - return new \stdClass(); + $this->factories['service_container']['foo'] = function ($lazyLoad = true) { + return new \stdClass(); + }; + + return $this->factories['service_container']['foo'](); } } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_non_shared_lazy_as_files.txt b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_non_shared_lazy_as_files.txt index ebdcacd1b4..a42d1e29db 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_non_shared_lazy_as_files.txt +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_non_shared_lazy_as_files.txt @@ -30,7 +30,11 @@ class getNonSharedFooService extends ProjectServiceContainer { include_once $container->targetDir.''.'/Fixtures/includes/foo_lazy.php'; - return new \Bar\FooLazyClass(); + $container->factories['non_shared_foo'] = function ($lazyLoad = true) { + return new \Bar\FooLazyClass(); + }; + + return $container->factories['non_shared_foo'](); } } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_service_locator_argument.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_service_locator_argument.php index 14873b484c..c695bb8e49 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_service_locator_argument.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_service_locator_argument.php @@ -107,7 +107,11 @@ class Symfony_DI_PhpDumper_Service_Locator_Argument extends Container */ protected function getFoo3Service() { - return new \stdClass(); + $this->factories['service_container']['foo3'] = function () { + return new \stdClass(); + }; + + return $this->factories['service_container']['foo3'](); } /** From 1b6bbc2e716566b8ec2cedcf5e68937fbdb586d6 Mon Sep 17 00:00:00 2001 From: "Alexander M. Turek" Date: Sat, 27 Jun 2020 23:56:40 +0200 Subject: [PATCH 2/2] [Notifier] Notifier 5.2 is incompatible with 5.1 bridges. --- src/Symfony/Component/Notifier/Bridge/Firebase/composer.json | 2 +- src/Symfony/Component/Notifier/Bridge/FreeMobile/composer.json | 2 +- src/Symfony/Component/Notifier/Bridge/Mattermost/composer.json | 2 +- src/Symfony/Component/Notifier/Bridge/Nexmo/composer.json | 2 +- src/Symfony/Component/Notifier/Bridge/OvhCloud/composer.json | 2 +- src/Symfony/Component/Notifier/Bridge/RocketChat/composer.json | 2 +- src/Symfony/Component/Notifier/Bridge/Sinch/composer.json | 2 +- src/Symfony/Component/Notifier/Bridge/Slack/composer.json | 2 +- src/Symfony/Component/Notifier/Bridge/Telegram/composer.json | 2 +- src/Symfony/Component/Notifier/Bridge/Twilio/composer.json | 2 +- 10 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/Symfony/Component/Notifier/Bridge/Firebase/composer.json b/src/Symfony/Component/Notifier/Bridge/Firebase/composer.json index f6d1152b74..799762400b 100644 --- a/src/Symfony/Component/Notifier/Bridge/Firebase/composer.json +++ b/src/Symfony/Component/Notifier/Bridge/Firebase/composer.json @@ -18,7 +18,7 @@ "require": { "php": ">=7.2.5", "symfony/http-client": "^4.3|^5.0", - "symfony/notifier": "^5.0" + "symfony/notifier": "^5.0,<5.2" }, "autoload": { "psr-4": { "Symfony\\Component\\Notifier\\Bridge\\Firebase\\": "" }, diff --git a/src/Symfony/Component/Notifier/Bridge/FreeMobile/composer.json b/src/Symfony/Component/Notifier/Bridge/FreeMobile/composer.json index d08811d689..97edbd7e51 100644 --- a/src/Symfony/Component/Notifier/Bridge/FreeMobile/composer.json +++ b/src/Symfony/Component/Notifier/Bridge/FreeMobile/composer.json @@ -19,7 +19,7 @@ "require": { "php": ">=7.2.5", "symfony/http-client": "^4.3|^5.1", - "symfony/notifier": "^5.1" + "symfony/notifier": "~5.1.0" }, "autoload": { "psr-4": { "Symfony\\Component\\Notifier\\Bridge\\FreeMobile\\": "" }, diff --git a/src/Symfony/Component/Notifier/Bridge/Mattermost/composer.json b/src/Symfony/Component/Notifier/Bridge/Mattermost/composer.json index 62458ec7b1..fa6a43f897 100644 --- a/src/Symfony/Component/Notifier/Bridge/Mattermost/composer.json +++ b/src/Symfony/Component/Notifier/Bridge/Mattermost/composer.json @@ -18,7 +18,7 @@ "require": { "php": ">=7.2.5", "symfony/http-client": "^4.3|^5.0", - "symfony/notifier": "^5.0" + "symfony/notifier": "^5.0,<5.2" }, "autoload": { "psr-4": { "Symfony\\Component\\Notifier\\Bridge\\Mattermost\\": "" }, diff --git a/src/Symfony/Component/Notifier/Bridge/Nexmo/composer.json b/src/Symfony/Component/Notifier/Bridge/Nexmo/composer.json index 1c8060aeff..5c6be2d6e2 100644 --- a/src/Symfony/Component/Notifier/Bridge/Nexmo/composer.json +++ b/src/Symfony/Component/Notifier/Bridge/Nexmo/composer.json @@ -18,7 +18,7 @@ "require": { "php": ">=7.2.5", "symfony/http-client": "^4.3|^5.0", - "symfony/notifier": "^5.0" + "symfony/notifier": "^5.0,<5.2" }, "autoload": { "psr-4": { "Symfony\\Component\\Notifier\\Bridge\\Nexmo\\": "" }, diff --git a/src/Symfony/Component/Notifier/Bridge/OvhCloud/composer.json b/src/Symfony/Component/Notifier/Bridge/OvhCloud/composer.json index ecea9f7cd4..88d7f960af 100644 --- a/src/Symfony/Component/Notifier/Bridge/OvhCloud/composer.json +++ b/src/Symfony/Component/Notifier/Bridge/OvhCloud/composer.json @@ -18,7 +18,7 @@ "require": { "php": ">=7.2.5", "symfony/http-client": "^4.3|^5.0", - "symfony/notifier": "^5.0" + "symfony/notifier": "^5.0,<5.2" }, "autoload": { "psr-4": { "Symfony\\Component\\Notifier\\Bridge\\OvhCloud\\": "" }, diff --git a/src/Symfony/Component/Notifier/Bridge/RocketChat/composer.json b/src/Symfony/Component/Notifier/Bridge/RocketChat/composer.json index 5f49b7efad..510d8a23fa 100644 --- a/src/Symfony/Component/Notifier/Bridge/RocketChat/composer.json +++ b/src/Symfony/Component/Notifier/Bridge/RocketChat/composer.json @@ -18,7 +18,7 @@ "require": { "php": ">=7.2.5", "symfony/http-client": "^4.3|^5.0", - "symfony/notifier": "^5.0" + "symfony/notifier": "^5.0,<5.2" }, "autoload": { "psr-4": { "Symfony\\Component\\Notifier\\Bridge\\RocketChat\\": "" }, diff --git a/src/Symfony/Component/Notifier/Bridge/Sinch/composer.json b/src/Symfony/Component/Notifier/Bridge/Sinch/composer.json index 6eadb5cb0e..9e3f52d70f 100644 --- a/src/Symfony/Component/Notifier/Bridge/Sinch/composer.json +++ b/src/Symfony/Component/Notifier/Bridge/Sinch/composer.json @@ -19,7 +19,7 @@ "php": ">=7.2.5", "ext-json": "*", "symfony/http-client": "^4.3|^5.0", - "symfony/notifier": "^5.0" + "symfony/notifier": "^5.0,<5.2" }, "autoload": { "psr-4": { "Symfony\\Component\\Notifier\\Bridge\\Sinch\\": "" }, diff --git a/src/Symfony/Component/Notifier/Bridge/Slack/composer.json b/src/Symfony/Component/Notifier/Bridge/Slack/composer.json index 9714d63233..0cf160687b 100644 --- a/src/Symfony/Component/Notifier/Bridge/Slack/composer.json +++ b/src/Symfony/Component/Notifier/Bridge/Slack/composer.json @@ -18,7 +18,7 @@ "require": { "php": ">=7.2.5", "symfony/http-client": "^4.3|^5.0", - "symfony/notifier": "^5.1" + "symfony/notifier": "~5.1.0" }, "require-dev": { "symfony/event-dispatcher": "^4.3|^5.0" diff --git a/src/Symfony/Component/Notifier/Bridge/Telegram/composer.json b/src/Symfony/Component/Notifier/Bridge/Telegram/composer.json index db7d3d8829..20ca97ec4e 100644 --- a/src/Symfony/Component/Notifier/Bridge/Telegram/composer.json +++ b/src/Symfony/Component/Notifier/Bridge/Telegram/composer.json @@ -18,7 +18,7 @@ "require": { "php": ">=7.2.5", "symfony/http-client": "^4.3|^5.0", - "symfony/notifier": "^5.1" + "symfony/notifier": "~5.1.0" }, "require-dev": { "symfony/event-dispatcher": "^4.3|^5.0" diff --git a/src/Symfony/Component/Notifier/Bridge/Twilio/composer.json b/src/Symfony/Component/Notifier/Bridge/Twilio/composer.json index 5606812fc3..3b7b179383 100644 --- a/src/Symfony/Component/Notifier/Bridge/Twilio/composer.json +++ b/src/Symfony/Component/Notifier/Bridge/Twilio/composer.json @@ -18,7 +18,7 @@ "require": { "php": ">=7.2.5", "symfony/http-client": "^4.3|^5.0", - "symfony/notifier": "^5.0" + "symfony/notifier": "^5.0,<5.2" }, "autoload": { "psr-4": { "Symfony\\Component\\Notifier\\Bridge\\Twilio\\": "" },