From b2600bfc181664fcfe448d100ca054017b0576dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Thu, 7 Oct 2010 15:50:20 +0200 Subject: Remove locals dependency from template. This means that templates does not need to store its source anymore, allowing us to reduce the ammount of memory taken by our Rails processes. Naively speaking, if your app/views contains 2MB of files, each of your processes (after being hit by a bunch of requests) will take 2MB less of memory after this commit. This is extremely important for the upcoming features. Since Rails will also render CSS and JS files, their source won't be stored as well allowing us to decrease the ammount of memory taken. --- actionpack/lib/action_view/lookup_context.rb | 16 ++--- actionpack/lib/action_view/paths.rb | 4 +- actionpack/lib/action_view/render/layouts.rb | 4 +- actionpack/lib/action_view/render/partials.rb | 80 ++++++++++++++----------- actionpack/lib/action_view/render/rendering.rb | 10 ++-- actionpack/lib/action_view/template.rb | 66 +++++++++++++------- actionpack/lib/action_view/template/resolver.rb | 30 +++++++--- actionpack/lib/action_view/template/text.rb | 4 -- actionpack/test/template/template_test.rb | 1 + 9 files changed, 130 insertions(+), 85 deletions(-) diff --git a/actionpack/lib/action_view/lookup_context.rb b/actionpack/lib/action_view/lookup_context.rb index 3ea8b86af1..794fdeae64 100644 --- a/actionpack/lib/action_view/lookup_context.rb +++ b/actionpack/lib/action_view/lookup_context.rb @@ -77,17 +77,17 @@ module ActionView @view_paths = ActionView::Base.process_view_paths(paths) end - def find(name, prefix = nil, partial = false) - @view_paths.find(*args_for_lookup(name, prefix, partial)) + def find(name, prefix = nil, partial = false, keys = []) + @view_paths.find(*args_for_lookup(name, prefix, partial, keys)) end alias :find_template :find - def find_all(name, prefix = nil, partial = false) - @view_paths.find_all(*args_for_lookup(name, prefix, partial)) + def find_all(name, prefix = nil, partial = false, keys = []) + @view_paths.find_all(*args_for_lookup(name, prefix, partial, keys)) end - def exists?(name, prefix = nil, partial = false) - @view_paths.exists?(*args_for_lookup(name, prefix, partial)) + def exists?(name, prefix = nil, partial = false, keys = []) + @view_paths.exists?(*args_for_lookup(name, prefix, partial, keys)) end alias :template_exists? :exists? @@ -106,9 +106,9 @@ module ActionView protected - def args_for_lookup(name, prefix, partial) #:nodoc: + def args_for_lookup(name, prefix, partial, keys) #:nodoc: name, prefix = normalize_name(name, prefix) - [name, prefix, partial || false, @details, details_key] + [name, prefix, partial || false, @details, keys, details_key] end # Support legacy foo.erb names even though we now ignore .erb diff --git a/actionpack/lib/action_view/paths.rb b/actionpack/lib/action_view/paths.rb index 9857d688d2..dc26d75ff3 100644 --- a/actionpack/lib/action_view/paths.rb +++ b/actionpack/lib/action_view/paths.rb @@ -10,8 +10,8 @@ module ActionView #:nodoc: METHOD end - def find(path, prefix = nil, partial = false, details = {}, key = nil) - template = find_all(path, prefix, partial, details, key).first + def find(path, prefix = nil, partial = false, details = {}, keys = [], key = nil) + template = find_all(path, prefix, partial, details, keys, key).first raise MissingTemplate.new(self, "#{prefix}/#{path}", details, partial) unless template template end diff --git a/actionpack/lib/action_view/render/layouts.rb b/actionpack/lib/action_view/render/layouts.rb index 8882acca31..08162f7fd5 100644 --- a/actionpack/lib/action_view/render/layouts.rb +++ b/actionpack/lib/action_view/render/layouts.rb @@ -62,11 +62,11 @@ module ActionView # This is the method which actually finds the layout using details in the lookup # context object. If no layout is found, it checks if at least a layout with # the given name exists across all details before raising the error. - def find_layout(layout) + def find_layout(layout, keys) begin with_layout_format do layout =~ /^\// ? - with_fallbacks { find_template(layout) } : find_template(layout) + with_fallbacks { find_template(layout, nil, false, keys) } : find_template(layout, nil, false, keys) end rescue ActionView::MissingTemplate => e update_details(:formats => nil) do diff --git a/actionpack/lib/action_view/render/partials.rb b/actionpack/lib/action_view/render/partials.rb index cc9b444837..07e844afc2 100644 --- a/actionpack/lib/action_view/render/partials.rb +++ b/actionpack/lib/action_view/render/partials.rb @@ -218,7 +218,6 @@ module ActionView def initialize(view_context, options, block) @view = view_context @partial_names = PARTIAL_NAMES[@view.controller.class.name] - setup(options, block) end @@ -237,16 +236,28 @@ module ActionView @object = partial if @collection = collection - paths = @collection_paths = @collection.map { |o| partial_path(o) } + paths = @collection_data = @collection.map { |o| partial_path(o) } @path = paths.uniq.size == 1 ? paths.first : nil else @path = partial_path end end + + if @path + @variable, @variable_counter = retrieve_variable(@path) + else + paths.map! { |path| retrieve_variable(path).unshift(path) } + end + end + + def retrieve_variable(path) + variable = @options[:as] || path[%r'_?(\w+)(\.\w+)*$', 1].to_sym + variable_counter = :"#{variable}_counter" if @collection + [variable, variable_counter] end def render - identifier = ((@template = find_template) ? @template.identifier : @path) + identifier = ((@template = find_partial) ? @template.identifier : @path) if @collection ActiveSupport::Notifications.instrument("render_collection.action_view", @@ -254,15 +265,16 @@ module ActionView render_collection end else + if !@block && (layout = @options[:layout]) + layout = find_template(layout) + end + content = ActiveSupport::Notifications.instrument("render_partial.action_view", :identifier => identifier) do render_partial end - if !@block && (layout = @options[:layout]) - content = @view._render_layout(find_template(layout), @locals){ content } - end - + content = @view._render_layout(layout, @locals){ content } if layout content end end @@ -278,16 +290,9 @@ module ActionView result.join(spacer).html_safe end - def collection_with_template(template = @template) + def collection_with_template segments, locals, template = [], @locals, @template - - if @options[:as] - as = @options[:as] - counter = "#{as}_counter".to_sym - else - as = template.variable_name - counter = template.counter_name - end + as, counter = @variable, @variable_counter locals[counter] = -1 @@ -300,20 +305,16 @@ module ActionView segments end - def collection_without_template(collection_paths = @collection_paths) - segments, locals = [], @locals + def collection_without_template + segments, locals, collection_data = [], @locals, @collection_data index, template = -1, nil - - if @options[:as] - as = @options[:as] - counter = "#{as}_counter" - end + keys = @locals.keys @collection.each_with_index do |object, i| - template = find_template(collection_paths[i]) - locals[as || template.variable_name] = object - locals[counter || template.counter_name] = (index += 1) - + path, *data = collection_data[i] + template = find_template(path, keys + data) + locals[data[0]] = object + locals[data[1]] = (index += 1) segments << template.render(@view, locals) end @@ -321,13 +322,14 @@ module ActionView segments end - def render_partial(object = @object) - locals, view, template = @locals, @view, @template + def render_partial + locals, view = @locals, @view + object, as = @object, @variable - object ||= locals[template.variable_name] - locals[@options[:as] || template.variable_name] = object + object ||= locals[as] + locals[as] = object - template.render(view, locals) do |*name| + @template.render(view, locals) do |*name| view._layout_for(*name, &@block) end end @@ -342,10 +344,18 @@ module ActionView end end - def find_template(path=@path) - return path unless path.is_a?(String) + def find_partial + if path = @path + locals = @locals.keys + locals << @variable + locals << @variable_counter if @collection + find_template(path, locals) + end + end + + def find_template(path=@path, locals=@locals.keys) prefix = @view.controller_path unless path.include?(?/) - @view.find_template(path, prefix, true) + @view.find_template(path, prefix, true, locals) end def partial_path(object = @object) diff --git a/actionpack/lib/action_view/render/rendering.rb b/actionpack/lib/action_view/render/rendering.rb index 5320245173..0771b40c37 100644 --- a/actionpack/lib/action_view/render/rendering.rb +++ b/actionpack/lib/action_view/render/rendering.rb @@ -34,16 +34,18 @@ module ActionView # Determine the template to be rendered using the given options. def _determine_template(options) #:nodoc: + keys = (options[:locals] ||= {}).keys + if options.key?(:inline) handler = Template.handler_class_for_extension(options[:type] || "erb") - Template.new(options[:inline], "inline template", handler, {}) + Template.new(options[:inline], "inline template", handler, { :locals => keys }) elsif options.key?(:text) Template::Text.new(options[:text], formats.try(:first)) elsif options.key?(:file) - with_fallbacks { find_template(options[:file], options[:prefix]) } + with_fallbacks { find_template(options[:file], options[:prefix], false, keys) } elsif options.key?(:template) options[:template].respond_to?(:render) ? - options[:template] : find_template(options[:template], options[:prefix]) + options[:template] : find_template(options[:template], options[:prefix], false, keys) end end @@ -51,7 +53,7 @@ module ActionView # supplied as well. def _render_template(template, layout = nil, options = {}) #:nodoc: locals = options[:locals] || {} - layout = find_layout(layout) if layout + layout = find_layout(layout, locals.keys) if layout ActiveSupport::Notifications.instrument("render_template.action_view", :identifier => template.identifier, :layout => layout.try(:virtual_path)) do diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index 164d177dcc..7e65654d0b 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -98,6 +98,8 @@ module ActionView extend Template::Handlers + attr_accessor :locals + attr_reader :source, :identifier, :handler, :virtual_path, :formats, :original_encoding @@ -117,23 +119,17 @@ module ActionView @handler = handler @original_encoding = nil @method_names = {} - - format = details[:format] || :html - @formats = Array.wrap(format).map(&:to_sym) - @virtual_path = details[:virtual_path].try(:sub, ".#{format}", "") + @locals = details[:locals] || [] + @formats = Array.wrap(details[:format] || :html).map(&:to_sym) + @virtual_path = details[:virtual_path].try(:sub, ".#{@formats.first}", "") + @compiled = false end def render(view, locals, &block) # Notice that we use a bang in this instrumentation because you don't want to # consume this in production. This is only slow if it's being listened to. ActiveSupport::Notifications.instrument("!render_template.action_view", :virtual_path => @virtual_path) do - if view.is_a?(ActionView::CompiledTemplates) - mod = ActionView::CompiledTemplates - else - mod = view.singleton_class - end - - method_name = compile(locals, view, mod) + compile!(view) view.send(method_name, locals, &block) end rescue Exception => e @@ -141,7 +137,7 @@ module ActionView e.sub_template_of(self) raise e else - raise Template::Error.new(self, view.respond_to?(:assigns) ? view.assigns : {}, e) + raise Template::Error.new(refresh(view) || self, view.respond_to?(:assigns) ? view.assigns : {}, e) end end @@ -149,10 +145,12 @@ module ActionView @mime_type ||= Mime::Type.lookup_by_extension(@formats.first.to_s) if @formats.first end + # TODO Remove me def variable_name @variable_name ||= @virtual_path[%r'_?(\w+)(\.\w+)*$', 1].to_sym end + # TODO Remove me def counter_name @counter_name ||= "#{variable_name}_counter".to_sym end @@ -166,7 +164,25 @@ module ActionView end end - private + def compile!(view) + return if @compiled + + if view.is_a?(ActionView::CompiledTemplates) + mod = ActionView::CompiledTemplates + else + mod = view.singleton_class + end + + compile(view, mod) + + # Just discard the source if we have a virtual path. This + # means we can get the template back. + @source = nil if @virtual_path + @compiled = true + end + + protected + # Among other things, this method is responsible for properly setting # the encoding of the source. Until this point, we assume that the # source is BINARY data. If no additional information is supplied, @@ -187,11 +203,9 @@ module ActionView # encode the source into Encoding.default_internal. In general, # this means that templates will be UTF-8 inside of Rails, # regardless of the original source encoding. - def compile(locals, view, mod) - method_name = build_method_name(locals) - return method_name if view.respond_to?(method_name) - - locals_code = locals.keys.map! { |key| "#{key} = local_assigns[:#{key}];" }.join + def compile(view, mod) + method_name = self.method_name + locals_code = @locals.map { |key| "#{key} = local_assigns[:#{key}];" }.join if source.encoding_aware? # Look for # encoding: *. If we find one, we'll encode the @@ -256,8 +270,6 @@ module ActionView begin mod.module_eval(source, identifier, 0) ObjectSpace.define_finalizer(self, Finalizer[method_name, mod]) - - method_name rescue Exception => e # errors from template code if logger = (view && view.logger) logger.debug "ERROR: compiling #{method_name} RAISED #{e}" @@ -269,12 +281,20 @@ module ActionView end end - def build_method_name(locals) - @method_names[locals.keys.hash] ||= "_#{identifier_method_name}__#{@identifier.hash}_#{__id__}_#{locals.keys.hash}".gsub('-', "_") + def refresh(view) + return unless @virtual_path + pieces = @virtual_path.split("/") + name = pieces.pop + partial = name.sub!(/^_/, "") + view.find_template(name, pieces.join, partial || false, @locals) + end + + def method_name + @method_name ||= "_#{identifier_method_name}__#{@identifier.hash}_#{__id__}".gsub('-', "_") end def identifier_method_name - @identifier_method_name ||= inspect.gsub(/[^a-z_]/, '_') + inspect.gsub(/[^a-z_]/, '_') end end end diff --git a/actionpack/lib/action_view/template/resolver.rb b/actionpack/lib/action_view/template/resolver.rb index a031d0dc33..9ec39b16f0 100644 --- a/actionpack/lib/action_view/template/resolver.rb +++ b/actionpack/lib/action_view/template/resolver.rb @@ -6,8 +6,8 @@ module ActionView # = Action View Resolver class Resolver def initialize - @cached = Hash.new { |h1,k1| h1[k1] = - Hash.new { |h2,k2| h2[k2] = Hash.new { |h3, k3| h3[k3] = {} } } } + @cached = Hash.new { |h1,k1| h1[k1] = Hash.new { |h2,k2| + h2[k2] = Hash.new { |h3,k3| h3[k3] = Hash.new { |h4,k4| h4[k4] = {} } } } } end def clear_cache @@ -15,8 +15,8 @@ module ActionView end # Normalizes the arguments and passes it on to find_template. - def find_all(name, prefix=nil, partial=false, details={}, key=nil) - cached(key, prefix, name, partial) do + def find_all(name, prefix=nil, partial=false, details={}, locals=[], key=nil) + cached(key, prefix, name, partial, locals) do find_templates(name, prefix, partial, details) end end @@ -51,9 +51,25 @@ module ActionView [handler, format] end - def cached(key, prefix, name, partial) - return yield unless key && caching? - @cached[key][prefix][name][partial] ||= yield + def cached(key, prefix, name, partial, locals) + locals = sort_locals(locals) + unless key && caching? + yield.each { |t| t.locals = locals } + else + @cached[key][prefix][name][partial][locals] ||= yield.each { |t| t.locals = locals } + end + end + + if :locale.respond_to?("<=>") + def sort_locals(locals) + locals.sort.freeze + end + else + def sort_locals(locals) + locals = locals.map{ |l| l.to_s } + locals.sort! + locals.freeze + end end end diff --git a/actionpack/lib/action_view/template/text.rb b/actionpack/lib/action_view/template/text.rb index 51be831dfb..4261c3b5e2 100644 --- a/actionpack/lib/action_view/template/text.rb +++ b/actionpack/lib/action_view/template/text.rb @@ -25,10 +25,6 @@ module ActionView #:nodoc: def formats [@mime_type.to_sym] end - - def partial? - false - end end end end diff --git a/actionpack/test/template/template_test.rb b/actionpack/test/template/template_test.rb index c7c33af670..210ba673f0 100644 --- a/actionpack/test/template/template_test.rb +++ b/actionpack/test/template/template_test.rb @@ -51,6 +51,7 @@ class TestERBTemplate < ActiveSupport::TestCase def test_locals @template = new_template("<%= my_local %>") + @template.locals = [:my_local] assert_equal "I'm a local", render(:my_local => "I'm a local") end -- cgit v1.2.3