merged branch Tobion/PhpMatcherDumper-Improvement (PR #3763)

Commits
-------

56c1e31 performance improvement in PhpMatcherDumper

Discussion
----------

performance improvement in PhpMatcherDumper

Tests pass: yes

The code generation uses a string internally instead of an array. The array wasn't used for random access anyway.
I also removed 4 unneeded iterations this way (when imploding, when merging and twice when applying the extra indention). A `preg_replace` could also be saved under certain circumstances by moving it.
And there was a small code errror in line 139.
This commit is contained in:
Fabien Potencier 2012-04-03 11:38:13 +02:00
commit 262041382f
5 changed files with 49 additions and 51 deletions

View File

@ -55,7 +55,7 @@ class PhpMatcherDumper extends MatcherDumper
private function addMatcher($supportsRedirections)
{
// we need to deep clone the routes as we will modify the structure to optimize the dump
$code = implode("\n", $this->compileRoutes(clone $this->getRoutes(), $supportsRedirections));
$code = rtrim($this->compileRoutes(clone $this->getRoutes(), $supportsRedirections), "\n");
return <<<EOF
@ -65,6 +65,7 @@ class PhpMatcherDumper extends MatcherDumper
\$pathinfo = urldecode(\$pathinfo);
$code
throw 0 < count(\$allow) ? new MethodNotAllowedException(array_unique(\$allow)) : new ResourceNotFoundException();
}
@ -73,7 +74,7 @@ EOF;
private function compileRoutes(RouteCollection $routes, $supportsRedirections, $parentPrefix = null)
{
$code = array();
$code = '';
$routeIterator = $routes->getIterator();
$keys = array_keys($routeIterator->getArrayCopy());
@ -86,10 +87,11 @@ EOF;
if ($route instanceof RouteCollection) {
$prefix = $route->getPrefix();
$optimizable = $prefix && count($route->all()) > 1 && false === strpos($route->getPrefix(), '{');
$indent = '';
$optimized = false;
if ($optimizable) {
for ($j = $i; $j < $keysCount; $j++) {
if ($keys[$j] === null) {
if (null === $keys[$j]) {
continue;
}
@ -113,28 +115,25 @@ EOF;
}
if ($prefix !== $parentPrefix) {
$code[] = sprintf(" if (0 === strpos(\$pathinfo, %s)) {", var_export($prefix, true));
$indent = ' ';
$code .= sprintf(" if (0 === strpos(\$pathinfo, %s)) {\n", var_export($prefix, true));
$optimized = true;
}
}
foreach ($this->compileRoutes($route, $supportsRedirections, $prefix) as $line) {
foreach (explode("\n", $line) as $l) {
if ($l) {
$code[] = $indent.$l;
} else {
$code[] = $l;
if ($optimized) {
foreach (explode("\n", $this->compileRoutes($route, $supportsRedirections, $prefix)) as $line) {
if ('' !== $line) {
$code .= ' '; // apply extra indention
}
$code .= $line."\n";
}
}
if ($optimizable && $prefix !== $parentPrefix) {
$code[] = " }\n";
$code = substr($code, 0, -2); // remove redundant last two line breaks
$code .= " }\n\n";
} else {
$code .= $this->compileRoutes($route, $supportsRedirections, $prefix);
}
} else {
foreach ($this->compileRoute($route, $name, $supportsRedirections, $parentPrefix) as $line) {
$code[] = $line;
}
$code .= $this->compileRoute($route, $name, $supportsRedirections, $parentPrefix)."\n";
}
}
@ -143,12 +142,13 @@ EOF;
private function compileRoute(Route $route, $name, $supportsRedirections, $parentPrefix = null)
{
$code = array();
$code = '';
$compiledRoute = $route->compile();
$conditions = array();
$hasTrailingSlash = false;
$matches = false;
$methods = array();
if ($req = $route->getRequirement('_method')) {
$methods = explode('|', strtoupper($req));
// GET and HEAD are equivalent
@ -156,6 +156,7 @@ EOF;
$methods[] = 'HEAD';
}
}
$supportsTrailingSlash = $supportsRedirections && (!$methods || in_array('HEAD', $methods));
if (!count($compiledRoute->getVariables()) && false !== preg_match('#^(.)\^(?P<url>.*?)\$\1#', str_replace(array("\n", ' '), '', $compiledRoute->getRegex()), $m)) {
@ -182,39 +183,42 @@ EOF;
$conditions = implode(' && ', $conditions);
$gotoname = 'not_'.preg_replace('/[^A-Za-z0-9_]/', '', $name);
$code[] = <<<EOF
$code .= <<<EOF
// $name
if ($conditions) {
EOF;
if ($methods) {
$gotoname = 'not_'.preg_replace('/[^A-Za-z0-9_]/', '', $name);
if (1 === count($methods)) {
$code[] = <<<EOF
$code .= <<<EOF
if (\$this->context->getMethod() != '$methods[0]') {
\$allow[] = '$methods[0]';
goto $gotoname;
}
EOF;
} else {
$methods = implode('\', \'', $methods);
$code[] = <<<EOF
$methods = implode("', '", $methods);
$code .= <<<EOF
if (!in_array(\$this->context->getMethod(), array('$methods'))) {
\$allow = array_merge(\$allow, array('$methods'));
goto $gotoname;
}
EOF;
}
}
if ($hasTrailingSlash) {
$code[] = sprintf(<<<EOF
$code .= <<<EOF
if (substr(\$pathinfo, -1) !== '/') {
return \$this->redirect(\$pathinfo.'/', '%s');
return \$this->redirect(\$pathinfo.'/', '$name');
}
EOF
, $name);
EOF;
}
if ($scheme = $route->getRequirement('_scheme')) {
@ -222,34 +226,32 @@ EOF
throw new \LogicException('The "_scheme" requirement is only supported for URL matchers that implement RedirectableUrlMatcherInterface.');
}
$code[] = sprintf(<<<EOF
$code .= <<<EOF
if (\$this->context->getScheme() !== '$scheme') {
return \$this->redirect(\$pathinfo, '%s', '$scheme');
return \$this->redirect(\$pathinfo, '$name', '$scheme');
}
EOF
, $name);
EOF;
}
// optimize parameters array
if (true === $matches && $compiledRoute->getDefaults()) {
$code[] = sprintf(" return array_merge(\$this->mergeDefaults(\$matches, %s), array('_route' => '%s'));"
$code .= sprintf(" return array_merge(\$this->mergeDefaults(\$matches, %s), array('_route' => '%s'));\n"
, str_replace("\n", '', var_export($compiledRoute->getDefaults(), true)), $name);
} elseif (true === $matches) {
$code[] = sprintf(" \$matches['_route'] = '%s';", $name);
$code[] = sprintf(" return \$matches;", $name);
$code .= sprintf(" \$matches['_route'] = '%s';\n", $name);
$code .= " return \$matches;\n";
} elseif ($compiledRoute->getDefaults()) {
$code[] = sprintf(' return %s;', str_replace("\n", '', var_export(array_merge($compiledRoute->getDefaults(), array('_route' => $name)), true)));
$code .= sprintf(" return %s;\n", str_replace("\n", '', var_export(array_merge($compiledRoute->getDefaults(), array('_route' => $name)), true)));
} else {
$code[] = sprintf(" return array('_route' => '%s');", $name);
$code .= sprintf(" return array('_route' => '%s');\n", $name);
}
$code[] = " }";
$code .= " }\n";
if ($methods) {
$code[] = " $gotoname:";
$code .= " $gotoname:\n";
}
$code[] = '';
return $code;
}

View File

@ -73,7 +73,7 @@ class RouteCollection implements \IteratorAggregate
}
/**
* Gets the current RouteCollection as an Iterator.
* Gets the current RouteCollection as an Iterator that includes all routes and child route collections.
*
* @return \ArrayIterator An \ArrayIterator interface
*/

View File

@ -131,7 +131,6 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Matcher\UrlMatcher
$matches['_route'] = 'bar2';
return $matches;
}
}
// overriden
@ -149,7 +148,6 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Matcher\UrlMatcher
$matches['_route'] = 'foo4';
return $matches;
}
}
// foo3

View File

@ -137,7 +137,6 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Tests\Fixtures\Redirec
$matches['_route'] = 'bar2';
return $matches;
}
}
// overriden
@ -155,7 +154,6 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Tests\Fixtures\Redirec
$matches['_route'] = 'foo4';
return $matches;
}
}
// foo3

View File

@ -20,12 +20,12 @@ class PhpMatcherDumperTest extends \PHPUnit_Framework_TestCase
{
public function testDump()
{
$dumper = new PhpMatcherDumper($this->getRouteCollection(), new RequestContext());
$collection = $this->getRouteCollection();
$dumper = new PhpMatcherDumper($collection, new RequestContext());
$this->assertStringEqualsFile(__DIR__.'/../../Fixtures/dumper/url_matcher1.php', $dumper->dump(), '->dump() dumps basic routes to the correct PHP file.');
$collection = $this->getRouteCollection();
// force HTTPS redirection
$collection->add('secure', new Route(
'/secure',