improved router performance a bit

mojolicious · Jul 31, 2014 · 53b3831 · 53b3831
1 parent 58ef4a1
commit 53b3831
Show file tree

Hide file tree

Showing 7 changed files with 45 additions and 31 deletions.
diff --git a/Changes b/Changes
@@ -1,5 +1,8 @@
 
 5.23  2014-07-31
+  - Improved router performance.
+  - Improved routes command to show format regular expression separately.
+  - Fixed partial route bug in Mojolicious::Routes::Match.
 
 5.22  2014-07-30
   - Added SOCKS5 support to Mojo::UserAgent.

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

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

diff --git a/lib/Mojolicious/Routes/Match.pm b/lib/Mojolicious/Routes/Match.pm
@@ -39,11 +39,13 @@ sub _match {
   my ($self, $r, $c, $options) = @_;
 
   # Pattern
-  my $path = $options->{path};
-  return
-    unless my $captures = $r->pattern->match_partial(\$path, $r->is_endpoint);
+  my $path    = $options->{path};
+  my $partial = $r->partial;
+  my $detect  = (my $endpoint = $r->is_endpoint) && !$partial;
+  return unless my $captures = $r->pattern->match_partial(\$path, $detect);
   local $options->{path} = $path;
-  $captures = $self->{captures} = {%{$self->{captures} || {}}, %$captures};
+  @{$self->{captures} ||= {}}{keys %$captures} = values %$captures;
+  $captures = $self->{captures};
 
   # Method
   my $methods = $r->via;
@@ -63,14 +65,13 @@ sub _match {
 
   # Partial
   my $empty = !length $path || $path eq '/';
-  if ($r->partial) {
+  if ($partial) {
     $captures->{path} = $path;
     $self->endpoint($r);
     $empty = 1;
   }
 
   # Endpoint (or bridge)
-  my $endpoint = $r->is_endpoint;
   if (($endpoint && $empty) || $r->inline) {
     push @{$self->stack}, {%$captures};
     if ($endpoint && $empty) {

diff --git a/lib/Mojolicious/Routes/Pattern.pm b/lib/Mojolicious/Routes/Pattern.pm
@@ -20,12 +20,11 @@ sub match_partial {
   my ($self, $pathref, $detect) = @_;
 
   # Compile on demand
-  my $regex = $self->regex || $self->_compile;
-  my $format
-    = $detect ? ($self->format_regex || $self->_compile_format) : undef;
+  $self->_compile unless $self->{regex};
+  $self->_compile_format if $detect && !$self->{format_regex};
 
   # Match
-  return undef unless my @captures = $$pathref =~ $regex;
+  return undef unless my @captures = $$pathref =~ $self->regex;
   $$pathref = ${^POSTMATCH};
 
   # Merge captures
@@ -37,11 +36,10 @@ sub match_partial {
   }
 
   # Format
-  my $constraint = $self->constraints->{format};
-  return $captures if !$detect || defined $constraint && !$constraint;
-  if ($$pathref =~ s!^/?$format!!) { $captures->{format} = $1 }
-  elsif ($constraint) { return undef unless $captures->{format} }
-
+  return $captures unless $detect && (my $regex = $self->format_regex);
+  return undef unless $$pathref =~ $regex;
+  $captures->{format} = $1 if defined $1;
+  $$pathref = '';
   return $captures;
 }
 
@@ -146,23 +144,23 @@ sub _compile {
   # Not rooted with a slash
   $regex = "$block$regex" if $block;
 
-  return $self->regex(qr/^$regex/ps)->regex;
+  $self->regex(qr/^$regex/ps);
 }
 
 sub _compile_format {
   my $self = shift;
 
   # Default regex
-  my $c = $self->constraints;
-  return $self->format_regex(qr!\.([^/]+)$!)->format_regex
-    unless defined $c->{format};
+  my $format = $self->constraints->{format};
+  return $self->format_regex(qr!^/?(?:\.([^/]+))?$!) unless defined $format;
 
   # No regex
-  return undef unless $c->{format};
+  return undef unless $format;
 
   # Compile custom regex
-  my $regex = _compile_req($c->{format});
-  return $self->format_regex(qr!\.$regex$!)->format_regex;
+  my $regex = '/?\.' . _compile_req($format);
+  $regex = "(?:$regex)?" if $self->defaults->{format};
+  $self->format_regex(qr!^$regex$!);
 }
 
 sub _compile_req {

diff --git a/lib/Mojolicious/Routes/Route.pm b/lib/Mojolicious/Routes/Route.pm
@@ -167,7 +167,7 @@ sub to {
     }
   }
 
-  $pattern->defaults({%{$pattern->defaults}, %defaults});
+  @{$pattern->defaults}{keys %defaults} = values %defaults;
 
   return $self;
 }

diff --git a/t/mojolicious/routes.t b/t/mojolicious/routes.t
@@ -194,6 +194,9 @@ $r->route('/missing/*/name')->to('missing#wildcard');
 $r->route('/missing/too/*', '' => ['test'])
   ->to('missing#too', '' => 'missing');
 
+# /partial/*
+$r->route('/partial')->detour('foo#bar');
+
 # Cached lookup
 my $fast = $r->route('/fast');
 is $r->find('fast'),   $fast, 'fast route found';
@@ -848,4 +851,16 @@ is_deeply $m->stack,
   'right structure';
 is $m->path_for->{path}, '/missing/too', 'right path';
 
+# Partial route
+$m = Mojolicious::Routes::Match->new(root => $r);
+$m->match($c => {method => 'GET', path => '/partial/test'});
+is_deeply $m->stack,
+  [{controller => 'foo', action => 'bar', 'path' => '/test'}],
+  'right structure';
+$m = Mojolicious::Routes::Match->new(root => $r);
+$m->match($c => {method => 'GET', path => '/partial.test'});
+is_deeply $m->stack,
+  [{controller => 'foo', action => 'bar', 'path' => '.test'}],
+  'right structure';
+
 done_testing();