Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 9e1b567

Browse filesBrowse files
fabpotfabpot
committed
feature #21755 [Routing] Optimised dumped router matcher, prevent unneeded function calls. (frankdejonge)
This PR was squashed before being merged into the 3.3-dev branch (closes #21755). Discussion ---------- [Routing] Optimised dumped router matcher, prevent unneeded function calls. | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | none | License | MIT | Doc PR | does not apply The application I'm working on is fairly large. Because we had a routing issue (not caused by the framework) I looked through the dumped routing code. I spotted some easy wins. These changes brought down the time for the `match` method to run from ~ 7.5ms to ~2.5ms. It's not a lot, but it's something. I've profiled it several times with blackfire to confirm. The results were very consistent. Mind you, our application has quite a serious amount of routes, a little over 900. Commits ------- dd647ff [Routing] Optimised dumped router matcher, prevent unneeded function calls.
2 parents 41fd5d1 + dd647ff commit 9e1b567
Copy full SHA for 9e1b567

File tree

6 files changed

+278
-70
lines changed
Filter options

6 files changed

+278
-70
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php
+48-14Lines changed: 48 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,16 @@ public function match(\$pathinfo)
105105
{
106106
\$allow = array();
107107
\$pathinfo = rawurldecode(\$pathinfo);
108+
\$trimmedPathinfo = rtrim(\$pathinfo, '/');
108109
\$context = \$this->context;
109110
\$request = \$this->request;
111+
\$requestMethod = \$canonicalMethod = \$context->getMethod();
112+
\$scheme = \$context->getScheme();
113+
114+
if ('HEAD' === \$requestMethod) {
115+
\$canonicalMethod = 'GET';
116+
}
117+
110118
111119
$code
112120
@@ -133,7 +141,7 @@ private function compileRoutes(RouteCollection $routes, $supportsRedirections)
133141
foreach ($groups as $collection) {
134142
if (null !== $regex = $collection->getAttribute('host_regex')) {
135143
if (!$fetchedHost) {
136-
$code .= " \$host = \$this->context->getHost();\n\n";
144+
$code .= " \$host = \$context->getHost();\n\n";
137145
$fetchedHost = true;
138146
}
139147

@@ -217,20 +225,15 @@ private function compileRoute(Route $route, $name, $supportsRedirections, $paren
217225
$hostMatches = false;
218226
$methods = $route->getMethods();
219227

220-
// GET and HEAD are equivalent
221-
if (in_array('GET', $methods) && !in_array('HEAD', $methods)) {
222-
$methods[] = 'HEAD';
223-
}
224-
225228
$supportsTrailingSlash = $supportsRedirections && (!$methods || in_array('HEAD', $methods));
226229
$regex = $compiledRoute->getRegex();
227230

228231
if (!count($compiledRoute->getPathVariables()) && false !== preg_match('#^(.)\^(?P<url>.*?)\$\1#'.(substr($regex, -1) === 'u' ? 'u' : ''), $regex, $m)) {
229232
if ($supportsTrailingSlash && substr($m['url'], -1) === '/') {
230-
$conditions[] = sprintf("rtrim(\$pathinfo, '/') === %s", var_export(rtrim(str_replace('\\', '', $m['url']), '/'), true));
233+
$conditions[] = sprintf('%s === $trimmedPathinfo', var_export(rtrim(str_replace('\\', '', $m['url']), '/'), true));
231234
$hasTrailingSlash = true;
232235
} else {
233-
$conditions[] = sprintf('$pathinfo === %s', var_export(str_replace('\\', '', $m['url']), true));
236+
$conditions[] = sprintf('%s === $pathinfo', var_export(str_replace('\\', '', $m['url']), true));
234237
}
235238
} else {
236239
if ($compiledRoute->getStaticPrefix() && $compiledRoute->getStaticPrefix() !== $parentPrefix) {
@@ -263,26 +266,57 @@ private function compileRoute(Route $route, $name, $supportsRedirections, $paren
263266
EOF;
264267

265268
$gotoname = 'not_'.preg_replace('/[^A-Za-z0-9_]/', '', $name);
269+
266270
if ($methods) {
267271
if (1 === count($methods)) {
268-
$code .= <<<EOF
269-
if (\$this->context->getMethod() != '$methods[0]') {
272+
if ($methods[0] === 'HEAD') {
273+
$code .= <<<EOF
274+
if ('HEAD' !== \$requestMethod) {
275+
\$allow[] = 'HEAD';
276+
goto $gotoname;
277+
}
278+
279+
280+
EOF;
281+
} else {
282+
$code .= <<<EOF
283+
if ('$methods[0]' !== \$canonicalMethod) {
270284
\$allow[] = '$methods[0]';
271285
goto $gotoname;
272286
}
273287
274288
275289
EOF;
290+
}
276291
} else {
277-
$methods = implode("', '", $methods);
278-
$code .= <<<EOF
279-
if (!in_array(\$this->context->getMethod(), array('$methods'))) {
292+
$methodVariable = 'requestMethod';
293+
294+
if (in_array('GET', $methods)) {
295+
// Since we treat HEAD requests like GET requests we don't need to match it.
296+
$methodVariable = 'canonicalMethod';
297+
$methods = array_filter($methods, function ($method) { return 'HEAD' !== $method; });
298+
}
299+
300+
if (1 === count($methods)) {
301+
$code .= <<<EOF
302+
if ('$methods[0]' !== \$$methodVariable) {
303+
\$allow[] = '$methods[0]';
304+
goto $gotoname;
305+
}
306+
307+
308+
EOF;
309+
} else {
310+
$methods = implode("', '", $methods);
311+
$code .= <<<EOF
312+
if (!in_array(\$$methodVariable, array('$methods'))) {
280313
\$allow = array_merge(\$allow, array('$methods'));
281314
goto $gotoname;
282315
}
283316
284317
285318
EOF;
319+
}
286320
}
287321
}
288322

@@ -303,7 +337,7 @@ private function compileRoute(Route $route, $name, $supportsRedirections, $paren
303337
$schemes = str_replace("\n", '', var_export(array_flip($schemes), true));
304338
$code .= <<<EOF
305339
\$requiredSchemes = $schemes;
306-
if (!isset(\$requiredSchemes[\$this->context->getScheme()])) {
340+
if (!isset(\$requiredSchemes[\$scheme])) {
307341
return \$this->redirect(\$pathinfo, '$name', key(\$requiredSchemes));
308342
}
309343

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher1.php
+33-25Lines changed: 33 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,16 @@ public function match($pathinfo)
2424
{
2525
$allow = array();
2626
$pathinfo = rawurldecode($pathinfo);
27+
$trimmedPathinfo = rtrim($pathinfo, '/');
2728
$context = $this->context;
2829
$request = $this->request;
30+
$requestMethod = $canonicalMethod = $context->getMethod();
31+
$scheme = $context->getScheme();
32+
33+
if ('HEAD' === $requestMethod) {
34+
$canonicalMethod = 'GET';
35+
}
36+
2937

3038
// foo
3139
if (0 === strpos($pathinfo, '/foo') && preg_match('#^/foo/(?P<bar>baz|symfony)$#s', $pathinfo, $matches)) {
@@ -35,8 +43,8 @@ public function match($pathinfo)
3543
if (0 === strpos($pathinfo, '/bar')) {
3644
// bar
3745
if (preg_match('#^/bar/(?P<foo>[^/]++)$#s', $pathinfo, $matches)) {
38-
if (!in_array($this->context->getMethod(), array('GET', 'HEAD'))) {
39-
$allow = array_merge($allow, array('GET', 'HEAD'));
46+
if ('GET' !== $canonicalMethod) {
47+
$allow[] = 'GET';
4048
goto not_bar;
4149
}
4250

@@ -46,8 +54,8 @@ public function match($pathinfo)
4654

4755
// barhead
4856
if (0 === strpos($pathinfo, '/barhead') && preg_match('#^/barhead/(?P<foo>[^/]++)$#s', $pathinfo, $matches)) {
49-
if (!in_array($this->context->getMethod(), array('GET', 'HEAD'))) {
50-
$allow = array_merge($allow, array('GET', 'HEAD'));
57+
if ('GET' !== $canonicalMethod) {
58+
$allow[] = 'GET';
5159
goto not_barhead;
5260
}
5361

@@ -60,17 +68,17 @@ public function match($pathinfo)
6068
if (0 === strpos($pathinfo, '/test')) {
6169
if (0 === strpos($pathinfo, '/test/baz')) {
6270
// baz
63-
if ($pathinfo === '/test/baz') {
71+
if ('/test/baz' === $pathinfo) {
6472
return array('_route' => 'baz');
6573
}
6674

6775
// baz2
68-
if ($pathinfo === '/test/baz.html') {
76+
if ('/test/baz.html' === $pathinfo) {
6977
return array('_route' => 'baz2');
7078
}
7179

7280
// baz3
73-
if ($pathinfo === '/test/baz3/') {
81+
if ('/test/baz3/' === $pathinfo) {
7482
return array('_route' => 'baz3');
7583
}
7684

@@ -83,7 +91,7 @@ public function match($pathinfo)
8391

8492
// baz5
8593
if (preg_match('#^/test/(?P<foo>[^/]++)/$#s', $pathinfo, $matches)) {
86-
if ($this->context->getMethod() != 'POST') {
94+
if ('POST' !== $canonicalMethod) {
8795
$allow[] = 'POST';
8896
goto not_baz5;
8997
}
@@ -94,7 +102,7 @@ public function match($pathinfo)
94102

95103
// baz.baz6
96104
if (preg_match('#^/test/(?P<foo>[^/]++)/$#s', $pathinfo, $matches)) {
97-
if ($this->context->getMethod() != 'PUT') {
105+
if ('PUT' !== $canonicalMethod) {
98106
$allow[] = 'PUT';
99107
goto not_bazbaz6;
100108
}
@@ -106,7 +114,7 @@ public function match($pathinfo)
106114
}
107115

108116
// foofoo
109-
if ($pathinfo === '/foofoo') {
117+
if ('/foofoo' === $pathinfo) {
110118
return array ( 'def' => 'test', '_route' => 'foofoo',);
111119
}
112120

@@ -116,7 +124,7 @@ public function match($pathinfo)
116124
}
117125

118126
// space
119-
if ($pathinfo === '/spa ce') {
127+
if ('/spa ce' === $pathinfo) {
120128
return array('_route' => 'space');
121129
}
122130

@@ -161,12 +169,12 @@ public function match($pathinfo)
161169
}
162170

163171
// overridden2
164-
if ($pathinfo === '/multi/new') {
172+
if ('/multi/new' === $pathinfo) {
165173
return array('_route' => 'overridden2');
166174
}
167175

168176
// hey
169-
if ($pathinfo === '/multi/hey/') {
177+
if ('/multi/hey/' === $pathinfo) {
170178
return array('_route' => 'hey');
171179
}
172180

@@ -184,7 +192,7 @@ public function match($pathinfo)
184192

185193
if (0 === strpos($pathinfo, '/aba')) {
186194
// ababa
187-
if ($pathinfo === '/ababa') {
195+
if ('/ababa' === $pathinfo) {
188196
return array('_route' => 'ababa');
189197
}
190198

@@ -195,59 +203,59 @@ public function match($pathinfo)
195203

196204
}
197205

198-
$host = $this->context->getHost();
206+
$host = $context->getHost();
199207

200208
if (preg_match('#^a\\.example\\.com$#si', $host, $hostMatches)) {
201209
// route1
202-
if ($pathinfo === '/route1') {
210+
if ('/route1' === $pathinfo) {
203211
return array('_route' => 'route1');
204212
}
205213

206214
// route2
207-
if ($pathinfo === '/c2/route2') {
215+
if ('/c2/route2' === $pathinfo) {
208216
return array('_route' => 'route2');
209217
}
210218

211219
}
212220

213221
if (preg_match('#^b\\.example\\.com$#si', $host, $hostMatches)) {
214222
// route3
215-
if ($pathinfo === '/c2/route3') {
223+
if ('/c2/route3' === $pathinfo) {
216224
return array('_route' => 'route3');
217225
}
218226

219227
}
220228

221229
if (preg_match('#^a\\.example\\.com$#si', $host, $hostMatches)) {
222230
// route4
223-
if ($pathinfo === '/route4') {
231+
if ('/route4' === $pathinfo) {
224232
return array('_route' => 'route4');
225233
}
226234

227235
}
228236

229237
if (preg_match('#^c\\.example\\.com$#si', $host, $hostMatches)) {
230238
// route5
231-
if ($pathinfo === '/route5') {
239+
if ('/route5' === $pathinfo) {
232240
return array('_route' => 'route5');
233241
}
234242

235243
}
236244

237245
// route6
238-
if ($pathinfo === '/route6') {
246+
if ('/route6' === $pathinfo) {
239247
return array('_route' => 'route6');
240248
}
241249

242250
if (preg_match('#^(?P<var1>[^\\.]++)\\.example\\.com$#si', $host, $hostMatches)) {
243251
if (0 === strpos($pathinfo, '/route1')) {
244252
// route11
245-
if ($pathinfo === '/route11') {
253+
if ('/route11' === $pathinfo) {
246254
return $this->mergeDefaults(array_replace($hostMatches, array('_route' => 'route11')), array ());
247255
}
248256

249257
// route12
250-
if ($pathinfo === '/route12') {
258+
if ('/route12' === $pathinfo) {
251259
return $this->mergeDefaults(array_replace($hostMatches, array('_route' => 'route12')), array ( 'var1' => 'val',));
252260
}
253261

@@ -280,15 +288,15 @@ public function match($pathinfo)
280288
}
281289

282290
// route17
283-
if ($pathinfo === '/route17') {
291+
if ('/route17' === $pathinfo) {
284292
return array('_route' => 'route17');
285293
}
286294

287295
}
288296

289297
if (0 === strpos($pathinfo, '/a')) {
290298
// a
291-
if ($pathinfo === '/a/a...') {
299+
if ('/a/a...' === $pathinfo) {
292300
return array('_route' => 'a');
293301
}
294302

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.