[DependencyInjection] Perf php dumper #12855
Conversation
Conflicts: src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services11.php src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services19.php src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php
| * | ||
| * @return array An array of the default parameters | ||
| */ | ||
| protected function getDefaultParameters() |
There was a problem hiding this comment.
this is a bc break which cannot be done in 2.3
There was a problem hiding this comment.
the method could be kept for BC. It is not incompatible with the usage of target dirs
There was a problem hiding this comment.
Then why did we remove it in 2.6?
There was a problem hiding this comment.
Ah, and keep in mind that this function is dumped optionally, IF there is a parameter.
There was a problem hiding this comment.
Well, IIRC, we thought that removing it in the minor version was an acceptable BC break, because it breaks BC only for a really weird edge case: people writing a class extending the dumped one and changing/using the method to get default parameters
There was a problem hiding this comment.
ok great, then I'll revert this bc also
|
This is ready: I reverted the drop of getDefaultParameters() - there is no gain to expect from here, even in 2.6. --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services19.php
+++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services19.php
@@ -17,6 +17,7 @@ use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag;
class ProjectServiceContainer extends Container
{
private $parameters;
+ private $targetDirs = array();
/**
* Constructor. |
| // but every other sub-dir is optional up to the full path in $dir | ||
| // Mandate at least 2 root dirs and not more that 5 optional dirs. |
There was a problem hiding this comment.
Because we need a distinctive enough string: zero would mean matching a single DIRECTORY_SEPARATOR, which is not enough - and one looked too low also to me. Two should work for everyone - and be distinctive enough
This reverts commit c11535b. Conflicts: src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services1-1.php src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services1.php src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services10.php src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services12.php src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services8.php src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9.php src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services9_compiled.php
nicolas-grekas
force-pushed
the
perf-php-dumper
branch
from
929167d
to
375f83e
Compare
|
Thank you @nicolas-grekas. |
This PR was merged into the 2.3 branch. Discussion ---------- [DependencyInjection] Perf php dumper | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - This PR came up after this comment to reduce the number of calls to dirname(): #12784 (comment) Commits ------- 375f83e Revert "[DependencyInjection] backport perf optim" fcd8ff9 [DependencyInjection] perf optim: call dirname() at most 5x c11535b [DependencyInjection] backport perf optim
This PR came up after this comment to reduce the number of calls to dirname():
#12784 (comment)