bug #20374 [FrameworkBundle] Improve performance of ControllerNameParser (enumag)
This PR was merged into the 2.7 branch.
Discussion
----------
[FrameworkBundle] Improve performance of ControllerNameParser
| Q | A
| ------------- | ---
| Branch? | 2.7
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets |
| License | MIT
| Doc PR |
Today I was searching for bottlenecks in my application using Blackfire. And among other things I found one in Symfony. Blackfire showed that `Symfony\Bundle\FrameworkBundle\Controller\ControllerNameParser::findAlternative()` was called almost 300 times which took 28 miliseconds.
It turns out that `Symfony\Bundle\FrameworkBundle\Routing\DelegatingLoader::load()` is calling `ControllerNameParser::parse()` without actually needing to do so because `$controller` is in the class::method notation already. `ControllerNameParser` threw an exception, DelegatingLoader caught and ignored it - that's ok. The problem is that generating the exception message took a lot of time because findAlternative is slow. In my case it called the levenshtein function over 5000 times which was completely useless because the exception is ignored anyway.
Commits
-------
cf333f3
[FrameworkBundle] Improve performance of ControllerNameParser
This commit is contained in:
commit
e62b602dc4
@ -46,11 +46,12 @@ class ControllerNameParser
|
|||||||
*/
|
*/
|
||||||
public function parse($controller)
|
public function parse($controller)
|
||||||
{
|
{
|
||||||
$originalController = $controller;
|
$parts = explode(':', $controller);
|
||||||
if (3 !== count($parts = explode(':', $controller))) {
|
if (3 !== count($parts) || in_array('', $parts, true)) {
|
||||||
throw new \InvalidArgumentException(sprintf('The "%s" controller is not a valid "a:b:c" controller string.', $controller));
|
throw new \InvalidArgumentException(sprintf('The "%s" controller is not a valid "a:b:c" controller string.', $controller));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
$originalController = $controller;
|
||||||
list($bundle, $controller, $action) = $parts;
|
list($bundle, $controller, $action) = $parts;
|
||||||
$controller = str_replace('/', '\\', $controller);
|
$controller = str_replace('/', '\\', $controller);
|
||||||
$bundles = array();
|
$bundles = array();
|
||||||
|
@ -21,7 +21,7 @@ use Psr\Log\LoggerInterface;
|
|||||||
* DelegatingLoader delegates route loading to other loaders using a loader resolver.
|
* DelegatingLoader delegates route loading to other loaders using a loader resolver.
|
||||||
*
|
*
|
||||||
* This implementation resolves the _controller attribute from the short notation
|
* This implementation resolves the _controller attribute from the short notation
|
||||||
* to the fully-qualified form (from a:b:c to class:method).
|
* to the fully-qualified form (from a:b:c to class::method).
|
||||||
*
|
*
|
||||||
* @author Fabien Potencier <fabien@symfony.com>
|
* @author Fabien Potencier <fabien@symfony.com>
|
||||||
*/
|
*/
|
||||||
@ -85,7 +85,10 @@ class DelegatingLoader extends BaseDelegatingLoader
|
|||||||
$this->loading = false;
|
$this->loading = false;
|
||||||
|
|
||||||
foreach ($collection->all() as $route) {
|
foreach ($collection->all() as $route) {
|
||||||
if ($controller = $route->getDefault('_controller')) {
|
if (!$controller = $route->getDefault('_controller')) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
try {
|
try {
|
||||||
$controller = $this->parser->parse($controller);
|
$controller = $this->parser->parse($controller);
|
||||||
} catch (\InvalidArgumentException $e) {
|
} catch (\InvalidArgumentException $e) {
|
||||||
@ -94,7 +97,6 @@ class DelegatingLoader extends BaseDelegatingLoader
|
|||||||
|
|
||||||
$route->setDefault('_controller', $controller);
|
$route->setDefault('_controller', $controller);
|
||||||
}
|
}
|
||||||
}
|
|
||||||
|
|
||||||
return $collection;
|
return $collection;
|
||||||
}
|
}
|
||||||
|
Reference in New Issue
Block a user