improved performance of nested helpers and helpers in templates signi…

…ficantly
mojolicious · Aug 12, 2014 · d32baef · d32baef
1 parent 5271482
commit d32baef
Show file tree

Hide file tree

Showing 5 changed files with 48 additions and 22 deletions.
diff --git a/Changes b/Changes
@@ -1,5 +1,7 @@
 
 5.28  2014-08-12
+  - Improved performance of nested helpers and helpers in templates by
+    significantly.
   - Improved Mojo::JSON to generate smaller JSON by not escaping the "/"
     character.
 

diff --git a/lib/Mojo/Util.pm b/lib/Mojo/Util.pm
@@ -11,6 +11,7 @@ use File::Basename 'dirname';
 use File::Spec::Functions 'catfile';
 use List::Util 'min';
 use MIME::Base64 qw(decode_base64 encode_base64);
+use Symbol 'delete_package';
 use Time::HiRes ();
 
 # Check for monotonic clock support
@@ -418,6 +419,15 @@ sub _stash {
   return $object;
 }
 
+sub _teardown {
+  return unless my $class = shift;
+
+  # @ISA has to be cleared first because of circular references
+  no strict 'refs';
+  @{"${class}::ISA"} = ();
+  delete_package $class;
+}
+
 1;
 
 =encoding utf8

diff --git a/lib/Mojolicious/Plugin/EPRenderer.pm b/lib/Mojolicious/Plugin/EPRenderer.pm
@@ -4,11 +4,15 @@ use Mojo::Base 'Mojolicious::Plugin';
 use Mojo::Template;
 use Mojo::Util qw(encode md5_sum monkey_patch);
 
+sub DESTROY { Mojo::Util::_teardown(shift->{namespace}) }
+
 sub register {
   my ($self, $app, $conf) = @_;
 
   # Auto escape by default to prevent XSS attacks
   my $template = {auto_escape => 1, %{$conf->{template} || {}}};
+  $self->{namespace} = $template->{namespace}
+    //= 'Mojo::Template::Sandbox::' . md5_sum("$self");
 
   # Add "ep" handler and make it the default
   $app->renderer->default_handler('ep')->add_handler(
@@ -53,8 +57,8 @@ sub register {
 sub _helpers {
   my ($ns, $helpers) = @_;
   for my $name (grep {/^\w+$/} keys %$helpers) {
-    monkey_patch $ns, $name,
-      sub { $ns->_C->app->renderer->helpers->{$name}->($ns->_C, @_) };
+    my $sub = $helpers->{$name};
+    monkey_patch $ns, $name, sub { $ns->_C->$sub(@_) };
   }
 }
 

diff --git a/lib/Mojolicious/Renderer.pm b/lib/Mojolicious/Renderer.pm
@@ -1,13 +1,12 @@
 package Mojolicious::Renderer;
 use Mojo::Base -base;
 
-use Carp ();
 use File::Spec::Functions 'catfile';
 use Mojo::Cache;
 use Mojo::JSON 'encode_json';
 use Mojo::Home;
 use Mojo::Loader;
-use Mojo::Util qw(decamelize encode slurp);
+use Mojo::Util qw(decamelize encode md5_sum monkey_patch slurp);
 
 has cache   => sub { Mojo::Cache->new };
 has classes => sub { ['main'] };
@@ -30,6 +29,8 @@ $HOME->parse(
   $HOME->parse($HOME->mojo_lib_dir)->rel_dir('Mojolicious/templates'));
 my %TEMPLATES = map { $_ => slurp $HOME->rel_file($_) } @{$HOME->list_files};
 
+sub DESTROY { Mojo::Util::_teardown($_) for @{shift->{namespaces}} }
+
 sub accepts {
   my ($self, $c) = (shift, shift);
 
@@ -70,10 +71,19 @@ sub get_data_template {
 
 sub get_helper {
   my ($self, $name) = @_;
+
   if (my $h = $self->helpers->{$name} || $self->{proxy}{$name}) { return $h }
-  return undef unless grep {/^\Q$name\E\./} keys %{$self->helpers};
-  return $self->{proxy}{$name}
-    = sub { bless [shift, $name], 'Mojolicious::Renderer::_Proxy' };
+
+  my $found;
+  my $class = 'Mojolicious::Renderer::Helpers::' . md5_sum("$name:$self");
+  for my $key (keys %{$self->helpers}) {
+    $key =~ /^(\Q$name\E\.([^.]+))/ ? ($found, my $method) = (1, $2) : next;
+    my $sub = $self->get_helper($1);
+    monkey_patch $class, $method => sub { ${shift()}->$sub(@_) };
+  }
+
+  $found ? push @{$self->{namespaces}}, $class : return undef;
+  return $self->{proxy}{$name} = sub { bless \shift, $class };
 }
 
 sub render {
@@ -203,6 +213,7 @@ sub template_path {
 sub _add {
   my ($self, $attr, $name, $cb) = @_;
   $self->$attr->{$name} = $cb;
+  delete $self->{proxy};
   return $self;
 }
 
@@ -248,21 +259,6 @@ sub _render_template {
   return undef;
 }
 
-package Mojolicious::Renderer::_Proxy;
-use Mojo::Base -strict;
-
-sub AUTOLOAD {
-  my $self = shift;
-
-  my ($package, $method) = split /::(\w+)$/, our $AUTOLOAD;
-  my $c = $self->[0];
-  Carp::croak qq{Can't locate object method "$method" via package "$package"}
-    unless my $helper = $c->app->renderer->get_helper("$self->[1].$method");
-  return $c->$helper(@_);
-}
-
-sub DESTROY { }
-
 1;
 
 =encoding utf8

diff --git a/t/mojolicious/renderer.t b/t/mojolicious/renderer.t
@@ -1,6 +1,7 @@
 use Mojo::Base -strict;
 
 use Test::More;
+use Mojo::Util 'decode';
 use Mojolicious::Controller;
 
 # Partial rendering
@@ -95,4 +96,17 @@ eval { $first->app->myapp->missing };
 like $@, qr/^Can't locate object method "missing" via package "$class"/,
   'right error';
 
+# No leaky namespaces
+my $helper_class = ref $second->myapp;
+is ref $second->myapp, $helper_class, 'same class';
+ok $helper_class->can('defaults'), 'helpers are active';
+my $template_class = decode 'UTF-8',
+  $second->render_to_string(inline => "<%= __PACKAGE__ =%>");
+is decode('UTF-8', $second->render_to_string(inline => "<%= __PACKAGE__ =%>")),
+  $template_class, 'same class';
+ok $template_class->can('stash'), 'helpers are active';
+undef $second;
+ok !$helper_class->can('defaults'), 'helpers have been cleaned up';
+ok !$template_class->can('stash'),  'helpers have been cleaned up';
+
 done_testing();