improve router performance

mojolicious · Nov 18, 2015 · c9b6598 · c9b6598
1 parent 86597e3
commit c9b6598
Show file tree

Hide file tree

Showing 7 changed files with 36 additions and 46 deletions.
diff --git a/Changes b/Changes
@@ -1,6 +1,8 @@
 
 6.32  2015-11-18
+  - Deprecated Mojolicious::Routes::Pattern::format_regex.
   - Added support for new HTTP status code.
+  - Improved router performance.
   - Improved Mojo::DOM::CSS performance slightly. (jamadam)
   - Fixed a few case-sensitivity and An+B notation issues in Mojo::DOM::CSS.
     (jamadam)

diff --git a/lib/Mojolicious/Command/routes.pm b/lib/Mojolicious/Command/routes.pm
@@ -45,9 +45,7 @@ sub _walk {
   # Regex (verbose)
   my $pattern = $route->pattern;
   $pattern->match('/', $route->is_endpoint && !$partial);
-  my $regex  = (regexp_pattern $pattern->regex)[0];
-  my $format = (regexp_pattern($pattern->format_regex))[0];
-  push @$row, $regex, $format ? $format : '' if $verbose;
+  push @$row, (regexp_pattern $pattern->regex)[0] if $verbose;
 
   $depth++;
   _walk($_, $depth, $rows, $verbose) for @{$route->children};

diff --git a/lib/Mojolicious/Guides/Routing.pod b/lib/Mojolicious/Guides/Routing.pod
@@ -787,10 +787,10 @@ to list all available routes together with name and underlying regular
 expressions.
 
   $ ./myapp.pl routes -v
-  /foo/:name  ....  POST  fooname  ^/foo/([^/\.]+)  ^/?(?:\.([^/]+))?$
-  /bar        ..U.  *     bar      ^/bar
-    +/baz     ...W  GET   baz      ^/baz            ^/?(?:\.([^/]+))?$
-  /yada       ....  *     yada     ^/yada           ^/?(?:\.([^/]+))?$
+  /foo/:name  ....  POST  fooname  (^\/foo/([^/.]+)/?(?:\.([^/]+))?$)
+  /bar        ..U.  *     bar      (^\/bar)
+    +/baz     ...W  GET   baz      (^\/baz/?(?:\.([^/]+))?$)
+  /yada       ....  *     yada     (^\/yada/?(?:\.([^/]+))?$)
 
 =head1 ADVANCED
 

diff --git a/lib/Mojolicious/Routes/Pattern.pm b/lib/Mojolicious/Routes/Pattern.pm
@@ -1,15 +1,23 @@
 package Mojolicious::Routes::Pattern;
 use Mojo::Base -base;
 
+use Mojo::Util 'deprecated';
+
 has [qw(constraints defaults)] => sub { {} };
-has [qw(format_regex regex unparsed)];
+has [qw(regex unparsed)];
 has placeholder_start => ':';
 has [qw(placeholders tree)] => sub { [] };
 has quote_end      => ')';
 has quote_start    => '(';
 has relaxed_start  => '#';
 has wildcard_start => '*';
 
+# DEPRECATED in Clinking Beer Mugs!
+sub format_regex {
+  deprecated 'Mojolicious::Routes::Pattern::format_regex is DEPRECATED';
+  return @_ > 1 ? $_[0] : undef;
+}
+
 sub match {
   my ($self, $path, $detect) = @_;
   my $captures = $self->match_partial(\$path, $detect);
@@ -20,24 +28,19 @@ sub match_partial {
   my ($self, $pathref, $detect) = @_;
 
   # Compile on demand
-  $self->_compile unless $self->{regex};
-  $self->_compile_format if $detect && !$self->{format_regex};
+  $self->_compile($detect) unless $self->{regex};
 
   # Path
   return undef unless my @captures = $$pathref =~ $self->regex;
   $$pathref = ${^POSTMATCH};
+  shift @captures;
   my $captures = {%{$self->defaults}};
-  for my $placeholder (@{$self->placeholders}) {
+  for my $placeholder (@{$self->placeholders}, 'format') {
     last unless @captures;
     my $capture = shift @captures;
     $captures->{$placeholder} = $capture if defined $capture;
   }
 
-  # Format
-  return $captures unless $detect && (my $regex = $self->format_regex);
-  return undef unless $$pathref =~ $regex;
-  $captures->{format} = $1 if defined $1;
-  $$pathref = '';
   return $captures;
 }
 
@@ -87,7 +90,7 @@ sub render {
 }
 
 sub _compile {
-  my $self = shift;
+  my ($self, $detect) = @_;
 
   my $placeholders = $self->placeholders;
   my $constraints  = $self->constraints;
@@ -135,23 +138,25 @@ sub _compile {
   # Not rooted with a slash
   $regex = "$block$regex" if $block;
 
-  $self->regex(qr/^$regex/ps);
+  # Format
+  $regex .= _compile_format($constraints->{format}, $defaults->{format})
+    if $detect;
+
+  $self->regex(qr/(^$regex)/ps);
 }
 
 sub _compile_format {
-  my $self = shift;
+  my ($format, $default) = @_;
 
   # Default regex
-  my $format = $self->constraints->{format};
-  return $self->format_regex(qr!^/?(?:\.([^/]+))?$!) unless defined $format;
+  return '/?(?:\.([^/]+))?$' unless defined $format;
 
   # No regex
-  return undef unless $format;
+  return '' unless $format;
 
   # Compile custom regex
   my $regex = '\.' . _compile_req($format);
-  $regex = "(?:$regex)?" if $self->defaults->{format};
-  $self->format_regex(qr!^/?$regex$!);
+  return $default ? "/?(?:$regex)?\$" : "/?$regex\$";
 }
 
 sub _compile_req {
@@ -257,13 +262,6 @@ Regular expression constraints.
 
 Default parameters.
 
-=head2 format_regex
-
-  my $regex = $pattern->format_regex;
-  $pattern  = $pattern->format_regex($regex);
-
-Compiled regular expression for format matching.
-
 =head2 placeholder_start
 
   my $start = $pattern->placeholder_start;

diff --git a/t/mojolicious/pattern.t b/t/mojolicious/pattern.t
@@ -148,19 +148,15 @@ is $pattern->render({test => $value}), "/$value", 'right result';
 # Format detection
 $pattern = Mojolicious::Routes::Pattern->new('/test');
 $pattern->defaults({action => 'index'});
-ok !$pattern->regex,        'no regex';
-ok !$pattern->format_regex, 'no format regex';
+ok !$pattern->regex, 'no regex';
 is_deeply $pattern->match('/test.xml', 1),
   {action => 'index', format => 'xml'}, 'right structure';
-ok $pattern->regex,        'regex has been compiled on demand';
-ok $pattern->format_regex, 'format regex has been compiled on demand';
+ok $pattern->regex, 'regex has been compiled on demand';
 $pattern = Mojolicious::Routes::Pattern->new('/test.json');
 $pattern->defaults({action => 'index'});
-ok !$pattern->regex,        'no regex';
-ok !$pattern->format_regex, 'no format regex';
+ok !$pattern->regex, 'no regex';
 is_deeply $pattern->match('/test.json'), {action => 'index'}, 'right structure';
 ok $pattern->regex, 'regex has been compiled on demand';
-ok !$pattern->format_regex, 'no format regex';
 is_deeply $pattern->match('/test.json', 1), {action => 'index'},
   'right structure';
 ok !$pattern->match('/test.xml'), 'no result';
@@ -169,21 +165,17 @@ ok !$pattern->match('/test'),     'no result';
 # Formats without detection
 $pattern = Mojolicious::Routes::Pattern->new('/test');
 $pattern->defaults({action => 'index'});
-ok !$pattern->regex,        'no regex';
-ok !$pattern->format_regex, 'no format regex';
+ok !$pattern->regex, 'no regex';
 ok !$pattern->match('/test.xml'), 'no result';
 ok $pattern->regex, 'regex has been compiled on demand';
-ok !$pattern->format_regex, 'no format regex';
 is_deeply $pattern->match('/test'), {action => 'index'}, 'right structure';
 
 # Format detection disabled
 $pattern = Mojolicious::Routes::Pattern->new('/test', format => 0);
 $pattern->defaults({action => 'index'});
-ok !$pattern->regex,        'no regex';
-ok !$pattern->format_regex, 'no format regex';
+ok !$pattern->regex, 'no regex';
 is_deeply $pattern->match('/test', 1), {action => 'index'}, 'right structure';
 ok $pattern->regex, 'regex has been compiled on demand';
-ok !$pattern->format_regex, 'no format regex';
 ok !$pattern->match('/test.xml', 1), 'no result';
 
 # Special pattern for disabling format detection

diff --git a/t/mojolicious/routes.t b/t/mojolicious/routes.t
@@ -75,7 +75,7 @@ $test2->route('/baz')->to('just#works');
 $r->route('/')->to(controller => 'hello', action => 'world');
 
 # /wildcards/1/*
-$r->route('/wildcards/1/(*wildcard)', wildcard => qr/(.*)/)
+$r->route('/wildcards/1/(*wildcard)', wildcard => qr/(?:.*)/)
   ->to(controller => 'wild', action => 'card');
 
 # /wildcards/2/*

diff --git a/t/pod_coverage.t b/t/pod_coverage.t
@@ -7,4 +7,4 @@ plan skip_all => 'set TEST_POD to enable this test (developer only!)'
 plan skip_all => 'Test::Pod::Coverage 1.04+ required for this test!'
   unless eval 'use Test::Pod::Coverage 1.04; 1';
 
-all_pod_coverage_ok({also_private => ['collecting']});
+all_pod_coverage_ok({also_private => [qw(collecting format_regex)]});