Skip to content

[DependencyInjection] made some perf improvements #10241

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.

Sign up for GitHub

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
Sep 26, 2014
Merged

[DependencyInjection] made some perf improvements #10241

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
43 changes: 12 additions & 31 deletions src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -800,16 +800,18 @@ class $class extends $baseClass
*/
private function addConstructor()
{
$arguments = $this->container->getParameterBag()->all() ? 'new ParameterBag($this->getDefaultParameters())' : null;
$parameters = $this->exportParameters($this->container->getParameterBag()->all());

$code = <<<EOF

private static \$parameters = $parameters;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the static property needed in case of non-frozen containers ? would it improve perf in this case ? The static array will need to be copied as soon as you manipulate parameters.

I know that this is kind of an edge case given that Symfony always freezes the container before dumping it (I'm not even sure there is valid use case for dumping a non-frozen container, given that parameters used in service definitions will always be inlined, so changing them in the ParameterBag will be useless for most of them)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, without a frozen container, the optimization won't work but it won't be worse than now, so as that should never happen, it does not really matters.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we prevent dumping a container which is not frozen in 3.0 given that the fact of dumping will freeze it partially ?


/**
* Constructor.
*/
public function __construct()
{
parent::__construct($arguments);
parent::__construct(new ParameterBag(self::\$parameters));

EOF;

Expand Down Expand Up @@ -837,23 +839,17 @@ public function __construct()
*/
private function addFrozenConstructor()
{
$parameters = $this->exportParameters($this->container->getParameterBag()->all());

$code = <<<EOF

private \$parameters;
private static \$parameters = $parameters;

/**
* Constructor.
*/
public function __construct()
{
EOF;

if ($this->container->getParameterBag()->all()) {
$code .= "\n \$this->parameters = \$this->getDefaultParameters();\n";
}

$code .= <<<EOF

\$this->services =
\$this->scopedServices =
\$this->scopeStacks = array();
Expand Down Expand Up @@ -961,8 +957,6 @@ private function addDefaultParametersMethod()
return '';
}

$parameters = $this->exportParameters($this->container->getParameterBag()->all());

$code = '';
if ($this->container->isFrozen()) {
$code .= <<<EOF
Expand All @@ -974,11 +968,11 @@ public function getParameter(\$name)
{
\$name = strtolower(\$name);

if (!(isset(\$this->parameters[\$name]) || array_key_exists(\$name, \$this->parameters))) {
if (!(isset(self::\$parameters[\$name]) || array_key_exists(\$name, self::\$parameters))) {
throw new InvalidArgumentException(sprintf('The parameter "%s" must be defined.', \$name));
}

return \$this->parameters[\$name];
return self::\$parameters[\$name];
}

/**
Expand All @@ -988,7 +982,7 @@ public function hasParameter(\$name)
{
\$name = strtolower(\$name);

return isset(\$this->parameters[\$name]) || array_key_exists(\$name, \$this->parameters);
return isset(self::\$parameters[\$name]) || array_key_exists(\$name, self::\$parameters);
}

/**
Expand All @@ -1005,27 +999,14 @@ public function setParameter(\$name, \$value)
public function getParameterBag()
{
if (null === \$this->parameterBag) {
\$this->parameterBag = new FrozenParameterBag(\$this->parameters);
\$this->parameterBag = new FrozenParameterBag(self::\$parameters);
}

return \$this->parameterBag;
}
EOF;
}

$code .= <<<EOF

/**
* Gets the default parameters.
*
* @return array An array of the default parameters
*/
protected function getDefaultParameters()
{
return $parameters;
}

EOF;
}

return $code;
}
Expand Down
6 changes: 5 additions & 1 deletion src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services1-1.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,15 @@
*/
class Container extends AbstractContainer
{
private static $parameters = array(

);

/**
* Constructor.
*/
public function __construct()
{
parent::__construct();
parent::__construct(new ParameterBag(self::$parameters));
}
}
6 changes: 5 additions & 1 deletion src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services1.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,15 @@
*/
class ProjectServiceContainer extends Container
{
private static $parameters = array(

);

/**
* Constructor.
*/
public function __construct()
{
parent::__construct();
parent::__construct(new ParameterBag(self::$parameters));
}
}
27 changes: 8 additions & 19 deletions src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services10.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,16 @@
*/
class ProjectServiceContainer extends Container
{
private $parameters;
private static $parameters = array(
'empty_value' => '',
'some_string' => '-',
);

/**
* Constructor.
*/
public function __construct()
{
$this->parameters = $this->getDefaultParameters();

$this->services =
$this->scopedServices =
$this->scopeStacks = array();
Expand Down Expand Up @@ -68,11 +69,11 @@ public function getParameter($name)
{
$name = strtolower($name);

if (!(isset($this->parameters[$name]) || array_key_exists($name, $this->parameters))) {
if (!(isset(self::$parameters[$name]) || array_key_exists($name, self::$parameters))) {
throw new InvalidArgumentException(sprintf('The parameter "%s" must be defined.', $name));
}

return $this->parameters[$name];
return self::$parameters[$name];
}

/**
Expand All @@ -82,7 +83,7 @@ public function hasParameter($name)
{
$name = strtolower($name);

return isset($this->parameters[$name]) || array_key_exists($name, $this->parameters);
return isset(self::$parameters[$name]) || array_key_exists($name, self::$parameters);
}

/**
Expand All @@ -99,21 +100,9 @@ public function setParameter($name, $value)
public function getParameterBag()
{
if (null === $this->parameterBag) {
$this->parameterBag = new FrozenParameterBag($this->parameters);
$this->parameterBag = new FrozenParameterBag(self::$parameters);
}

return $this->parameterBag;
}
/**
* Gets the default parameters.
*
* @return array An array of the default parameters
*/
protected function getDefaultParameters()
{
return array(
'empty_value' => '',
'some_string' => '-',
);
}
}
4 changes: 3 additions & 1 deletion src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services11.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
*/
class ProjectServiceContainer extends Container
{
private $parameters;
private static $parameters = array(

);

/**
* Constructor.
Expand Down
24 changes: 8 additions & 16 deletions src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services8.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,7 @@
*/
class ProjectServiceContainer extends Container
{
/**
* Constructor.
*/
public function __construct()
{
parent::__construct(new ParameterBag($this->getDefaultParameters()));
}

/**
* Gets the default parameters.
*
* @return array An array of the default parameters
*/
protected function getDefaultParameters()
{
return array(
private static $parameters = array(
'foo' => '%baz%',
'baz' => 'bar',
'bar' => 'foo is %%foo bar',
Expand All @@ -47,5 +32,12 @@ protected function getDefaultParameters()
7 => 'null',
),
);

/**
* Constructor.
*/
public function __construct()
{
parent::__construct(new ParameterBag(self::$parameters));
}
}
22 changes: 7 additions & 15 deletions src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,18 @@
*/
class ProjectServiceContainer extends Container
{
private static $parameters = array(
'baz_class' => 'BazClass',
'foo_class' => 'Bar\\FooClass',
'foo' => 'bar',
);

/**
* Constructor.
*/
public function __construct()
{
parent::__construct(new ParameterBag($this->getDefaultParameters()));
parent::__construct(new ParameterBag(self::$parameters));
$this->methodMap = array(
'bar' => 'getBarService',
'baz' => 'getBazService',
Expand Down Expand Up @@ -370,18 +376,4 @@ protected function getNewFactoryService()

return $instance;
}

/**
* Gets the default parameters.
*
* @return array An array of the default parameters
*/
protected function getDefaultParameters()
{
return array(
'baz_class' => 'BazClass',
'foo_class' => 'Bar\\FooClass',
'foo' => 'bar',
);
}
}
29 changes: 9 additions & 20 deletions src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,17 @@
*/
class ProjectServiceContainer extends Container
{
private $parameters;
private static $parameters = array(
'baz_class' => 'BazClass',
'foo_class' => 'Bar\\FooClass',
'foo' => 'bar',
);

/**
* Constructor.
*/
public function __construct()
{
$this->parameters = $this->getDefaultParameters();

$this->services =
$this->scopedServices =
$this->scopeStacks = array();
Expand Down Expand Up @@ -320,11 +322,11 @@ public function getParameter($name)
{
$name = strtolower($name);

if (!(isset($this->parameters[$name]) || array_key_exists($name, $this->parameters))) {
if (!(isset(self::$parameters[$name]) || array_key_exists($name, self::$parameters))) {
throw new InvalidArgumentException(sprintf('The parameter "%s" must be defined.', $name));
}

return $this->parameters[$name];
return self::$parameters[$name];
}

/**
Expand All @@ -334,7 +336,7 @@ public function hasParameter($name)
{
$name = strtolower($name);

return isset($this->parameters[$name]) || array_key_exists($name, $this->parameters);
return isset(self::$parameters[$name]) || array_key_exists($name, self::$parameters);
}

/**
Expand All @@ -351,22 +353,9 @@ public function setParameter($name, $value)
public function getParameterBag()
{
if (null === $this->parameterBag) {
$this->parameterBag = new FrozenParameterBag($this->parameters);
$this->parameterBag = new FrozenParameterBag(self::$parameters);
}

return $this->parameterBag;
}
/**
* Gets the default parameters.
*
* @return array An array of the default parameters
*/
protected function getDefaultParameters()
{
return array(
'baz_class' => 'BazClass',
'foo_class' => 'Bar\\FooClass',
'foo' => 'bar',
);
}
}