8000 performance improvement in PhpMatcherDumper by Tobion · Pull Request #3763 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

performance improvement in PhpMatcherDumper #3763

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
Apr 3, 2012
Merged

performance improvement in PhpMatcherDumper #3763

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
88 changes: 45 additions & 43 deletions src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php
View file
Open in desktop
10000
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public function dump(array $options = array())
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

Expand All @@ -65,6 +65,7 @@ public function match(\$pathinfo)
\$pathinfo = urldecode(\$pathinfo);

$code

throw 0 < count(\$allow) ? new MethodNotAllowedException(array_unique(\$allow)) : new ResourceNotFoundException();
}

Expand All @@ -73,7 +74,7 @@ public function match(\$pathinfo)

private function compileRoutes(RouteCollection $routes, $supportsRedirections, $parentPrefix = null)
{
$code = array();
$code = '';

$routeIterator = $routes->getIterator();
$keys = array_keys($routeIterator->getArrayCopy());
Expand All @@ -86,10 +87,11 @@ private function compileRoutes(RouteCollection $routes, $supportsRedirections, $
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;
}

Expand All @@ -113,28 +115,25 @@ private function compileRoutes(RouteCollection $routes, $supportsRedirections, $
}

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";
}
}

Expand All @@ -143,19 +142,21 @@ private function compileRoutes(RouteCollection $routes, $supportsRedirections, $

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
if (in_array('GET', $methods) && !in_array('HEAD', $methods)) {
$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)) {
Expand All @@ -182,74 +183,75 @@ private function compileRoute(Route $route, $name, $supportsRedirections, $paren

$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')) {
if (!$supportsRedirections) {
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;
}

Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Routing/RouteCollection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public function setParent(RouteCollection $parent)
}

/**
* 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
*/
Expand Down
2 changes: 0 additions & 2 deletions src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher1.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,6 @@ public function match($pathinfo)
$matches['_route'] = 'bar2';
return $matches;
}

}

// overriden
Expand All @@ -149,7 +148,6 @@ public function match($pathinfo)
$matches['_route'] = 'foo4';
return $matches;
}

}

// foo3
Expand Down
2 changes: 0 additions & 2 deletions src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher2.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ public function match($pathinfo)
$matches['_route'] = 'bar2';
return $matches;
}

}

// overriden
Expand All @@ -155,7 +154,6 @@ public function match($pathinfo)
$matches['_route'] = 'foo4';
return $matches;
}

}

// foo3
Expand Down
6 changes: 3 additions & 3 deletions src/Symfony/Component/Routing/Tests/Matcher/Dumper/PhpMatcherDumperTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ class PhpMatcherDumperTest extends \PHPUnit_Framework_TestCase
{
public function testDump()
{
$dumper = new PhpMatcherDumper($this->getRouteCollection(), new RequestContext());
$collection = $this->getRouteCollection();

$this->assertStringEqualsFile(__DIR__.'/../../Fixtures/dumper/url_matcher1.php', $dumper->dump(), '->dump() dumps basic routes to the correct PHP file.');
$dumper = new PhpMatcherDumper($collection, new RequestContext());

$collection = $this->getRouteCollection();
$this->assertStringEqualsFile(__DIR__.'/../../Fixtures/dumper/url_matcher1.php', $dumper->dump(), '->dump() dumps basic routes to the correct PHP file.');

// force HTTPS redirection
$collection->add('secure', new Route(
Expand Down
0