Improve performance of the rendering stack by freezing formats as a sign that they shouldn't be further modified.

7 files changed, 66 insertions, 64 deletions

diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb
index 2ac199265d..fa69ab2038 100644
--- a/actionpack/lib/action_controller/metal/mime_responds.rb
+++ b/actionpack/lib/action_controller/metal/mime_responds.rb
@@ -263,6 +263,7 @@ module ActionController #:nodoc:
       if format = request.negotiate_mime(collector.order)
         self.content_type ||= format.to_s
         self.formats = [format.to_sym]
+        self.formats.freeze
         collector.response_for(format)
       else
         head :not_acceptable
diff --git a/actionpack/lib/action_view/helpers/prototype_helper.rb b/actionpack/lib/action_view/helpers/prototype_helper.rb
index aba2565c67..f53aafa679 100644
--- a/actionpack/lib/action_view/helpers/prototype_helper.rb
+++ b/actionpack/lib/action_view/helpers/prototype_helper.rb
@@ -148,11 +148,9 @@ module ActionView
       class JavaScriptGenerator #:nodoc:
         def initialize(context, &block) #:nodoc:
           @context, @lines = context, []
-          @context.update_details(:formats => [:js, :html]) do
-            include_helpers_from_context
-            @context.with_output_buffer(@lines) do
-              @context.instance_exec(self, &block)
-            end
+          include_helpers_from_context
+          @context.with_output_buffer(@lines) do
+            @context.instance_exec(self, &block)
           end
         end
 
diff --git a/actionpack/lib/action_view/lookup_context.rb b/actionpack/lib/action_view/lookup_context.rb
index 598007c7a4..cf28772f12 100644
--- a/actionpack/lib/action_view/lookup_context.rb
+++ b/actionpack/lib/action_view/lookup_context.rb
@@ -1,4 +1,4 @@
-require 'active_support/core_ext/object/try'
+require 'active_support/core_ext/array/wrap'
 require 'active_support/core_ext/object/blank'
 
 module ActionView
@@ -15,21 +15,23 @@ module ActionView
 
     def self.register_detail(name, options = {}, &block)
       self.registered_details << name
-      Setters.send :define_method, :"_#{name}_defaults", &block
-      Setters.module_eval <<-METHOD, __FILE__, __LINE__ + 1
-        def #{name}=(value)
-          value = Array(value.presence || _#{name}_defaults)
+      Accessors.send :define_method, :"_#{name}_defaults", &block
+      Accessors.module_eval <<-METHOD, __FILE__, __LINE__ + 1
+        def #{name}
+          @details[:#{name}]
+        end
 
-          unless value == @details[:#{name}]
-            @details_key, @details = nil, @details.merge(:#{name} => value)
-            @details.freeze
-          end
+        def #{name}=(value)
+          value = Array.wrap(value.presence || _#{name}_defaults)
+          @details_key = nil unless value == @details[:#{name}]
+          # Always set the value to handle frozen arrays
+          @details[:#{name}] = value
         end
       METHOD
     end
 
-    # Holds raw setters for the registered details.
-    module Setters #:nodoc:
+    # Holds accessors for the registered details.
+    module Accessors #:nodoc:
     end
 
     register_detail(:formats) { Mime::SET.symbols }
@@ -52,9 +54,9 @@ module ActionView
     end
 
     def initialize(view_paths, details = {})
-      @details, @details_key = {}, nil
+      @details, @details_key = { :handlers => default_handlers }, nil
       self.view_paths = view_paths
-      self.details = details
+      self.update_details(details, true)
     end
 
     module ViewPaths
@@ -97,9 +99,7 @@ module ActionView
 
       def args_for_lookup(name, prefix, partial) #:nodoc:
         name, prefix = normalize_name(name, prefix)
-        details_key  = self.details_key
-        details      = self.details.merge(:handlers => default_handlers)
-        [name, prefix, partial || false, details, details_key]
+        [name, prefix, partial || false, @details, details_key]
       end
 
       # Support legacy foo.erb names even though we now ignore .erb
@@ -121,48 +121,44 @@ module ActionView
     end
 
     module Details
-      attr_reader :details
-
-      def details=(given_details)
-        registered_details.each { |key| send(:"#{key}=", given_details[key]) }
-      end
-
-      def details_key
+      # Calculate the details key. Remove the handlers from calculation to improve performance
+      # since the user cannot modify it explicitly.
+      def details_key #:nodoc:
         @details_key ||= DetailsKey.get(@details)
       end
 
-      # Shortcut to read formats from details.
-      def formats
-        @details[:formats].compact
-      end
-
       # Overload formats= to reject [:"*/*"] values.
       def formats=(value)
-        value = nil if value == [:"*/*"]
+        value = nil    if value == [:"*/*"]
+        value << :html if value == [:js]
         super(value)
       end
 
-      # Shortcut to read locale.
+      # Overload locale to return a symbol instead of array
       def locale
-        I18n.locale
+        @details[:locale].first
       end
 
       # Overload locale= to also set the I18n.locale. If the current I18n.config object responds
       # to i18n_config, it means that it's has a copy of the original I18n configuration and it's
       # acting as proxy, which we need to skip.
       def locale=(value)
-        value = value.first if value.is_a?(Array)
-        config = I18n.config.respond_to?(:i18n_config) ? I18n.config.i18n_config : I18n.config
-        config.locale = value if value
+        if value
+          config = I18n.config.respond_to?(:i18n_config) ? I18n.config.i18n_config : I18n.config
+          config.locale = value
+        end
         super(I18n.locale)
       end
 
       # Update the details keys by merging the given hash into the current
       # details hash. If a block is given, the details are modified just during
       # the execution of the block and reverted to the previous value after.
-      def update_details(new_details)
-        old_details  = @details
-        self.details = old_details.merge(new_details)
+      def update_details(new_details, force=false)
+        old_details = @details.dup
+
+        registered_details.each do |key|
+          send(:"#{key}=", new_details[key]) if force || new_details.key?(key)
+        end
 
         if block_given?
           begin
@@ -174,7 +170,7 @@ module ActionView
       end
     end
 
-    include Setters
+    include Accessors
     include Details
     include ViewPaths
   end
diff --git a/actionpack/lib/action_view/render/layouts.rb b/actionpack/lib/action_view/render/layouts.rb
index b4720aa681..c790a70b3e 100644
--- a/actionpack/lib/action_view/render/layouts.rb
+++ b/actionpack/lib/action_view/render/layouts.rb
@@ -51,7 +51,7 @@ module ActionView
         if formats.size == 1
           _find_layout(layout)
         else
-          update_details(:formats => self.formats[0,1]){ _find_layout(layout) }
+          update_details(:formats => self.formats.first){ _find_layout(layout) }
         end
       rescue ActionView::MissingTemplate => e
         update_details(:formats => nil) do
diff --git a/actionpack/lib/action_view/render/rendering.rb b/actionpack/lib/action_view/render/rendering.rb
index b0fed556c3..a8f745afd1 100644
--- a/actionpack/lib/action_view/render/rendering.rb
+++ b/actionpack/lib/action_view/render/rendering.rb
@@ -21,7 +21,7 @@ module ActionView
           _render_partial(options)
         else
           template = _determine_template(options)
-          self.formats = template.formats
+          _freeze_formats(template.formats)
           _render_template(template, options[:layout], options)
         end
       when :update
@@ -58,12 +58,18 @@ module ActionView
         content = template.render(self, locals) { |*name| _layout_for(*name) }
         @_content_for[:layout] = content
 
-        if layout
-          content  = _render_layout(layout, locals)
-        end
-
+        content = _render_layout(layout, locals) if layout
         content
       end
     end
+
+    # Freeze the current formats in the lookup context. By freezing them, you are guaranteeing
+    # that next template lookups are not going to modify the formats. The controller can also
+    # use this, to ensure that formats won't be further modified (as it does in respond_to blocks).
+    def _freeze_formats(formats) #:nodoc:
+      return if self.formats.frozen?
+      self.formats = formats
+      self.formats.freeze
+    end
   end
 end
diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb
index 9145d007a8..8abc1633ff 100644
--- a/actionpack/lib/action_view/template.rb
+++ b/actionpack/lib/action_view/template.rb
@@ -1,7 +1,7 @@
 # encoding: utf-8
 # This is so that templates compiled in this file are UTF-8
-
-require 'set'
+require 'active_support/core_ext/array/wrap'
+require 'active_support/core_ext/object/blank'
 
 module ActionView
   class Template
@@ -26,12 +26,8 @@ module ActionView
       @virtual_path = details[:virtual_path]
       @method_names = {}
 
-      format    = details[:format]
-      format  ||= handler.default_format.to_sym if handler.respond_to?(:default_format)
-      format  ||= :html
-
-      @formats  = [format.to_sym]
-      @formats << :html if @formats.first == :js
+      format   = details[:format] || :html
+      @formats = Array.wrap(format).map(&:to_sym)
     end
 
     def render(view, locals, &block)
diff --git a/actionpack/lib/action_view/template/resolver.rb b/actionpack/lib/action_view/template/resolver.rb
index 28cd30a959..8e8afaa43f 100644
--- a/actionpack/lib/action_view/template/resolver.rb
+++ b/actionpack/lib/action_view/template/resolver.rb
@@ -1,6 +1,5 @@
 require "pathname"
 require "active_support/core_ext/class"
-require "active_support/core_ext/array/wrap"
 require "action_view/template"
 
 module ActionView
@@ -57,7 +56,7 @@ module ActionView
 
     def find_templates(name, prefix, partial, details)
       path = build_path(name, prefix, partial, details)
-      query(path, EXTENSION_ORDER.map { |ext| details[ext] })
+      query(path, EXTENSION_ORDER.map { |ext| details[ext] }, details[:formats])
     end
 
     def build_path(name, prefix, partial, details)
@@ -67,7 +66,7 @@ module ActionView
       path
     end
 
-    def query(path, exts)
+    def query(path, exts, formats)
       query = File.join(@path, path)
 
       exts.each do |ext|
@@ -75,18 +74,24 @@ module ActionView
       end
 
       Dir[query].reject { |p| File.directory?(p) }.map do |p|
-        handler, format = extract_handler_and_format(p)
+        handler, format = extract_handler_and_format(p, formats)
         Template.new(File.read(p), File.expand_path(p), handler,
           :virtual_path => path, :format => format)
       end
     end
 
-    def extract_handler_and_format(path)
+    # Extract handler and formats from path. If a format cannot be a found neither
+    # from the path, or the handler, we should return the array of formats given
+    # to the resolver.
+    def extract_handler_and_format(path, default_formats)
       pieces = File.basename(path).split(".")
       pieces.shift
 
-      handler = Template.handler_class_for_extension(pieces.pop)
-      format  = pieces.last && Mime[pieces.last] && pieces.pop.to_sym
+      handler  = Template.handler_class_for_extension(pieces.pop)
+      format   = pieces.last && Mime[pieces.last] && pieces.pop.to_sym
+      format ||= handler.default_format if handler.respond_to?(:default_format)
+      format ||= default_formats
+
       [handler, format]
     end
   end