Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 7a461cf

Browse filesBrowse files
nicolas-grekasnicolas-grekas
committed
minor #25867 [DI] Put non-shared service factories in closures (nicolas-grekas)
This PR was merged into the 4.1-dev branch. Discussion ---------- [DI] Put non-shared service factories in closures | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - With this change, non-shared services are moved to dedicated files (unless they are on the hot path). Previously, they were always dumped as methods. The goal of this change is to dump factories as methods *if and only if* the services they build are on the hot-path. By doing so, it will become very easy to figure out which services are on the hot path, vs the rest. And then people will be able to optimize their configurations: if too many things are dumped as methods, it will trivially mean some laziness is missing in definitions. I spotted this while reviewing the dumped container of Blackfire, where we sometimes have long chains of dependencies that are on the hot path for no real reason - mixed with big non-shared factories (Sonata admin blocks in our case.) Commits ------- 22c5325 [DI] Put non-shared service factories in closures
2 parents 9be03d8 + 22c5325 commit 7a461cf
Copy full SHA for 7a461cf

File tree

3 files changed

+37
-18
lines changed
Filter options

3 files changed

+37
-18
lines changed

‎src/Symfony/Component/DependencyInjection/Container.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Container.php
+5-1Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ class Container implements ResettableContainerInterface
4343
protected $services = array();
4444
protected $fileMap = array();
4545
protected $methodMap = array();
46+
protected $factories = array();
4647
protected $aliases = array();
4748
protected $loading = array();
4849
protected $resolving = array();
@@ -227,6 +228,9 @@ public function get($id, $invalidBehavior = /* self::EXCEPTION_ON_INVALID_REFERE
227228
if ('service_container' === $id) {
228229
return $this;
229230
}
231+
if (isset($this->factories[$id])) {
232+
return $this->factories[$id]();
233+
}
230234

231235
if (isset($this->loading[$id])) {
232236
throw new ServiceCircularReferenceException($id, array_keys($this->loading));
@@ -296,7 +300,7 @@ public function initialized($id)
296300
*/
297301
public function reset()
298302
{
299-
$this->services = array();
303+
$this->services = $this->factories = array();
300304
}
301305

302306
/**

‎src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
+17-6Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -713,7 +713,7 @@ private function addService(string $id, Definition $definition, string &$file =
713713
$lazyInitialization = '';
714714
}
715715

716-
$asFile = $this->asFiles && $definition->isShared() && !$this->isHotPath($definition);
716+
$asFile = $this->asFiles && !$this->isHotPath($definition);
717717
$methodName = $this->generateMethodName($id);
718718
if ($asFile) {
719719
$file = $methodName.'.php';
@@ -787,7 +787,7 @@ private function addServices(): string
787787
$definitions = $this->container->getDefinitions();
788788
ksort($definitions);
789789
foreach ($definitions as $id => $definition) {
790-
if ($definition->isSynthetic() || ($this->asFiles && $definition->isShared() && !$this->isHotPath($definition))) {
790+
if ($definition->isSynthetic() || ($this->asFiles && !$this->isHotPath($definition))) {
791791
continue;
792792
}
793793
if ($definition->isPublic()) {
@@ -805,8 +805,15 @@ private function generateServiceFiles()
805805
$definitions = $this->container->getDefinitions();
806806
ksort($definitions);
807807
foreach ($definitions as $id => $definition) {
808-
if (!$definition->isSynthetic() && $definition->isShared() && !$this->isHotPath($definition)) {
808+
if (!$definition->isSynthetic() && !$this->isHotPath($definition)) {
809809
$code = $this->addService($id, $definition, $file);
810+
811+
if (!$definition->isShared()) {
812+
$code = implode("\n", array_map(function ($line) { return $line ? ' '.$line : $line; }, explode("\n", $code)));
813+
$factory = sprintf('$this->factories%s[\'%s\']', $definition->isPublic() ? '' : "['service_container']", $id);
814+
$code = sprintf("\n%s = function () {\n%s};\n\nreturn %1\$s();\n", $factory, $code);
815+
}
816+
810817
yield $file => $code;
811818
}
812819
}
@@ -1038,7 +1045,7 @@ private function addMethodMap(): string
10381045
$definitions = $this->container->getDefinitions();
10391046
ksort($definitions);
10401047
foreach ($definitions as $id => $definition) {
1041-
if (!$definition->isSynthetic() && $definition->isPublic() && (!$this->asFiles || !$definition->isShared() || $this->isHotPath($definition))) {
1048+
if (!$definition->isSynthetic() && $definition->isPublic() && (!$this->asFiles || $this->isHotPath($definition))) {
10421049
$code .= ' '.$this->doExport($id).' => '.$this->doExport($this->generateMethodName($id)).",\n";
10431050
}
10441051
}
@@ -1052,7 +1059,7 @@ private function addFileMap(): string
10521059
$definitions = $this->container->getDefinitions();
10531060
ksort($definitions);
10541061
foreach ($definitions as $id => $definition) {
1055-
if (!$definition->isSynthetic() && $definition->isPublic() && $definition->isShared() && !$this->isHotPath($definition)) {
1062+
if (!$definition->isSynthetic() && $definition->isPublic() && !$this->isHotPath($definition)) {
10561063
$code .= sprintf(" %s => __DIR__.'/%s.php',\n", $this->doExport($id), $this->generateMethodName($id));
10571064
}
10581065
}
@@ -1623,8 +1630,12 @@ private function getServiceCall(string $id, Reference $reference = null): string
16231630
if ($definition->isShared()) {
16241631
$code = sprintf('$this->%s[\'%s\'] = %s', $definition->isPublic() ? 'services' : 'privates', $id, $code);
16251632
}
1626-
} elseif ($this->asFiles && $definition->isShared() && !$this->isHotPath($definition)) {
1633+
} elseif ($this->asFiles && !$this->isHotPath($definition)) {
16271634
$code = sprintf("\$this->load(__DIR__.'/%s.php')", $this->generateMethodName($id));
1635+
if (!$definition->isShared()) {
1636+
$factory = sprintf('$this->factories%s[\'%s\']', $definition->isPublic() ? '' : "['service_container']", $id);
1637+
$code = sprintf('(isset(%s) ? %1$s() : %s)', $factory, $code);
1638+
}
16281639
} else {
16291640
$code = sprintf('$this->%s()', $this->generateMethodName($id));
16301641
}

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_as_files.txt

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_as_files.txt
+15-11Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,20 @@ $this->services['foo.baz'] = $instance = \BazClass::getInstance();
179179

180180
return $instance;
181181

182+
[Container%s/getFooBarService.php] => <?php
183+
184+
use Symfony\Component\DependencyInjection\Argument\RewindableGenerator;
185+
186+
// This file has been auto-generated by the Symfony Dependency Injection Component for internal use.
187+
188+
$this->factories['foo_bar'] = function () {
189+
// Returns the public 'foo_bar' service.
190+
191+
return new \Bar\FooClass(($this->services['deprecated_service'] ?? $this->load(__DIR__.'/getDeprecatedServiceService.php')));
192+
};
193+
194+
return $this->factories['foo_bar']();
195+
182196
[Container%s/getFooWithInlineService.php] => <?php
183197

184198
use Symfony\Component\DependencyInjection\Argument\RewindableGenerator;
@@ -331,7 +345,6 @@ class ProjectServiceContainer extends Container
331345
);
332346
$this->methodMap = array(
333347
'bar' => 'getBarService',
334-
'foo_bar' => 'getFooBarService',
335348
);
336349
$this->fileMap = array(
337350
'BAR' => __DIR__.'/getBAR2Service.php',
@@ -347,6 +360,7 @@ class ProjectServiceContainer extends Container
347360
'factory_service_simple' => __DIR__.'/getFactoryServiceSimpleService.php',
348361
'foo' => __DIR__.'/getFooService.php',
349362
'foo.baz' => __DIR__.'/getFoo_BazService.php',
363+
'foo_bar' => __DIR__.'/getFooBarService.php',
350364
'foo_with_inline' => __DIR__.'/getFooWithInlineService.php',
351365
'lazy_context' => __DIR__.'/getLazyContextService.php',
352366
'lazy_context_ignore_invalid_ref' => __DIR__.'/getLazyContextIgnoreInvalidRefService.php',
@@ -404,16 +418,6 @@ class ProjectServiceContainer extends Container
404418
return $instance;
405419
}
406420

407-
/**
408-
* Gets the public 'foo_bar' service.
409-
*
410-
* @return \Bar\FooClass
411-
*/
412-
protected function getFooBarService()
413-
{
414-
return new \Bar\FooClass(($this->services['deprecated_service'] ?? $this->load(__DIR__.'/getDeprecatedServiceService.php')));
415-
}
416-
417421
public function getParameter($name)
418422
{
419423
$name = (string) $name;

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.