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
This commit is contained in:
David Heinemeier Hansson
2005-07-24 16:45:39 +00:00
parent 3ccea931fa
commit 4f40b2d8fb
15 changed files with 186 additions and 198 deletions

View File

@@ -1,6 +1,8 @@
*SVN*
* Improved performance with 5-30% through a series of Action Pack optimizations #1811 [Stefan Kaes]
* Improved performance of test app req/sec with ~10% refactoring the render method #1823 [Stefan Kaes]
* Improved performance of test app req/sec with 5-30% through a series of Action Pack optimizations #1811 [Stefan Kaes]
* Changed caching/expiration/hit to report using the DEBUG log level and errors to use the ERROR log level instead of both using INFO

View File

@@ -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'

View File

@@ -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 <tt>render</tt>, 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

View File

@@ -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

View File

@@ -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

View File

@@ -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
# <tt>render_action "show_many"</tt> 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 <tt>render_file "/Users/david/Code/Ruby/template"</tt> 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 <tt>render_template "Hello, <%= @user.name %>"</tt> to greet the current user. Or if you want to render as Builder
# template, you could do <tt>render_template "xml.h1 @user.name", nil, "rxml"</tt>.
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 <tt>partial_path</tt>, 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 <tt>partial_name</tt> 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

View File

@@ -194,66 +194,76 @@ module ActionController #:nodoc:
# 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")
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)
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
@content_for_layout = render_with_no_layout(options.merge(:layout => false))
erase_render_results
@assigns["content_for_layout"] = @content_for_layout
render_with_no_layout(options.merge({ :text => @template.render_file(layout, true), :status => options[:status] || deprecated_status }))
@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

View File

@@ -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|

View File

@@ -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?

View File

@@ -10,3 +10,4 @@ require 'action_controller/test_process'
ActionController::Base.logger = nil
ActionController::Base.ignore_missing_templates = false
ActionController::Routing::Routes.reload rescue nil

View File

@@ -48,7 +48,7 @@ class ActionPackAssertionsController < ActionController::Base
# assign some template instance variables
def assign_this
@howdy = "ho"
render_text "Mr. Henke"
render :inline => "Mr. Henke"
end
def render_based_on_parameters

View File

@@ -47,7 +47,7 @@ class ActiveRecordAssertionsController < ActionController::Base
@company = Company.new
@company.name = "B"
@company.rating = 2
render_text "snicker...."
render :inline => "snicker...."
end
# fail with 2 bad columns
@@ -55,7 +55,7 @@ class ActiveRecordAssertionsController < ActionController::Base
@company = Company.new
@company.name = ""
@company.rating = 2
render_text "double snicker...."
render :inline => "double snicker...."
end
# this will pass validation
@@ -63,13 +63,13 @@ class ActiveRecordAssertionsController < ActionController::Base
@company = Company.new
@company.name = "A"
@company.rating = 69
render_text "Goodness Gracious!"
render :inline => "Goodness Gracious!"
end
# this will fail validation
def bad_company
@company = Company.new
render_text "Who's Bad?"
render :inline => "Who's Bad?"
end
# the safety dance......

View File

@@ -5,7 +5,7 @@ class FilterTest < Test::Unit::TestCase
before_filter :ensure_login
def show
render_text "ran action"
render :inline => "ran action"
end
private
@@ -20,26 +20,26 @@ class FilterTest < Test::Unit::TestCase
def show
@ran_action = true
render_text "ran action"
render :inline => "ran action"
end
private
def render_something_else
render_text "something else"
render :inline => "something else"
end
end
class ConditionalFilterController < ActionController::Base
def show
render_text "ran action"
render :inline => "ran action"
end
def another_action
render_text "ran action"
render :inline => "ran action"
end
def show_without_filter
render_text "ran action without filter"
render :inline => "ran action without filter"
end
private

View File

@@ -4,32 +4,32 @@ class FlashTest < Test::Unit::TestCase
class TestController < ActionController::Base
def set_flash
flash["that"] = "hello"
render_text "hello"
render :inline => "hello"
end
def set_flash_now
flash.now["that"] = "hello"
@flash_copy = {}.update flash
render_text "hello"
render :inline => "hello"
end
def attempt_to_use_flash_now
@flash_copy = {}.update flash
@flashy = flash["that"]
render_text "hello"
render :inline => "hello"
end
def use_flash
@flash_copy = {}.update flash
@flashy = flash["that"]
render_text "hello"
render :inline => "hello"
end
def use_flash_and_keep_it
@flash_copy = {}.update flash
@flashy = flash["that"]
keep_flash
render_text "hello"
render :inline => "hello"
end
def rescue_action(e)
@@ -91,4 +91,4 @@ class FlashTest < Test::Unit::TestCase
def process_request
TestController.process(@request, @response)
end
end
end

View File

@@ -311,8 +311,8 @@ class NewRenderTest < Test::Unit::TestCase
assert_equal "<title>Talking to the layout</title>\nAction was here!", @response.body
end
# def test_partials_list
# get :partials_list
# assert_equal "goodbyeHello: davidHello: marygoodbye\n", @response.body
# end
def test_partials_list
get :partials_list
assert_equal "goodbyeHello: davidHello: marygoodbye\n", @response.body
end
end