From 4f40b2d8fbcfb437a628a353eb281553fc840728 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Sun, 24 Jul 2005 16:45:39 +0000 Subject: Improved performance of test app req/sec with ~10% refactoring the render method #1823 [Stefan Kaes] git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@1915 5ecf4fe2-1ee6-0310-87b1-e25e094e27de --- actionpack/lib/action_controller.rb | 2 +- actionpack/lib/action_controller/base.rb | 148 ++++++++++++--------- actionpack/lib/action_controller/benchmarking.rb | 6 +- .../lib/action_controller/deprecated_redirects.rb | 17 +++ .../deprecated_renders_and_redirects.rb | 76 ----------- actionpack/lib/action_controller/layout.rb | 70 +++++----- actionpack/lib/action_view/base.rb | 6 +- actionpack/lib/action_view/partials.rb | 12 +- 8 files changed, 161 insertions(+), 176 deletions(-) create mode 100644 actionpack/lib/action_controller/deprecated_redirects.rb delete mode 100644 actionpack/lib/action_controller/deprecated_renders_and_redirects.rb (limited to 'actionpack/lib') diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index 253ff6a467..cbcb35b686 100755 --- a/actionpack/lib/action_controller.rb +++ b/actionpack/lib/action_controller.rb @@ -32,7 +32,7 @@ rescue LoadError end require 'action_controller/base' -require 'action_controller/deprecated_renders_and_redirects' +require 'action_controller/deprecated_redirects' require 'action_controller/rescue' require 'action_controller/benchmarking' require 'action_controller/filters' diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 471eb34a17..995edfd9e6 100755 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -571,78 +571,106 @@ module ActionController #:nodoc: # # # Renders an empty response with status code 401 (access denied) # render :nothing => true, :status => 401 - def render(options = {}, deprecated_status = nil) #:doc: - # puts "Rendering: #{options.inspect}" - raise DoubleRenderError if performed? + def render(options = nil, deprecated_status = nil) #:doc: + raise DoubleRenderError, "Can only render or redirect once per action" if performed? # Backwards compatibility - return render({ :template => options || default_template_name, :status => deprecated_status }) if !options.is_a?(Hash) + unless options.is_a?(Hash) + return render_file(options || default_template_name, deprecated_status, true) + end - add_variables_to_assigns - options[:status] = (options[:status] || DEFAULT_RENDER_STATUS_CODE).to_s - - if options[:text] - @response.headers["Status"] = options[:status] - @response.body = options[:text] - @performed_render = true - return options[:text] - - elsif options[:file] - assert_existance_of_template_file(options[:file]) if options[:use_full_path] - logger.info("Rendering #{options[:file]} (#{options[:status]})") unless logger.nil? - render(options.merge({ :text => @template.render_file(options[:file], options[:use_full_path])})) - - elsif options[:template] - render(options.merge({ :file => options[:template], :use_full_path => true })) - - elsif options[:inline] - render(options.merge({ - :text => - @template.render_template( - options[:type] || :rhtml, - options[:inline], - nil, - options[:locals] || {} - ) - })) - - elsif options[:action] - render(options.merge({ :template => default_template_name(options[:action]) })) - - elsif options[:partial] && options[:collection] - render(options.merge({ - :text => ( - @template.render_partial_collection( - options[:partial] == true ? default_template_name : options[:partial], - options[:collection], options[:spacer_template], - options[:locals] || {} - ) || '' - ) - })) - - elsif options[:partial] - render(options.merge({ :text => @template.render_partial( - options[:partial] == true ? default_template_name : options[:partial], - options[:object], options[:locals] || {} - ) })) - - elsif options[:nothing] - # Safari doesn't pass the headers of the return if the response is zero length - render(options.merge({ :text => " " })) + if text = options[:text] + render_text(text, options[:status]) else - render(options.merge({ :action => action_name })) + if file = options[:file] + render_file(file, options[:status], options[:use_full_path]) + + elsif template = options[:template] + render_file(template, options[:status], true) + + elsif inline = options[:inline] + render_template(inline, options[:status], options[:type]) + + elsif action_name = options[:action] + render_action(action_name, options[:status], options[:layout]) + + elsif partial = options[:partial] + partial = default_template_name if partial == true + if collection = options[:collection] + render_partial_collection(partial, collection, options[:spacer_template], options[:locals], options[:status]) + else + render_partial(partial, options[:object], options[:locals], options[:status]) + end + + elsif options[:nothing] + # Safari doesn't pass the headers of the return if the response is zero length + render_text(" ", options[:status]) + + else + render_file(default_template_name, options[:status], true) + + end end end # Renders according to the same rules as render, but returns the result in a string instead # of sending it as the response body to the browser. - def render_to_string(options = {}) #:doc: + def render_to_string(options = nil) #:doc: result = render(options) erase_render_results - return result + result + end + + def render_action(action_name, status = nil, with_layout = true) + # logger.info("Rendering action #{action_name} (#{status},#{with_layout})") if logger + if with_layout + render_with_layout(default_template_name(action_name), status) + else + render_with_no_layout(default_template_name(action_name), status) + end + end + + def render_file(template_path, status = nil, use_full_path = false) + add_variables_to_assigns + assert_existance_of_template_file(template_path) if use_full_path + logger.info("Rendering #{template_path} (#{status})") if logger + render_text(@template.render_file(template_path, use_full_path), status) end - + + def render_template(template, status = nil, type = :rhtml) + add_variables_to_assigns + render_text(@template.render_template(type, template), status) + end + + def render_text(text = nil, status = nil) + @performed_render = true + @response.headers['Status'] = (status || DEFAULT_RENDER_STATUS_CODE).to_s + @response.body = text + end + + def render_nothing(status = nil) + render_text(' ', status) + end + + def render_partial(partial_path = default_template_name, object = nil, local_assigns = nil, status = nil) + add_variables_to_assigns + render_text(@template.render_partial(partial_path, object, local_assigns), status) + end + + def render_partial_collection(partial_name, collection, partial_spacer_template = nil, local_assigns = nil, status = nil) + add_variables_to_assigns + render_text(@template.render_partial_collection(partial_name, collection, partial_spacer_template, local_assigns), status) + end + + def render_with_layout(template_name = default_template_name, status = nil, layout = nil) + render_with_a_layout(template_name, status, layout) + end + + def render_without_layout(template_name = default_template_name, status = nil) + render_with_no_layout(template_name, status) + end + # Clears the rendered results, allowing for another render to be performed. def erase_render_results #:nodoc: @@ -770,8 +798,6 @@ module ActionController #:nodoc: @template = @response.template @assigns = @response.template.assigns @headers = @response.headers - - @variables_added = nil end def initialize_current_url diff --git a/actionpack/lib/action_controller/benchmarking.rb b/actionpack/lib/action_controller/benchmarking.rb index db129f1839..386a291196 100644 --- a/actionpack/lib/action_controller/benchmarking.rb +++ b/actionpack/lib/action_controller/benchmarking.rb @@ -15,8 +15,8 @@ module ActionController #:nodoc: } end - def render_with_benchmark(options = {}, deprecated_status = nil) - if logger.nil? + def render_with_benchmark(options = nil, deprecated_status = nil) + unless logger render_without_benchmark(options, deprecated_status) else db_runtime = ActiveRecord::Base.connection.reset_runtime if Object.const_defined?("ActiveRecord") && ActiveRecord::Base.connected? @@ -35,7 +35,7 @@ module ActionController #:nodoc: end def perform_action_with_benchmark - if logger.nil? + unless logger perform_action_without_benchmark else runtime = [Benchmark::measure{ perform_action_without_benchmark }.real, 0.0001].max diff --git a/actionpack/lib/action_controller/deprecated_redirects.rb b/actionpack/lib/action_controller/deprecated_redirects.rb new file mode 100644 index 0000000000..264ac8de82 --- /dev/null +++ b/actionpack/lib/action_controller/deprecated_redirects.rb @@ -0,0 +1,17 @@ +module ActionController + class Base + protected + # Deprecated in favor of calling redirect_to directly with the path. + def redirect_to_path(path) + redirect_to(path) + end + + # Deprecated in favor of calling redirect_to directly with the url. If the resource has moved permanently, it's possible to pass + # true as the second parameter and the browser will get "301 Moved Permanently" instead of "302 Found". This can also be done through + # just setting the headers["Status"] to "301 Moved Permanently" before using the redirect_to. + def redirect_to_url(url, permanently = false) + headers["Status"] = "301 Moved Permanently" if permanently + redirect_to(url) + end + end +end diff --git a/actionpack/lib/action_controller/deprecated_renders_and_redirects.rb b/actionpack/lib/action_controller/deprecated_renders_and_redirects.rb deleted file mode 100644 index 44715d9cae..0000000000 --- a/actionpack/lib/action_controller/deprecated_renders_and_redirects.rb +++ /dev/null @@ -1,76 +0,0 @@ -module ActionController - class Base - protected - # Works like render, but instead of requiring a full template name, you can get by with specifying the action name. So calling - # render_action "show_many" in WeblogController#display will render "#{template_root}/weblog/show_many.rhtml" or - # "#{template_root}/weblog/show_many.rxml". - def render_action(action_name, status = nil) - render :action => action_name, :status => status - end - - # Works like render, but disregards the template_root and requires a full path to the template that needs to be rendered. Can be - # used like render_file "/Users/david/Code/Ruby/template" to render "/Users/david/Code/Ruby/template.rhtml" or - # "/Users/david/Code/Ruby/template.rxml". - def render_file(template_path, status = nil, use_full_path = false) - render :file => template_path, :status => status, :use_full_path => use_full_path - end - - # Renders the +template+ string, which is useful for rendering short templates you don't want to bother having a file for. So - # you'd call render_template "Hello, <%= @user.name %>" to greet the current user. Or if you want to render as Builder - # template, you could do render_template "xml.h1 @user.name", nil, "rxml". - def render_template(template, status = nil, type = "rhtml") - render :inline => template, :status => status, :type => type - end - - # Renders the +text+ string without parsing it through any template engine. Useful for rendering static information as it's - # considerably faster than rendering through the template engine. - # Use block for response body if provided (useful for deferred rendering or streaming output). - def render_text(text = nil, status = nil) - render :text => text, :status => status - end - - # Renders an empty response that can be used when the request is only interested in triggering an effect. Do note that good - # HTTP manners mandate that you don't use GET requests to trigger data changes. - def render_nothing(status = nil) - render :nothing => true, :status => status - end - - # Renders the partial specified by partial_path, which by default is the name of the action itself. Example: - # - # class WeblogController < ActionController::Base - # def show - # render_partial # renders "weblog/_show.r(xml|html)" - # end - # end - def render_partial(partial_path = default_template_name, object = nil, local_assigns = {}) - render :partial => partial_path, :object => object, :locals => local_assigns - end - - # Renders a collection of partials using partial_name to iterate over the +collection+. - def render_partial_collection(partial_name, collection, partial_spacer_template = nil, local_assigns = {}) - render :partial => partial_name, :collection => collection, :spacer_template => partial_spacer_template, :locals => local_assigns - end - - def render_with_layout(template_name = default_template_name, status = nil, layout = nil) - render :template => template_name, :status => status, :layout => layout - end - - def render_without_layout(template_name = default_template_name, status = nil) - render :template => template_name, :status => status, :layout => false - end - - - # Deprecated in favor of calling redirect_to directly with the path. - def redirect_to_path(path) - redirect_to(path) - end - - # Deprecated in favor of calling redirect_to directly with the url. If the resource has moved permanently, it's possible to pass - # true as the second parameter and the browser will get "301 Moved Permanently" instead of "302 Found". This can also be done through - # just setting the headers["Status"] to "301 Moved Permanently" before using the redirect_to. - def redirect_to_url(url, permanently = false) - headers["Status"] = "301 Moved Permanently" if permanently - redirect_to(url) - end - end -end \ No newline at end of file diff --git a/actionpack/lib/action_controller/layout.rb b/actionpack/lib/action_controller/layout.rb index a6a8d4096f..4247e8a1cf 100644 --- a/actionpack/lib/action_controller/layout.rb +++ b/actionpack/lib/action_controller/layout.rb @@ -194,66 +194,76 @@ module ActionController #:nodoc: # weblog/standard, but layout "standard" will return layouts/standard. def active_layout(passed_layout = nil) layout = passed_layout || self.class.read_inheritable_attribute("layout") + active_layout = case layout when Symbol then send(layout) when Proc then layout.call(self) when String then layout end + active_layout.include?("/") ? active_layout : "layouts/#{active_layout}" if active_layout end - def render_with_a_layout(options = {}, deprecated_status = nil, deprecated_layout = nil) #:nodoc: - options = render_with_a_layout_options(options) - if (layout = pick_layout(options, deprecated_layout)) - logger.info("Rendering #{options[:template]} within #{layout}") unless logger.nil? + def render_with_a_layout(options = nil, deprecated_status = nil, deprecated_layout = nil) #:nodoc: + template_with_options = options.is_a?(Hash) - @content_for_layout = render_with_no_layout(options.merge(:layout => false)) - erase_render_results + if apply_layout?(template_with_options, options) && (layout = pick_layout(template_with_options, options, deprecated_layout)) + logger.info("Rendering #{options} within #{layout}") if logger + + if template_with_options + content_for_layout = render_with_no_layout(options) + deprecated_status = options[:status] || deprecated_status + else + content_for_layout = render_with_no_layout(options, deprecated_status) + end - @assigns["content_for_layout"] = @content_for_layout - render_with_no_layout(options.merge({ :text => @template.render_file(layout, true), :status => options[:status] || deprecated_status })) + erase_render_results + @template.instance_variable_set("@content_for_layout", content_for_layout) + render_text(@template.render_file(layout, true), deprecated_status) else render_with_no_layout(options, deprecated_status) end end private - def render_with_a_layout_options(options) - return { :template => options } unless options.is_a?(Hash) - if options.values_at(:text, :file, :inline, :partial, :nothing).compact.empty? - options + def apply_layout?(template_with_options, options) + if template_with_options + (options.has_key?(:layout) && options[:layout]!=false) || options.values_at(:text, :file, :inline, :partial, :nothing).compact.empty? else - { :layout => false }.merge(options) + true end end - def pick_layout(options = {}, deprecated_layout = nil) - return deprecated_layout if !deprecated_layout.nil? - - if options.is_a?(Hash) - case options[:layout] + def pick_layout(template_with_options, options, deprecated_layout) + if deprecated_layout + deprecated_layout + elsif template_with_options + case layout = options[:layout] when FalseClass nil when NilClass, TrueClass active_layout if action_has_layout? else - active_layout(options[:layout]) + active_layout(layout) end else - (deprecated_layout || active_layout) if action_has_layout? + active_layout if action_has_layout? end end - + def action_has_layout? - conditions = self.class.layout_conditions || {} - case - when conditions[:only] - conditions[:only].include?(action_name) - when conditions[:except] - !conditions[:except].include?(action_name) - else - true + if conditions = self.class.layout_conditions + case + when only = conditions[:only] + only.include?(action_name) + when except = conditions[:except] + !except.include?(action_name) + else + true + end + else + true end end end -end +end \ No newline at end of file diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 304d6bf1ed..7fc0c3b020 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -158,6 +158,7 @@ module ActionView #:nodoc: def initialize(base_path = nil, assigns_for_first_render = {}, controller = nil)#:nodoc: @base_path, @assigns = base_path, assigns_for_first_render + @assigns_added = nil @controller = controller @logger = controller && controller.logger end @@ -280,7 +281,10 @@ module ActionView #:nodoc: end def evaluate_assigns(local_assigns = {}) - @assigns.each { |key, value| instance_variable_set("@#{key}", value) } + unless @assigns_added + @assigns.each { |key, value| instance_variable_set("@#{key}", value) } + @assigns_added = true + end saved_locals = {} local_assigns.each do |key, value| diff --git a/actionpack/lib/action_view/partials.rb b/actionpack/lib/action_view/partials.rb index e078c52262..be9d40ca53 100644 --- a/actionpack/lib/action_view/partials.rb +++ b/actionpack/lib/action_view/partials.rb @@ -46,21 +46,25 @@ module ActionView # This will render the partial "advertisement/_ad.rhtml" regardless of which controller this is being called from. module Partials # Deprecated, use render :partial - def render_partial(partial_path, local_assigns = {}, deprecated_local_assigns = {}) #:nodoc: + def render_partial(partial_path, local_assigns = nil, deprecated_local_assigns = nil) #:nodoc: path, partial_name = partial_pieces(partial_path) object = extracting_object(partial_name, local_assigns, deprecated_local_assigns) local_assigns = extract_local_assigns(local_assigns, deprecated_local_assigns) + local_assigns = local_assigns ? local_assigns.clone : {} add_counter_to_local_assigns!(partial_name, local_assigns) + local_assigns[partial_name] = object - render("#{path}/_#{partial_name}", { partial_name => object }.merge(local_assigns)) + render("#{path}/_#{partial_name}", local_assigns) end # Deprecated, use render :partial, :collection - def render_partial_collection(partial_name, collection, partial_spacer_template = nil, local_assigns = {}) #:nodoc: + def render_partial_collection(partial_name, collection, partial_spacer_template = nil, local_assigns = nil) #:nodoc: collection_of_partials = Array.new counter_name = partial_counter_name(partial_name) + local_assigns = local_assigns ? local_assigns.clone : {} collection.each_with_index do |element, counter| - collection_of_partials.push(render_partial(partial_name, element, { counter_name => counter }.merge(local_assigns))) + local_assigns[counter_name] = counter + collection_of_partials.push(render_partial(partial_name, element, local_assigns)) end return nil if collection_of_partials.empty? -- cgit v1.2.3