Skip to content
This repository
Browse code

[Routing] improve matching performance by using possesive quantifiers…

… when possible (closes #5471)

My benchmarks showed a performance improvement of 20% when matching routes that make use of possesive quantifiers because it prevents backtracking when it's not needed
  • Loading branch information...
commit 4eee88f22bc8b19a5cc17ba8280c5f64dfe821f4 1 parent a3147e9
Tobias Schultze authored September 08, 2012 fabpot committed October 03, 2012
8  src/Symfony/Component/Routing/RouteCompiler.php
@@ -79,6 +79,14 @@ public function compile(Route $route)
79 79
                 // part of {_format} when generating the URL, e.g. _format = 'mobile.html'.
80 80
                 $nextSeparator = $this->findNextSeparator($followingPattern);
81 81
                 $regexp = sprintf('[^/%s]+', '/' !== $nextSeparator && '' !== $nextSeparator ? preg_quote($nextSeparator, self::REGEX_DELIMITER) : '');
  82
+                if (('' !== $nextSeparator && !preg_match('#^\{\w+\}#', $followingPattern)) || '' === $followingPattern) {
  83
+                    // When we have a separator, which is disallowed for the variable, we can optimize the regex with a possessive
  84
+                    // quantifier. This prevents useless backtracking of PCRE and improves performance by 20% for matching those patterns.
  85
+                    // Given the above example, there is no point in backtracking into {page} (that forbids the dot) when a dot must follow 
  86
+                    // after it. This optimization cannot be applied when the next char is no real separator or when the next variable is
  87
+                    // directly adjacent, e.g. '/{x}{y}'.
  88
+                    $regexp .= '+';
  89
+                }
82 90
             }
83 91
 
84 92
             $tokens[] = array('variable', $isSeparator ? $precedingChar : '', $regexp, $varName);
24  src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher1.apache
@@ -7,21 +7,21 @@ RewriteCond %{REQUEST_URI} ^/foo/(baz|symfony)$
7 7
 RewriteRule .* app.php [QSA,L,E=_ROUTING__route:foo,E=_ROUTING_bar:%1,E=_ROUTING_DEFAULTS_def:test]
8 8
 
9 9
 # foobar
10  
-RewriteCond %{REQUEST_URI} ^/foo(?:/([^/]+))?$
  10
+RewriteCond %{REQUEST_URI} ^/foo(?:/([^/]++))?$
11 11
 RewriteRule .* app.php [QSA,L,E=_ROUTING__route:foobar,E=_ROUTING_bar:%1,E=_ROUTING_DEFAULTS_bar:toto]
12 12
 
13 13
 # bar
14  
-RewriteCond %{REQUEST_URI} ^/bar/([^/]+)$
  14
+RewriteCond %{REQUEST_URI} ^/bar/([^/]++)$
15 15
 RewriteCond %{REQUEST_METHOD} !^(GET|HEAD)$ [NC]
16 16
 RewriteRule .* - [S=1,E=_ROUTING__allow_GET:1,E=_ROUTING__allow_HEAD:1]
17  
-RewriteCond %{REQUEST_URI} ^/bar/([^/]+)$
  17
+RewriteCond %{REQUEST_URI} ^/bar/([^/]++)$
18 18
 RewriteRule .* app.php [QSA,L,E=_ROUTING__route:bar,E=_ROUTING_foo:%1]
19 19
 
20 20
 # baragain
21  
-RewriteCond %{REQUEST_URI} ^/baragain/([^/]+)$
  21
+RewriteCond %{REQUEST_URI} ^/baragain/([^/]++)$
22 22
 RewriteCond %{REQUEST_METHOD} !^(GET|POST|HEAD)$ [NC]
23 23
 RewriteRule .* - [S=1,E=_ROUTING__allow_GET:1,E=_ROUTING__allow_POST:1,E=_ROUTING__allow_HEAD:1]
24  
-RewriteCond %{REQUEST_URI} ^/baragain/([^/]+)$
  24
+RewriteCond %{REQUEST_URI} ^/baragain/([^/]++)$
25 25
 RewriteRule .* app.php [QSA,L,E=_ROUTING__route:baragain,E=_ROUTING_foo:%1]
26 26
 
27 27
 # baz
@@ -39,25 +39,25 @@ RewriteCond %{REQUEST_URI} ^/test/baz3/$
39 39
 RewriteRule .* app.php [QSA,L,E=_ROUTING__route:baz3]
40 40
 
41 41
 # baz4
42  
-RewriteCond %{REQUEST_URI} ^/test/([^/]+)$
  42
+RewriteCond %{REQUEST_URI} ^/test/([^/]++)$
43 43
 RewriteRule .* $0/ [QSA,L,R=301]
44  
-RewriteCond %{REQUEST_URI} ^/test/([^/]+)/$
  44
+RewriteCond %{REQUEST_URI} ^/test/([^/]++)/$
45 45
 RewriteRule .* app.php [QSA,L,E=_ROUTING__route:baz4,E=_ROUTING_foo:%1]
46 46
 
47 47
 # baz5
48  
-RewriteCond %{REQUEST_URI} ^/test/([^/]+)/$
  48
+RewriteCond %{REQUEST_URI} ^/test/([^/]++)/$
49 49
 RewriteCond %{REQUEST_METHOD} !^(GET|HEAD)$ [NC]
50 50
 RewriteRule .* - [S=2,E=_ROUTING__allow_GET:1,E=_ROUTING__allow_HEAD:1]
51  
-RewriteCond %{REQUEST_URI} ^/test/([^/]+)$
  51
+RewriteCond %{REQUEST_URI} ^/test/([^/]++)$
52 52
 RewriteRule .* $0/ [QSA,L,R=301]
53  
-RewriteCond %{REQUEST_URI} ^/test/([^/]+)/$
  53
+RewriteCond %{REQUEST_URI} ^/test/([^/]++)/$
54 54
 RewriteRule .* app.php [QSA,L,E=_ROUTING__route:baz5,E=_ROUTING_foo:%1]
55 55
 
56 56
 # baz5unsafe
57  
-RewriteCond %{REQUEST_URI} ^/testunsafe/([^/]+)/$
  57
+RewriteCond %{REQUEST_URI} ^/testunsafe/([^/]++)/$
58 58
 RewriteCond %{REQUEST_METHOD} !^(POST)$ [NC]
59 59
 RewriteRule .* - [S=1,E=_ROUTING__allow_POST:1]
60  
-RewriteCond %{REQUEST_URI} ^/testunsafe/([^/]+)/$
  60
+RewriteCond %{REQUEST_URI} ^/testunsafe/([^/]++)/$
61 61
 RewriteRule .* app.php [QSA,L,E=_ROUTING__route:baz5unsafe,E=_ROUTING_foo:%1]
62 62
 
63 63
 # baz6
30  src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher1.php
@@ -31,7 +31,7 @@ public function match($pathinfo)
31 31
         }
32 32
 
33 33
         // bar
34  
-        if (0 === strpos($pathinfo, '/bar') && preg_match('#^/bar/(?<foo>[^/]+)$#s', $pathinfo, $matches)) {
  34
+        if (0 === strpos($pathinfo, '/bar') && preg_match('#^/bar/(?<foo>[^/]++)$#s', $pathinfo, $matches)) {
35 35
             if (!in_array($this->context->getMethod(), array('GET', 'HEAD'))) {
36 36
                 $allow = array_merge($allow, array('GET', 'HEAD'));
37 37
                 goto not_bar;
@@ -44,7 +44,7 @@ public function match($pathinfo)
44 44
         not_bar:
45 45
 
46 46
         // barhead
47  
-        if (0 === strpos($pathinfo, '/barhead') && preg_match('#^/barhead/(?<foo>[^/]+)$#s', $pathinfo, $matches)) {
  47
+        if (0 === strpos($pathinfo, '/barhead') && preg_match('#^/barhead/(?<foo>[^/]++)$#s', $pathinfo, $matches)) {
48 48
             if (!in_array($this->context->getMethod(), array('GET', 'HEAD'))) {
49 49
                 $allow = array_merge($allow, array('GET', 'HEAD'));
50 50
                 goto not_barhead;
@@ -72,14 +72,14 @@ public function match($pathinfo)
72 72
         }
73 73
 
74 74
         // baz4
75  
-        if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?<foo>[^/]+)/$#s', $pathinfo, $matches)) {
  75
+        if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?<foo>[^/]++)/$#s', $pathinfo, $matches)) {
76 76
             $matches['_route'] = 'baz4';
77 77
 
78 78
             return $matches;
79 79
         }
80 80
 
81 81
         // baz5
82  
-        if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?<foo>[^/]+)/$#s', $pathinfo, $matches)) {
  82
+        if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?<foo>[^/]++)/$#s', $pathinfo, $matches)) {
83 83
             if ($this->context->getMethod() != 'POST') {
84 84
                 $allow[] = 'POST';
85 85
                 goto not_baz5;
@@ -92,7 +92,7 @@ public function match($pathinfo)
92 92
         not_baz5:
93 93
 
94 94
         // baz.baz6
95  
-        if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?<foo>[^/]+)/$#s', $pathinfo, $matches)) {
  95
+        if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?<foo>[^/]++)/$#s', $pathinfo, $matches)) {
96 96
             if ($this->context->getMethod() != 'PUT') {
97 97
                 $allow[] = 'PUT';
98 98
                 goto not_bazbaz6;
@@ -124,14 +124,14 @@ public function match($pathinfo)
124 124
         if (0 === strpos($pathinfo, '/a')) {
125 125
             if (0 === strpos($pathinfo, '/a/b\'b')) {
126 126
                 // foo1
127  
-                if (preg_match('#^/a/b\'b/(?<foo>[^/]+)$#s', $pathinfo, $matches)) {
  127
+                if (preg_match('#^/a/b\'b/(?<foo>[^/]++)$#s', $pathinfo, $matches)) {
128 128
                     $matches['_route'] = 'foo1';
129 129
 
130 130
                     return $matches;
131 131
                 }
132 132
 
133 133
                 // bar1
134  
-                if (preg_match('#^/a/b\'b/(?<bar>[^/]+)$#s', $pathinfo, $matches)) {
  134
+                if (preg_match('#^/a/b\'b/(?<bar>[^/]++)$#s', $pathinfo, $matches)) {
135 135
                     $matches['_route'] = 'bar1';
136 136
 
137 137
                     return $matches;
@@ -148,14 +148,14 @@ public function match($pathinfo)
148 148
 
149 149
             if (0 === strpos($pathinfo, '/a/b\'b')) {
150 150
                 // foo2
151  
-                if (preg_match('#^/a/b\'b/(?<foo1>[^/]+)$#s', $pathinfo, $matches)) {
  151
+                if (preg_match('#^/a/b\'b/(?<foo1>[^/]++)$#s', $pathinfo, $matches)) {
152 152
                     $matches['_route'] = 'foo2';
153 153
 
154 154
                     return $matches;
155 155
                 }
156 156
 
157 157
                 // bar2
158  
-                if (preg_match('#^/a/b\'b/(?<bar1>[^/]+)$#s', $pathinfo, $matches)) {
  158
+                if (preg_match('#^/a/b\'b/(?<bar1>[^/]++)$#s', $pathinfo, $matches)) {
159 159
                     $matches['_route'] = 'bar2';
160 160
 
161 161
                     return $matches;
@@ -167,7 +167,7 @@ public function match($pathinfo)
167 167
 
168 168
         if (0 === strpos($pathinfo, '/multi')) {
169 169
             // helloWorld
170  
-            if (0 === strpos($pathinfo, '/multi/hello') && preg_match('#^/multi/hello(?:/(?<who>[^/]+))?$#s', $pathinfo, $matches)) {
  170
+            if (0 === strpos($pathinfo, '/multi/hello') && preg_match('#^/multi/hello(?:/(?<who>[^/]++))?$#s', $pathinfo, $matches)) {
171 171
                 return array_merge($this->mergeDefaults($matches, array (  'who' => 'World!',)), array('_route' => 'helloWorld'));
172 172
             }
173 173
 
@@ -184,14 +184,14 @@ public function match($pathinfo)
184 184
         }
185 185
 
186 186
         // foo3
187  
-        if (preg_match('#^/(?<_locale>[^/]+)/b/(?<foo>[^/]+)$#s', $pathinfo, $matches)) {
  187
+        if (preg_match('#^/(?<_locale>[^/]++)/b/(?<foo>[^/]++)$#s', $pathinfo, $matches)) {
188 188
             $matches['_route'] = 'foo3';
189 189
 
190 190
             return $matches;
191 191
         }
192 192
 
193 193
         // bar3
194  
-        if (preg_match('#^/(?<_locale>[^/]+)/b/(?<bar>[^/]+)$#s', $pathinfo, $matches)) {
  194
+        if (preg_match('#^/(?<_locale>[^/]++)/b/(?<bar>[^/]++)$#s', $pathinfo, $matches)) {
195 195
             $matches['_route'] = 'bar3';
196 196
 
197 197
             return $matches;
@@ -203,7 +203,7 @@ public function match($pathinfo)
203 203
         }
204 204
 
205 205
         // foo4
206  
-        if (0 === strpos($pathinfo, '/aba') && preg_match('#^/aba/(?<foo>[^/]+)$#s', $pathinfo, $matches)) {
  206
+        if (0 === strpos($pathinfo, '/aba') && preg_match('#^/aba/(?<foo>[^/]++)$#s', $pathinfo, $matches)) {
207 207
             $matches['_route'] = 'foo4';
208 208
 
209 209
             return $matches;
@@ -217,14 +217,14 @@ public function match($pathinfo)
217 217
 
218 218
             if (0 === strpos($pathinfo, '/a/b')) {
219 219
                 // b
220  
-                if (preg_match('#^/a/b/(?<var>[^/]+)$#s', $pathinfo, $matches)) {
  220
+                if (preg_match('#^/a/b/(?<var>[^/]++)$#s', $pathinfo, $matches)) {
221 221
                     $matches['_route'] = 'b';
222 222
 
223 223
                     return $matches;
224 224
                 }
225 225
 
226 226
                 // c
227  
-                if (0 === strpos($pathinfo, '/a/b/c') && preg_match('#^/a/b/c/(?<var>[^/]+)$#s', $pathinfo, $matches)) {
  227
+                if (0 === strpos($pathinfo, '/a/b/c') && preg_match('#^/a/b/c/(?<var>[^/]++)$#s', $pathinfo, $matches)) {
228 228
                     $matches['_route'] = 'c';
229 229
 
230 230
                     return $matches;
30  src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher2.php
@@ -31,7 +31,7 @@ public function match($pathinfo)
31 31
         }
32 32
 
33 33
         // bar
34  
-        if (0 === strpos($pathinfo, '/bar') && preg_match('#^/bar/(?<foo>[^/]+)$#s', $pathinfo, $matches)) {
  34
+        if (0 === strpos($pathinfo, '/bar') && preg_match('#^/bar/(?<foo>[^/]++)$#s', $pathinfo, $matches)) {
35 35
             if (!in_array($this->context->getMethod(), array('GET', 'HEAD'))) {
36 36
                 $allow = array_merge($allow, array('GET', 'HEAD'));
37 37
                 goto not_bar;
@@ -44,7 +44,7 @@ public function match($pathinfo)
44 44
         not_bar:
45 45
 
46 46
         // barhead
47  
-        if (0 === strpos($pathinfo, '/barhead') && preg_match('#^/barhead/(?<foo>[^/]+)$#s', $pathinfo, $matches)) {
  47
+        if (0 === strpos($pathinfo, '/barhead') && preg_match('#^/barhead/(?<foo>[^/]++)$#s', $pathinfo, $matches)) {
48 48
             if (!in_array($this->context->getMethod(), array('GET', 'HEAD'))) {
49 49
                 $allow = array_merge($allow, array('GET', 'HEAD'));
50 50
                 goto not_barhead;
@@ -76,7 +76,7 @@ public function match($pathinfo)
76 76
         }
77 77
 
78 78
         // baz4
79  
-        if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?<foo>[^/]+)/?$#s', $pathinfo, $matches)) {
  79
+        if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?<foo>[^/]++)/?$#s', $pathinfo, $matches)) {
80 80
             if (substr($pathinfo, -1) !== '/') {
81 81
                 return $this->redirect($pathinfo.'/', 'baz4');
82 82
             }
@@ -87,7 +87,7 @@ public function match($pathinfo)
87 87
         }
88 88
 
89 89
         // baz5
90  
-        if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?<foo>[^/]+)/$#s', $pathinfo, $matches)) {
  90
+        if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?<foo>[^/]++)/$#s', $pathinfo, $matches)) {
91 91
             if ($this->context->getMethod() != 'POST') {
92 92
                 $allow[] = 'POST';
93 93
                 goto not_baz5;
@@ -100,7 +100,7 @@ public function match($pathinfo)
100 100
         not_baz5:
101 101
 
102 102
         // baz.baz6
103  
-        if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?<foo>[^/]+)/$#s', $pathinfo, $matches)) {
  103
+        if (0 === strpos($pathinfo, '/test') && preg_match('#^/test/(?<foo>[^/]++)/$#s', $pathinfo, $matches)) {
104 104
             if ($this->context->getMethod() != 'PUT') {
105 105
                 $allow[] = 'PUT';
106 106
                 goto not_bazbaz6;
@@ -132,14 +132,14 @@ public function match($pathinfo)
132 132
         if (0 === strpos($pathinfo, '/a')) {
133 133
             if (0 === strpos($pathinfo, '/a/b\'b')) {
134 134
                 // foo1
135  
-                if (preg_match('#^/a/b\'b/(?<foo>[^/]+)$#s', $pathinfo, $matches)) {
  135
+                if (preg_match('#^/a/b\'b/(?<foo>[^/]++)$#s', $pathinfo, $matches)) {
136 136
                     $matches['_route'] = 'foo1';
137 137
 
138 138
                     return $matches;
139 139
                 }
140 140
 
141 141
                 // bar1
142  
-                if (preg_match('#^/a/b\'b/(?<bar>[^/]+)$#s', $pathinfo, $matches)) {
  142
+                if (preg_match('#^/a/b\'b/(?<bar>[^/]++)$#s', $pathinfo, $matches)) {
143 143
                     $matches['_route'] = 'bar1';
144 144
 
145 145
                     return $matches;
@@ -156,14 +156,14 @@ public function match($pathinfo)
156 156
 
157 157
             if (0 === strpos($pathinfo, '/a/b\'b')) {
158 158
                 // foo2
159  
-                if (preg_match('#^/a/b\'b/(?<foo1>[^/]+)$#s', $pathinfo, $matches)) {
  159
+                if (preg_match('#^/a/b\'b/(?<foo1>[^/]++)$#s', $pathinfo, $matches)) {
160 160
                     $matches['_route'] = 'foo2';
161 161
 
162 162
                     return $matches;
163 163
                 }
164 164
 
165 165
                 // bar2
166  
-                if (preg_match('#^/a/b\'b/(?<bar1>[^/]+)$#s', $pathinfo, $matches)) {
  166
+                if (preg_match('#^/a/b\'b/(?<bar1>[^/]++)$#s', $pathinfo, $matches)) {
167 167
                     $matches['_route'] = 'bar2';
168 168
 
169 169
                     return $matches;
@@ -175,7 +175,7 @@ public function match($pathinfo)
175 175
 
176 176
         if (0 === strpos($pathinfo, '/multi')) {
177 177
             // helloWorld
178  
-            if (0 === strpos($pathinfo, '/multi/hello') && preg_match('#^/multi/hello(?:/(?<who>[^/]+))?$#s', $pathinfo, $matches)) {
  178
+            if (0 === strpos($pathinfo, '/multi/hello') && preg_match('#^/multi/hello(?:/(?<who>[^/]++))?$#s', $pathinfo, $matches)) {
179 179
                 return array_merge($this->mergeDefaults($matches, array (  'who' => 'World!',)), array('_route' => 'helloWorld'));
180 180
             }
181 181
 
@@ -196,14 +196,14 @@ public function match($pathinfo)
196 196
         }
197 197
 
198 198
         // foo3
199  
-        if (preg_match('#^/(?<_locale>[^/]+)/b/(?<foo>[^/]+)$#s', $pathinfo, $matches)) {
  199
+        if (preg_match('#^/(?<_locale>[^/]++)/b/(?<foo>[^/]++)$#s', $pathinfo, $matches)) {
200 200
             $matches['_route'] = 'foo3';
201 201
 
202 202
             return $matches;
203 203
         }
204 204
 
205 205
         // bar3
206  
-        if (preg_match('#^/(?<_locale>[^/]+)/b/(?<bar>[^/]+)$#s', $pathinfo, $matches)) {
  206
+        if (preg_match('#^/(?<_locale>[^/]++)/b/(?<bar>[^/]++)$#s', $pathinfo, $matches)) {
207 207
             $matches['_route'] = 'bar3';
208 208
 
209 209
             return $matches;
@@ -215,7 +215,7 @@ public function match($pathinfo)
215 215
         }
216 216
 
217 217
         // foo4
218  
-        if (0 === strpos($pathinfo, '/aba') && preg_match('#^/aba/(?<foo>[^/]+)$#s', $pathinfo, $matches)) {
  218
+        if (0 === strpos($pathinfo, '/aba') && preg_match('#^/aba/(?<foo>[^/]++)$#s', $pathinfo, $matches)) {
219 219
             $matches['_route'] = 'foo4';
220 220
 
221 221
             return $matches;
@@ -229,14 +229,14 @@ public function match($pathinfo)
229 229
 
230 230
             if (0 === strpos($pathinfo, '/a/b')) {
231 231
                 // b
232  
-                if (preg_match('#^/a/b/(?<var>[^/]+)$#s', $pathinfo, $matches)) {
  232
+                if (preg_match('#^/a/b/(?<var>[^/]++)$#s', $pathinfo, $matches)) {
233 233
                     $matches['_route'] = 'b';
234 234
 
235 235
                     return $matches;
236 236
                 }
237 237
 
238 238
                 // c
239  
-                if (0 === strpos($pathinfo, '/a/b/c') && preg_match('#^/a/b/c/(?<var>[^/]+)$#s', $pathinfo, $matches)) {
  239
+                if (0 === strpos($pathinfo, '/a/b/c') && preg_match('#^/a/b/c/(?<var>[^/]++)$#s', $pathinfo, $matches)) {
240 240
                     $matches['_route'] = 'c';
241 241
 
242 242
                     return $matches;
2  src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher3.php
@@ -32,7 +32,7 @@ public function match($pathinfo)
32 32
             }
33 33
 
34 34
             // dynamic
35  
-            if (preg_match('#^/rootprefix/(?<var>[^/]+)$#s', $pathinfo, $matches)) {
  35
+            if (preg_match('#^/rootprefix/(?<var>[^/]++)$#s', $pathinfo, $matches)) {
36 36
                 $matches['_route'] = 'dynamic';
37 37
 
38 38
                 return $matches;
52  src/Symfony/Component/Routing/Tests/RouteCompilerTest.php
@@ -43,51 +43,51 @@ public function provideCompileData()
43 43
             array(
44 44
                 'Route with a variable',
45 45
                 array('/foo/{bar}'),
46  
-                '/foo', '#^/foo/(?<bar>[^/]+)$#s', array('bar'), array(
47  
-                    array('variable', '/', '[^/]+', 'bar'),
  46
+                '/foo', '#^/foo/(?<bar>[^/]++)$#s', array('bar'), array(
  47
+                    array('variable', '/', '[^/]++', 'bar'),
48 48
                     array('text', '/foo'),
49 49
                 )),
50 50
 
51 51
             array(
52 52
                 'Route with a variable that has a default value',
53 53
                 array('/foo/{bar}', array('bar' => 'bar')),
54  
-                '/foo', '#^/foo(?:/(?<bar>[^/]+))?$#s', array('bar'), array(
55  
-                    array('variable', '/', '[^/]+', 'bar'),
  54
+                '/foo', '#^/foo(?:/(?<bar>[^/]++))?$#s', array('bar'), array(
  55
+                    array('variable', '/', '[^/]++', 'bar'),
56 56
                     array('text', '/foo'),
57 57
                 )),
58 58
 
59 59
             array(
60 60
                 'Route with several variables',
61 61
                 array('/foo/{bar}/{foobar}'),
62  
-                '/foo', '#^/foo/(?<bar>[^/]+)/(?<foobar>[^/]+)$#s', array('bar', 'foobar'), array(
63  
-                    array('variable', '/', '[^/]+', 'foobar'),
64  
-                    array('variable', '/', '[^/]+', 'bar'),
  62
+                '/foo', '#^/foo/(?<bar>[^/]++)/(?<foobar>[^/]++)$#s', array('bar', 'foobar'), array(
  63
+                    array('variable', '/', '[^/]++', 'foobar'),
  64
+                    array('variable', '/', '[^/]++', 'bar'),
65 65
                     array('text', '/foo'),
66 66
                 )),
67 67
 
68 68
             array(
69 69
                 'Route with several variables that have default values',
70 70
                 array('/foo/{bar}/{foobar}', array('bar' => 'bar', 'foobar' => '')),
71  
-                '/foo', '#^/foo(?:/(?<bar>[^/]+)(?:/(?<foobar>[^/]+))?)?$#s', array('bar', 'foobar'), array(
72  
-                    array('variable', '/', '[^/]+', 'foobar'),
73  
-                    array('variable', '/', '[^/]+', 'bar'),
  71
+                '/foo', '#^/foo(?:/(?<bar>[^/]++)(?:/(?<foobar>[^/]++))?)?$#s', array('bar', 'foobar'), array(
  72
+                    array('variable', '/', '[^/]++', 'foobar'),
  73
+                    array('variable', '/', '[^/]++', 'bar'),
74 74
                     array('text', '/foo'),
75 75
                 )),
76 76
 
77 77
             array(
78 78
                 'Route with several variables but some of them have no default values',
79 79
                 array('/foo/{bar}/{foobar}', array('bar' => 'bar')),
80  
-                '/foo', '#^/foo/(?<bar>[^/]+)/(?<foobar>[^/]+)$#s', array('bar', 'foobar'), array(
81  
-                    array('variable', '/', '[^/]+', 'foobar'),
82  
-                    array('variable', '/', '[^/]+', 'bar'),
  80
+                '/foo', '#^/foo/(?<bar>[^/]++)/(?<foobar>[^/]++)$#s', array('bar', 'foobar'), array(
  81
+                    array('variable', '/', '[^/]++', 'foobar'),
  82
+                    array('variable', '/', '[^/]++', 'bar'),
83 83
                     array('text', '/foo'),
84 84
                 )),
85 85
 
86 86
             array(
87 87
                 'Route with an optional variable as the first segment',
88 88
                 array('/{bar}', array('bar' => 'bar')),
89  
-                '', '#^/(?<bar>[^/]+)?$#s', array('bar'), array(
90  
-                    array('variable', '/', '[^/]+', 'bar'),
  89
+                '', '#^/(?<bar>[^/]++)?$#s', array('bar'), array(
  90
+                    array('variable', '/', '[^/]++', 'bar'),
91 91
                 )),
92 92
 
93 93
             array(
@@ -107,16 +107,16 @@ public function provideCompileData()
107 107
             array(
108 108
                 'Route with only optional variables',
109 109
                 array('/{foo}/{bar}', array('foo' => 'foo', 'bar' => 'bar')),
110  
-                '', '#^/(?<foo>[^/]+)?(?:/(?<bar>[^/]+))?$#s', array('foo', 'bar'), array(
111  
-                    array('variable', '/', '[^/]+', 'bar'),
112  
-                    array('variable', '/', '[^/]+', 'foo'),
  110
+                '', '#^/(?<foo>[^/]++)?(?:/(?<bar>[^/]++))?$#s', array('foo', 'bar'), array(
  111
+                    array('variable', '/', '[^/]++', 'bar'),
  112
+                    array('variable', '/', '[^/]++', 'foo'),
113 113
                 )),
114 114
 
115 115
             array(
116 116
                 'Route with a variable in last position',
117 117
                 array('/foo-{bar}'),
118  
-                '/foo', '#^/foo\-(?<bar>[^/]+)$#s', array('bar'), array(
119  
-                array('variable', '-', '[^/]+', 'bar'),
  118
+                '/foo', '#^/foo\-(?<bar>[^/]++)$#s', array('bar'), array(
  119
+                array('variable', '-', '[^/]++', 'bar'),
120 120
                 array('text', '/foo'),
121 121
             )),
122 122
 
@@ -132,9 +132,9 @@ public function provideCompileData()
132 132
             array(
133 133
                 'Route without separator between variables',
134 134
                 array('/{w}{x}{y}{z}.{_format}', array('z' => 'default-z', '_format' => 'html'), array('y' => '(y|Y)')),
135  
-                '', '#^/(?<w>[^/\.]+)(?<x>[^/\.]+)(?<y>(y|Y))(?:(?<z>[^/\.]+)(?:\.(?<_format>[^/]+))?)?$#s', array('w', 'x', 'y', 'z', '_format'), array(
136  
-                array('variable', '.', '[^/]+', '_format'),
137  
-                array('variable', '', '[^/\.]+', 'z'),
  135
+                '', '#^/(?<w>[^/\.]+)(?<x>[^/\.]+)(?<y>(y|Y))(?:(?<z>[^/\.]++)(?:\.(?<_format>[^/]++))?)?$#s', array('w', 'x', 'y', 'z', '_format'), array(
  136
+                array('variable', '.', '[^/]++', '_format'),
  137
+                array('variable', '', '[^/\.]++', 'z'),
138 138
                 array('variable', '', '(y|Y)', 'y'),
139 139
                 array('variable', '', '[^/\.]+', 'x'),
140 140
                 array('variable', '/', '[^/\.]+', 'w'),
@@ -143,9 +143,9 @@ public function provideCompileData()
143 143
             array(
144 144
                 'Route with a format',
145 145
                 array('/foo/{bar}.{_format}'),
146  
-                '/foo', '#^/foo/(?<bar>[^/\.]+)\.(?<_format>[^/]+)$#s', array('bar', '_format'), array(
147  
-                array('variable', '.', '[^/]+', '_format'),
148  
-                array('variable', '/', '[^/\.]+', 'bar'),
  146
+                '/foo', '#^/foo/(?<bar>[^/\.]++)\.(?<_format>[^/]++)$#s', array('bar', '_format'), array(
  147
+                array('variable', '.', '[^/]++', '_format'),
  148
+                array('variable', '/', '[^/\.]++', 'bar'),
149 149
                 array('text', '/foo'),
150 150
             )),
151 151
         );

0 notes on commit 4eee88f

Markus Staab

dont know how hot this path is, but maybe doing the comparison the other way arround, would trigger the most expensive operation, the preg_match(), just when really required...

if ('' === $followingPattern || '' !== $nextSeparator && !preg_match('#^\{\w+\}#', $followingPattern)) {

I also stripped one pair of parentesis, which were obsolete

Tobias Schultze

Reversing the order has no effect here at all because preg_match cannot be called when following pattern is empty anyway.

Markus Staab

Hmm the origin code executes the regex-function but the quoted code in my previous comment does not. So it saves the preg-match call, while the origin code everytime executes the preg-match, no matter if $followingPattern is empty or not,...

Tobias Schultze

No. Please think about it. In my code:
'' !== $nextSeparator -> preg_match
'' === $nextSeparator -> no preg_match
In your code when '' === $followingPattern it means '' === $nextSeparator is also true. So you safe nothing.

Please sign in to comment.
Something went wrong with that request. Please try again.