Skip to content

[DI] fine tune dumped factories #27268

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

Merged
merged 1 commit into from
May 17, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 26 additions & 4 deletions src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use Symfony\Component\DependencyInjection\Variable;
use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\Compiler\AnalyzeServiceReferencesPass;
use Symfony\Component\DependencyInjection\Compiler\ServiceReferenceGraphNode;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Container;
use Symfony\Component\DependencyInjection\ContainerInterface;
Expand Down Expand Up @@ -68,6 +69,7 @@ class PhpDumper extends Dumper
private $inlineRequires;
private $inlinedRequires = array();
private $circularReferences = array();
private $singleUsePrivateIds = array();

/**
* @var ProxyDumper
Expand Down Expand Up @@ -141,10 +143,14 @@ public function dump(array $options = array())

(new AnalyzeServiceReferencesPass())->process($this->container);
$this->circularReferences = array();
$this->singleUsePrivateIds = array();
$checkedNodes = array();
foreach ($this->container->getCompiler()->getServiceReferenceGraph()->getNodes() as $id => $node) {
$currentPath = array($id => $id);
$this->analyzeCircularReferences($node->getOutEdges(), $checkedNodes, $currentPath);
if ($this->isSingleUsePrivateNode($node)) {
$this->singleUsePrivateIds[$id] = $id;
}
}
$this->container->getCompiler()->getServiceReferenceGraph()->clear();

Expand Down Expand Up @@ -526,7 +532,7 @@ private function addServiceInstance(string $id, Definition $definition, string $
$isProxyCandidate = $this->getProxyDumper()->isProxyCandidate($definition);
$instantiation = '';

if (!$isProxyCandidate && $definition->isShared()) {
if (!$isProxyCandidate && $definition->isShared() && !isset($this->singleUsePrivateIds[$id])) {
$instantiation = sprintf('$this->%s[\'%s\'] = %s', $this->container->getDefinition($id)->isPublic() ? 'services' : 'privates', $id, $isSimpleInstance ? '' : '$instance');
} elseif (!$isSimpleInstance) {
$instantiation = '$instance';
Expand Down Expand Up @@ -819,7 +825,7 @@ private function generateServiceFiles()
$definitions = $this->container->getDefinitions();
ksort($definitions);
foreach ($definitions as $id => $definition) {
if (!$definition->isSynthetic() && !$this->isHotPath($definition)) {
if (!$definition->isSynthetic() && !$this->isHotPath($definition) && ($definition->isPublic() || !$this->isTrivialInstance($definition))) {
$code = $this->addService($id, $definition, $file);

if (!$definition->isShared()) {
Expand Down Expand Up @@ -1662,7 +1668,7 @@ private function getServiceCall(string $id, Reference $reference = null): string
$code = 'null';
} elseif ($this->isTrivialInstance($definition)) {
$code = substr($this->addNewInstance($definition, '', '', $id), 8, -2);
if ($definition->isShared()) {
if ($definition->isShared() && !isset($this->singleUsePrivateIds[$id])) {
$code = sprintf('$this->%s[\'%s\'] = %s', $definition->isPublic() ? 'services' : 'privates', $id, $code);
}
} elseif ($this->asFiles && !$this->isHotPath($definition)) {
Expand All @@ -1674,7 +1680,7 @@ private function getServiceCall(string $id, Reference $reference = null): string
} else {
$code = sprintf('$this->%s()', $this->generateMethodName($id));
}
if ($definition->isShared()) {
if ($definition->isShared() && !isset($this->singleUsePrivateIds[$id])) {
$code = sprintf('($this->%s[\'%s\'] ?? %s)', $definition->isPublic() ? 'services' : 'privates', $id, $code);
}

Expand Down Expand Up @@ -1798,6 +1804,22 @@ private function isHotPath(Definition $definition)
return $this->hotPathTag && $definition->hasTag($this->hotPathTag) && !$definition->isDeprecated();
}

private function isSingleUsePrivateNode(ServiceReferenceGraphNode $node): bool
{
if ($node->getValue()->isPublic()) {
return false;
}
$ids = array();
foreach ($node->getInEdges() as $edge) {
if ($edge->isLazy() || !$edge->getSourceNode()->getValue()->isShared()) {
return false;
}
$ids[$edge->getSourceNode()->getId()] = true;
}

return 1 === \count($ids);
}

private function export($value)
{
if (null !== $this->targetDirRegex && is_string($value) && preg_match($this->targetDirRegex, $value, $matches, PREG_OFFSET_CAPTURE)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ use Symfony\Component\DependencyInjection\Exception\RuntimeException;
// This file has been auto-generated by the Symfony Dependency Injection Component for internal use.
// Returns the public 'factory_service_simple' shared service.

return $this->services['factory_service_simple'] = ($this->privates['factory_simple'] ?? $this->load('getFactorySimpleService.php'))->getInstance();
return $this->services['factory_service_simple'] = $this->load('getFactorySimpleService.php')->getInstance();

[Container%s/getFactorySimpleService.php] => <?php

Expand All @@ -167,7 +167,7 @@ use Symfony\Component\DependencyInjection\Exception\RuntimeException;

@trigger_error('The "factory_simple" service is deprecated. You should stop using it, as it will soon be removed.', E_USER_DEPRECATED);

return $this->privates['factory_simple'] = new \SimpleFactoryClass('foo');
return new \SimpleFactoryClass('foo');

[Container%s/getFooService.php] => <?php

Expand Down Expand Up @@ -326,7 +326,7 @@ use Symfony\Component\DependencyInjection\Exception\RuntimeException;
// This file has been auto-generated by the Symfony Dependency Injection Component for internal use.
// Returns the public 'runtime_error' shared service.

return $this->services['runtime_error'] = new \stdClass(($this->privates['errored_definition'] ?? $this->load('getErroredDefinitionService.php')));
return $this->services['runtime_error'] = new \stdClass($this->load('getErroredDefinitionService.php'));

[Container%s/getServiceFromStaticMethodService.php] => <?php

Expand All @@ -351,16 +351,6 @@ return $this->services['tagged_iterator'] = new \Bar(new RewindableGenerator(fun
yield 1 => ($this->privates['tagged_iterator_foo'] ?? $this->privates['tagged_iterator_foo'] = new \Bar());
}, 2));

[Container%s/getTaggedIteratorFooService.php] => <?php

use Symfony\Component\DependencyInjection\Argument\RewindableGenerator;
use Symfony\Component\DependencyInjection\Exception\RuntimeException;

// This file has been auto-generated by the Symfony Dependency Injection Component for internal use.
// Returns the private 'tagged_iterator_foo' shared service.

return $this->privates['tagged_iterator_foo'] = new \Bar();

[Container%s/ProjectServiceContainer.php] => <?php

namespace Container%s;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ protected function getFactoryServiceService()
*/
protected function getFactoryServiceSimpleService()
{
return $this->services['factory_service_simple'] = ($this->privates['factory_simple'] ?? $this->getFactorySimpleService())->getInstance();
return $this->services['factory_service_simple'] = $this->getFactorySimpleService()->getInstance();
}

/**
Expand Down Expand Up @@ -381,7 +381,7 @@ protected function getNewFactoryServiceService()
*/
protected function getRuntimeErrorService()
{
return $this->services['runtime_error'] = new \stdClass(($this->privates['errored_definition'] ?? $this->getErroredDefinitionService()));
return $this->services['runtime_error'] = new \stdClass($this->getErroredDefinitionService());
}

/**
Expand Down Expand Up @@ -428,7 +428,7 @@ protected function getFactorySimpleService()
{
@trigger_error('The "factory_simple" service is deprecated. You should stop using it, as it will soon be removed.', E_USER_DEPRECATED);

return $this->privates['factory_simple'] = new \SimpleFactoryClass('foo');
return new \SimpleFactoryClass('foo');
}

public function getParameter($name)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ protected function getFactoryServiceService()
*/
protected function getFactoryServiceSimpleService()
{
return $this->services['factory_service_simple'] = ($this->privates['factory_simple'] ?? $this->getFactorySimpleService())->getInstance();
return $this->services['factory_service_simple'] = $this->getFactorySimpleService()->getInstance();
}

/**
Expand Down Expand Up @@ -381,7 +381,7 @@ protected function getNewFactoryServiceService()
*/
protected function getRuntimeErrorService()
{
return $this->services['runtime_error'] = new \stdClass(($this->privates['errored_definition'] ?? $this->getErroredDefinitionService()));
return $this->services['runtime_error'] = new \stdClass($this->getErroredDefinitionService());
}

/**
Expand Down Expand Up @@ -428,7 +428,7 @@ protected function getFactorySimpleService()
{
@trigger_error('The "factory_simple" service is deprecated. You should stop using it, as it will soon be removed.', E_USER_DEPRECATED);

return $this->privates['factory_simple'] = new \SimpleFactoryClass('foo');
return new \SimpleFactoryClass('foo');
}

public function getParameter($name)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,6 @@ public function getRemovedIds()
*/
protected function getPublicFooService()
{
return $this->services['public_foo'] = new \stdClass(($this->privates['private_foo'] ?? $this->privates['private_foo'] = new \stdClass()));
return $this->services['public_foo'] = new \stdClass(new \stdClass());
}
}