8000 merged branch Tobion/PhpMatcherDumper-Improvement (PR #3763) · marcw/symfony@2620413 · GitHub
[go: up one dir, main page]
Skip to content

Commit 2620413

Browse files
fabpotfabpot
committed
merged branch Tobion/PhpMatcherDumper-Improvement (PR symfony#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.
2 parents 4923483 + 56c1e31 commit 2620413

File tree

5 files changed

+49
-51
lines changed

5 files changed

+49
-51
lines changed

‎src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php

Lines changed: 45 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public function dump(array $options = array())
5555
private function addMatcher($supportsRedirections)
5656
{
5757
// we need to deep clone the routes as we will modify the structure to optimize the dump
58-
$code = implode("\n", $this->compileRoutes(clone $this->getRoutes(), $supportsRedirections));
58+
$code = rtrim($this->compileRoutes(clone $this->getRoutes(), $supportsRedirections), "\n");
5959

6060
return <<<EOF
6161
@@ -65,6 +65,7 @@ public function match(\$pathinfo)
6565
\$pathinfo = urldecode(\$pathinfo);
6666
6767
$code
68+
6869
throw 0 < count(\$allow) ? new MethodNotAllowedException(array_unique(\$allow)) : new ResourceNotFoundException();
6970
}
7071
@@ -73,7 +74,7 @@ public function match(\$pathinfo)
7374

7475
private function compileRoutes(RouteCollection $routes, $supportsRedirections, $parentPrefix = null)
7576
{
76-
$code = array();
77+
$code = '';
7778

7879
$routeIterator = $routes->getIterator();
7980
$keys = array_keys($routeIterator->getArrayCopy());
@@ -86,10 +87,11 @@ private function compileRoutes(RouteCollection $routes, $supportsRedirections, $
8687
if ($route instanceof RouteCollection) {
8788
$prefix = $route->getPrefix();
8889
$optimizable = $prefix && count($route->all()) > 1 && false === strpos($route->getPrefix(), '{');
89-
$indent = '';
90+
$optimized = false;
91+
9092
if ($optimizable) {
9193
for ($j = $i; $j < $keysCount; $j++) {
92-
if ($keys[$j] === null) {
94+
if (null === $keys[$j]) {
9395
continue;
9496
}
9597

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

115117
if ($prefix !== $parentPrefix) {
116-
$code[] = sprintf(" if (0 === strpos(\$pathinfo, %s)) {", var_export($prefix, true));
117-
$indent = ' ';
118+
$code .= sprintf(" if (0 === strpos(\$pathinfo, %s)) {\n", var_export($prefix, true));
119+
$optimized = true;
118120
}
119121
}
120122

121-
foreach ($this->compileRoutes($route, $supportsRedirections, $prefix) as $line) {
122-
foreach (explode("\n", $line) as $l) {
123-
if ($l) {
124-
$code[] = $indent.$l;
125-
} else {
126-
$code[] = $l;
123+
if ($optimized) {
124+
foreach (explode("\n", $this->compileRoutes($route, $supportsRedirections, $prefix)) as $line) {
125+
if ('' !== $line) {
126+
$code .= ' '; // apply extra indention
127127
}
128+
$code .= $line."\n";
128129
}
129-
}
130-
131-
if ($optimizable && $prefix !== $parentPrefix) {
132-
$code[] = " }\n";
130+
$code = substr($code, 0, -2); // remove redundant last two line breaks
131+
$code .= " }\n\n";
132+
} else {
133+
$code .= $this->compileRoutes($route, $supportsRedirections, $prefix);
133134
}
134135
} else {
135-
foreach ($this->compileRoute($route, $name, $supportsRedirections, $parentPrefix) as $line) {
136-
$code[] = $line;
137-
}
136+
$code .= $this->compileRoute($route, $name, $supportsRedirections, $parentPrefix)."\n";
138137
}
139138
}
140139

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

144143
private function compileRoute(Route $route, $name, $supportsRedirections, $parentPrefix = null)
145144
{
146-
$code = array();
145+
$code = '';
147146
$compiledRoute = $route->compile();
148147
$conditions = array();
149148
$hasTrailingSlash = false;
150149
$matches = false;
151150
$methods = array();
151+
152152
if ($req = $route->getRequirement('_method')) {
153153
$methods = explode('|', strtoupper($req));
154154
// GET and HEAD are equivalent
155155
if (in_array('GET', $methods) && !in_array('HEAD', $methods)) {
156156
$methods[] = 'HEAD';
157157
}
158158
}
159+
159160
$supportsTrailingSlash = $supportsRedirections && (!$methods || in_array('HEAD', $methods));
160161

161162
if (!count($compiledRoute->getVariables()) && false !== preg_match('#^(.)\^(?P<url>.*?)\$\1#', str_replace(array("\n", ' '), '', $compiledRoute->getRegex()), $m)) {
@@ -182,74 +183,75 @@ private function compileRoute(Route $route, $name, $supportsRedirections, $paren
182183

183184
$conditions = implode(' && ', $conditions);
184185

185-
$gotoname = 'not_'.preg_replace('/[^A-Za-z0-9_]/', '', $name);
186-
187-
$code[] = <<<EOF
186+
$code .= <<<EOF
188187
// $name
189188
if ($conditions) {
189+
190190
EOF;
191191

192192
if ($methods) {
193+
$gotoname = 'not_'.preg_replace('/[^A-Za-z0-9_]/', '', $name);
194+
193195
if (1 === count($methods)) {
194-
$code[] = <<<EOF
196+
$code .= <<<EOF
195197
if (\$this->context->getMethod() != '$methods[0]') {
196198
\$allow[] = '$methods[0]';
197199
goto $gotoname;
198200
}
201+
199202
EOF;
200203
} else {
201-
$methods = implode('\', \'', $methods);
202-
$code[] = <<<EOF
204+
$methods = implode("', '", $methods);
205+
$code .= <<<EOF
203206
if (!in_array(\$this->context->getMethod(), array('$methods'))) {
204207
\$allow = array_merge(\$allow, array('$methods'));
205208
goto $gotoname;
206209
}
210+
207211
EOF;
208212
}
209213
}
210214

211215
if ($hasTrailingSlash) {
212-
$code[] = sprintf(<<<EOF
216+
$code .= <<<EOF
213217
if (substr(\$pathinfo, -1) !== '/') {
214-
return \$this->redirect(\$pathinfo.'/', '%s');
218+
return \$this->redirect(\$pathinfo.'/', '$name');
215219
}
216-
EOF
217-
, $name);
220+
221+
EOF;
218222
}
219223

220224
if ($scheme = $route->getRequirement('_scheme')) {
221225
if (!$supportsRedirections) {
222226
throw new \LogicException('The "_scheme" requirement is only supported for URL matchers that implement RedirectableUrlMatcherInterface.');
223227
}
224228

225-
$code[] = sprintf(<<<EOF
229+
$code .= <<<EOF
226230
if (\$this->context->getScheme() !== '$scheme') {
227-
return \$this->redirect(\$pathinfo, '%s', '$scheme');
231+
return \$this->redirect(\$pathinfo, '$name', '$scheme');
228232
}
229-
EOF
230-
, $name);
233+
234+
EOF;
231235
}
232236

233237
// optimize parameters array
234238
if (true === $matches && $compiledRoute->getDefaults()) {
235-
$code[] = sprintf(" return array_merge(\$this->mergeDefaults(\$matches, %s), array('_route' => '%s'));"
239+
$code .= sprintf(" return array_merge(\$this->mergeDefaults(\$matches, %s), array('_route' => '%s'));\n"
236240
, str_replace("\n", '', var_export($compiledRoute->getDefaults(), true)), $name);
237241
} elseif (true === $matches) {
238-
$code[] = sprintf(" \$matches['_route'] = '%s';", $name);
239-
$code[] = sprintf(" return \$matches;", $name);
242+
$code .= sprintf(" \$matches['_route'] = '%s';\n", $name);
243+
$code .= " return \$matches;\n";
240244
} elseif ($compiledRoute->getDefaults()) {
241-
$code[] = sprintf(' return %s;', str_replace("\n", '', var_export(array_merge($compiledRoute->getDefaults(), array('_route' => $name)), true)));
245+
$code .= sprintf(" return %s;\n", str_replace("\n", '', var_export(array_merge($compiledRoute->getDefaults(), array('_route' => $name)), true)));
242246
} else {
243-
$code[] = sprintf(" return array('_route' => '%s');", $name);
247+
$code .= sprintf(" return array('_route' => '%s');\n", $name);
244248
}
245-
$code[] = " }";
249+
$code .= " }\n";
246250

247251
if ($methods) {
248-
$code[] = " $gotoname:";
252+
$code .= " $gotoname:\n";
249253
}
250254

251-
$code[] = '';
252-
253255
return $code;
254256
}
255257

‎src/Symfony/Component/Routing/RouteCollection.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public function setParent(RouteCollection $parent)
7373
}
7474

7575
/**
76-
* Gets the current RouteCollection as an Iterator.
76+
* Gets the current RouteCollection as an Iterator that includes all routes and child route collections.
7777
*
7878
* @return \ArrayIterator An \ArrayIterator interface
7979
*/

‎src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher1.php

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,6 @@ public function match($pathinfo)
131131
$matches['_route'] = 'bar2';
132132
return $matches;
133133
}
134-
135134
}
136135

137136
// overriden
@@ -149,7 +148,6 @@ public function match($pathinfo)
149148
$matches['_route'] = 'foo4';
150149
return $matches;
151150
}
152-
153151
}
154152

155153
// foo3

‎src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher2.php

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,6 @@ public function match($pathinfo)
137137
$matches['_route'] = 'bar2';
138138
return $matches;
139139
}
140-
141140
}
142141

143142
// overriden
@@ -155,7 +154,6 @@ public function match($pathinfo)
155154
$matches['_route'] = 'foo4';
156155
return $matches;
157156
}
158-
159157
}
160158

161159
// foo3

‎src/Symfony/Component/Routing/Tests/Matcher/Dumper/PhpMatcherDumperTest.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,11 @@ class PhpMatcherDumperTest extends \PHPUnit_Framework_TestCase
2020
{
2121
public function testDump()
2222
{
23-
$dumper = new PhpMatcherDumper($this->getRouteCollection(), new RequestContext());
23+
$collection = $this->getRouteCollection();
2424

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

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

2929
// force HTTPS redirection
3030
$collection->add('secure', new Route(

0 commit comments

Comments
 (0)
0