Performance speedup for ActionController (closes #4174) [Stefan Kaes] Includes caching of filter chains -- be on the lookout for problems with that!

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@3989 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

8 files changed, 80 insertions, 78 deletions

diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb
index afb17d7655..52bddac092 100755
--- a/actionpack/lib/action_controller/base.rb
+++ b/actionpack/lib/action_controller/base.rb
@@ -371,13 +371,13 @@ module ActionController #:nodoc:
         initialize_template_class(response)
         assign_shortcuts(request, response)
         initialize_current_url
-        action_name(:refresh)
+        assign_names
         forget_variables_added_to_assigns
         
         log_processing
         send(method, *arguments)
         
-        return response
+        response
       ensure
         process_cleanup
       end
@@ -469,15 +469,6 @@ module ActionController #:nodoc:
         self.class.controller_name
       end
 
-      # Returns the name of the current action
-      def action_name(refresh = false)
-        if @action_name.nil? || refresh
-          @action_name = (params['action'] || 'index')
-        end
-        
-        @action_name
-      end
-
       def session_enabled?
         request.session_options[:disabled] != false
       end
@@ -868,12 +859,11 @@ module ActionController #:nodoc:
     
     private
       def self.view_class
-        unless @view_class
+        @view_class ||=
           # create a new class based on the default template class and include helper methods
-          @view_class = Class.new(ActionView::Base)
-          @view_class.send(:include, master_helper_module)
-        end
-        @view_class
+          returning Class.new(ActionView::Base) do |view_class|
+            view_class.send(:include, master_helper_module)
+          end
       end
 
       def self.view_root
@@ -928,6 +918,10 @@ module ActionController #:nodoc:
         @performed_render || @performed_redirect
       end
 
+      def assign_names
+        @action_name = (params['action'] || 'index')
+      end
+      
       def action_methods
         self.class.action_methods
       end
@@ -997,9 +991,7 @@ module ActionController #:nodoc:
       end
 
       def template_exempt_from_layout?(template_name = default_template_name)
-        template_name.last(3) == "rjs" || @template.pick_template_extension(template_name).to_sym == :rjs
-      rescue
-        false
+        template_name =~ /\.rjs$/ || (@template.pick_template_extension(template_name) == :rjs rescue false)
       end
 
       def assert_existance_of_template_file(template_name)
@@ -1010,20 +1002,22 @@ module ActionController #:nodoc:
         end
       end
 
-      def default_template_name(default_action_name = action_name)
-        if default_action_name
-          default_action_name = default_action_name.to_s.dup
-          strip_out_controller!(default_action_name) if template_path_includes_controller?(default_action_name)
+      def default_template_name(action_name = self.action_name)
+        if action_name
+          action_name = action_name.to_s
+          if action_name.include?('/') && template_path_includes_controller?(action_name)
+            action_name = strip_out_controller(action_name)
+          end
         end
-        "#{self.class.controller_path}/#{default_action_name}"
+        "#{self.class.controller_path}/#{action_name}"
       end
       
-      def strip_out_controller!(path)
-        path.replace path.split('/', 2).last
+      def strip_out_controller(path)
+        path.split('/', 2).last
       end
       
       def template_path_includes_controller?(path)
-        path.to_s['/'] && self.class.controller_path.split('/')[-1] == path.split('/')[0]
+        self.class.controller_path.split('/')[-1] == path.split('/')[0]
       end
 
       def process_cleanup
diff --git a/actionpack/lib/action_controller/cgi_process.rb b/actionpack/lib/action_controller/cgi_process.rb
index b710f05d7c..ce8acdf313 100644
--- a/actionpack/lib/action_controller/cgi_process.rb
+++ b/actionpack/lib/action_controller/cgi_process.rb
@@ -159,7 +159,6 @@ end_msg
 
       def session_options_with_string_keys
         @session_options_with_string_keys ||= DEFAULT_SESSION_OPTIONS.merge(@session_options).inject({}) { |options, (k,v)| options[k.to_s] = v; options }
-        @session_options_with_string_keys
       end
   end
 
diff --git a/actionpack/lib/action_controller/components.rb b/actionpack/lib/action_controller/components.rb
index 3ea83463e3..cc57574f08 100644
--- a/actionpack/lib/action_controller/components.rb
+++ b/actionpack/lib/action_controller/components.rb
@@ -44,7 +44,11 @@ module ActionController #:nodoc:
           @controller.send(:render_component_as_string, options)
         end
       end
-
+            
+      # If this controller was instantiated to process a component request,
+      # +parent_controller+ points to the instantiator of this controller.
+      base.send :attr_accessor, :parent_controller
+      
       base.class_eval do
         alias_method :process_cleanup_without_components, :process_cleanup
         alias_method :process_cleanup, :process_cleanup_with_components
@@ -54,11 +58,9 @@ module ActionController #:nodoc:
         
         alias_method :flash_without_components, :flash
         alias_method :flash, :flash_with_components
+
+        alias_method :component_request?, :parent_controller       
       end
-      
-      # If this controller was instantiated to process a component request,
-      # +parent_controller+ points to the instantiator of this controller.
-      base.send :attr_accessor, :parent_controller
     end
 
     module ClassMethods
@@ -171,10 +173,6 @@ module ActionController #:nodoc:
             yield
           end
         end
-        
-        def component_request?
-          !@parent_controller.nil?
-        end
 
         def set_session_options_with_components(request)
           set_session_options_without_components(request) unless component_request?
diff --git a/actionpack/lib/action_controller/filters.rb b/actionpack/lib/action_controller/filters.rb
index 0a6a648885..2b260b80d7 100644
--- a/actionpack/lib/action_controller/filters.rb
+++ b/actionpack/lib/action_controller/filters.rb
@@ -283,22 +283,22 @@ module ActionController #:nodoc:
       
       # Returns all the before filters for this class and all its ancestors.
       def before_filters #:nodoc:
-        read_inheritable_attribute("before_filters") || []
+        @before_filters ||= read_inheritable_attribute("before_filters") || []
       end
       
       # Returns all the after filters for this class and all its ancestors.
       def after_filters #:nodoc:
-        read_inheritable_attribute("after_filters") || []
+        @after_filters ||= read_inheritable_attribute("after_filters") || []
       end
       
       # Returns a mapping between filters and the actions that may run them.
       def included_actions #:nodoc:
-        read_inheritable_attribute("included_actions") || {}
+        @included_actions ||= read_inheritable_attribute("included_actions") || {}
       end
       
       # Returns a mapping between filters and actions that may not run them.
       def excluded_actions #:nodoc:
-        read_inheritable_attribute("excluded_actions") || {}
+        @excluded_actions ||= read_inheritable_attribute("excluded_actions") || {}
       end
       
       private
diff --git a/actionpack/lib/action_controller/layout.rb b/actionpack/lib/action_controller/layout.rb
index fdee677bbb..7ecff73380 100644
--- a/actionpack/lib/action_controller/layout.rb
+++ b/actionpack/lib/action_controller/layout.rb
@@ -170,7 +170,11 @@ module ActionController #:nodoc:
       end
 
       def layout_conditions #:nodoc:
-        read_inheritable_attribute("layout_conditions")
+        @layout_conditions ||= read_inheritable_attribute("layout_conditions")
+      end
+      
+      def default_layout #:nodoc:
+        @default_layout ||= read_inheritable_attribute("layout")
       end
 
       private
@@ -205,7 +209,7 @@ module ActionController #:nodoc:
     # object). If the layout was defined without a directory, layouts is assumed. So <tt>layout "weblog/standard"</tt> will return
     # weblog/standard, but <tt>layout "standard"</tt> will return layouts/standard.
     def active_layout(passed_layout = nil)
-      layout = passed_layout || self.class.read_inheritable_attribute("layout")
+      layout = passed_layout || self.class.default_layout
 
       active_layout = case layout
         when String then layout
@@ -249,6 +253,7 @@ module ActionController #:nodoc:
     end
 
     private
+    
       def apply_layout?(template_with_options, options)
         return false if options == :update
         template_with_options ?  candidate_for_layout?(options) : !template_exempt_from_layout?
diff --git a/actionpack/lib/action_controller/request.rb b/actionpack/lib/action_controller/request.rb
index 566ae42ba0..76257c1dbb 100755
--- a/actionpack/lib/action_controller/request.rb
+++ b/actionpack/lib/action_controller/request.rb
@@ -48,30 +48,30 @@ module ActionController
     # For backward compatibility, the post format is extracted from the
     # X-Post-Data-Format HTTP header if present.
     def content_type
-      return @content_type if @content_type
-
-      @content_type = @env['CONTENT_TYPE'].to_s.downcase
-
-      if @env['HTTP_X_POST_DATA_FORMAT']          
-        case @env['HTTP_X_POST_DATA_FORMAT'].downcase.to_sym
-          when :yaml
-            @content_type = 'application/x-yaml'
-          when :xml
-            @content_type = 'application/xml'
+      @content_type ||=
+        begin
+          content_type = @env['CONTENT_TYPE'].to_s.downcase
+          
+          if x_post_format = @env['HTTP_X_POST_DATA_FORMAT']
+            case x_post_format.to_s.downcase
+            when 'yaml'
+              content_type = 'application/x-yaml'
+            when 'xml'
+              content_type = 'application/xml'
+            end
           end
-      end
-
-      @content_type = Mime::Type.lookup(@content_type)
+          
+          Mime::Type.lookup(content_type)
+        end
     end
 
     def accepts
-      return @accepts if @accepts
-      
-      @accepts = if @env['HTTP_ACCEPT'].to_s.strip.blank?
-        [ content_type, Mime::ALL ]
-      else
-        Mime::Type.parse(@env['HTTP_ACCEPT'])
-      end
+      @accepts ||=
+        if @env['HTTP_ACCEPT'].to_s.strip.empty?
+          [ content_type, Mime::ALL ]
+        else
+          Mime::Type.parse(@env['HTTP_ACCEPT'])
+        end
     end
 
     # Returns true if the request's "X-Requested-With" header contains
diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb
index dff5a365d4..f4505e90ad 100644
--- a/actionpack/lib/action_view/base.rb
+++ b/actionpack/lib/action_view/base.rb
@@ -182,7 +182,11 @@ module ActionView #:nodoc:
     # map method names to the names passed in local assigns so far
     @@template_args = {}
     # count the number of inline templates
-    @@inline_template_count = 0    
+    @@inline_template_count = 0
+    # maps template paths without extension to their file extension returned by pick_template_extension.
+    # if for a given path, path.ext1 and path.ext2 exist on the file system, the order of extensions
+    # used by pick_template_extension determines whether ext1 or ext2 will be stored
+    @@cached_template_extension = {}
 
     class ObjectWrapper < Struct.new(:value) #:nodoc:
     end
@@ -218,7 +222,7 @@ module ActionView #:nodoc:
     # it's relative to the template_root, otherwise it's absolute. The hash in <tt>local_assigns</tt> 
     # is made available as local variables.
     def render_file(template_path, use_full_path = true, local_assigns = {})
-      @first_render = template_path if @first_render.nil?
+      @first_render ||= template_path
 
       if use_full_path
         template_path_without_extension, template_extension = path_and_extension(template_path)
@@ -226,7 +230,7 @@ module ActionView #:nodoc:
         if template_extension
           template_file_name = full_template_path(template_path_without_extension, template_extension)
         else
-          template_extension = pick_template_extension(template_path)
+          template_extension = pick_template_extension(template_path).to_s
           template_file_name = full_template_path(template_path, template_extension)
         end
       else
@@ -308,14 +312,15 @@ module ActionView #:nodoc:
     end
 
     def pick_template_extension(template_path)#:nodoc:
-      if match = delegate_template_exists?(template_path)
-        match.first
-      elsif erb_template_exists?(template_path):        'rhtml'
-      elsif builder_template_exists?(template_path):    'rxml'
-      elsif javascript_template_exists?(template_path): 'rjs'
-      else
-        raise ActionViewError, "No rhtml, rxml, rjs or delegate template found for #{template_path}"
-      end
+      @@cached_template_extension[template_path] ||=
+        if match = delegate_template_exists?(template_path)
+          match.first.to_sym
+        elsif erb_template_exists?(template_path):        :rhtml
+        elsif builder_template_exists?(template_path):    :rxml
+        elsif javascript_template_exists?(template_path): :rjs
+        else
+          raise ActionViewError, "No rhtml, rxml, rjs or delegate template found for #{template_path}"
+        end
     end
 
     def delegate_template_exists?(template_path)#:nodoc:
@@ -340,9 +345,10 @@ module ActionView #:nodoc:
       if template_file_extension
         template_exists?(template_file_name, template_file_extension)
       else
-        %w(erb builder javascript delegate).any? do |template_type| 
-          send("#{template_type}_template_exists?", template_path)
-        end
+        @@cached_template_extension[template_path] ||
+           %w(erb builder javascript delegate).any? do |template_type| 
+             send("#{template_type}_template_exists?", template_path)
+           end
       end
     end
 
diff --git a/activesupport/lib/active_support/core_ext/hash/indifferent_access.rb b/activesupport/lib/active_support/core_ext/hash/indifferent_access.rb
index 241506b4b8..80012aa17d 100644
--- a/activesupport/lib/active_support/core_ext/hash/indifferent_access.rb
+++ b/activesupport/lib/active_support/core_ext/hash/indifferent_access.rb
@@ -21,7 +21,7 @@ class HashWithIndifferentAccess < Hash
   end
 
   def update(other_hash)
-    other_hash.each_pair {|key, value| regular_writer(convert_key(key), convert_value(value))}
+    other_hash.each_pair { |key, value| regular_writer(convert_key(key), convert_value(value)) }
     self
   end
   alias_method :merge!, :update