Skip to content
Browse files

Improved performance with 5-30% through a series of Action Pack optim…

…izations #1811 [Stefan Kaes]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@1905 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
  • Loading branch information...
1 parent d172592 commit b366dbd952417a610913e05ad58024b7da03fdb8 @dhh dhh committed Jul 23, 2005
View
2 actionpack/CHANGELOG
@@ -1,5 +1,7 @@
*SVN*
+* Improved performance 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
* Added support for per-action session management #1763
View
27 actionpack/lib/action_controller/base.rb
@@ -215,6 +215,10 @@ class Base
@@view_controller_internals = true
cattr_accessor :view_controller_internals
+ # Protected instance variable cache
+ @@protected_variables_cache = nil
+ cattr_accessor :protected_variables_cache
+
# Prepends all the URL-generating helpers from AssetHelper. This makes it possible to easily move javascripts, stylesheets,
# and images to a dedicated asset server away from the main web server. Example:
# ActionController::Base.asset_host = "http://assets.example.com"
@@ -351,12 +355,13 @@ def process(request, response, method = :perform_action, *arguments) #:nodoc:
assign_shortcuts(request, response)
initialize_current_url
@action_name = params[:action] || 'index'
+ @variables_added = nil
- log_processing unless logger.nil?
+ log_processing if logger
send(method, *arguments)
close_session
- return @response
+ @response
end
# Returns a URL that has been rewritten according to the options hash and the defined Routes.
@@ -765,6 +770,8 @@ def assign_shortcuts(request, response)
@template = @response.template
@assigns = @response.template.assigns
@headers = @response.headers
+
+ @variables_added = nil
end
def initialize_current_url
@@ -777,7 +784,7 @@ def log_processing
end
def perform_action
- if action_methods.include?(action_name) || action_methods.include?('method_missing')
+ if self.class.action_methods.include?(action_name) || self.class.action_methods.include?('method_missing')
send(action_name)
render unless performed?
elsif template_exists? && template_public?
@@ -792,17 +799,23 @@ def performed?
end
def action_methods
- @action_methods ||= (self.class.public_instance_methods - self.class.hidden_actions)
+ self.class.action_methods
end
+ def self.action_methods
+ @action_methods ||= (public_instance_methods - hidden_actions).inject({}) { |h, k| h[k] = true; h }
+ end
def add_variables_to_assigns
- add_instance_variables_to_assigns
- add_class_variables_to_assigns if view_controller_internals
+ unless @variables_added
+ add_instance_variables_to_assigns
+ add_class_variables_to_assigns if view_controller_internals
+ @variables_added = true
+ end
end
def add_instance_variables_to_assigns
- @@protected_variables_cache = protected_instance_variables.inject({}) { |h, k| h[k] = true; h }
+ @@protected_variables_cache ||= protected_instance_variables.inject({}) { |h, k| h[k] = true; h }
instance_variables.each do |var|
next if @@protected_variables_cache.include?(var)
@assigns[var[1..-1]] = instance_variable_get(var)
View
33 actionpack/lib/action_controller/cgi_ext/cgi_methods.rb
@@ -12,10 +12,10 @@ def CGIMethods.parse_query_parameters(query_string)
query_string.split(/[&;]/).each { |p|
k, v = p.split('=',2)
- v = nil if (!v.nil? && v.empty?)
+ v = nil if (v && v.empty?)
- k = CGI.unescape(k) unless k.nil?
- v = CGI.unescape(v) unless v.nil?
+ k = CGI.unescape(k) if k
+ v = CGI.unescape(v) if v
keys = split_key(k)
last_key = keys.pop
@@ -27,7 +27,7 @@ def CGIMethods.parse_query_parameters(query_string)
end
}
- return parsed_params
+ parsed_params
end
# Returns the request (POST/GET) parameters in a parsed form where pairs such as "customer[address][street]" /
@@ -38,14 +38,16 @@ def CGIMethods.parse_request_parameters(params)
for key, value in params
value = [value] if key =~ /.*\[\]$/
- CGIMethods.build_deep_hash(
- CGIMethods.get_typed_value(value[0]),
- parsed_params,
- CGIMethods.get_levels(key)
- )
+ unless key.include?('[')
+ # much faster to test for the most common case first (GET)
+ # and avoid the call to build_deep_hash
+ parsed_params[key] = get_typed_value(value[0])
+ else
+ build_deep_hash(get_typed_value(value[0]), parsed_params, get_levels(key))
+ end
end
- return parsed_params
+ parsed_params
end
def self.parse_formatted_request_parameters(format, raw_post_data)
@@ -72,14 +74,17 @@ def CGIMethods.split_key(key)
keys.concat($2[1..-2].split(']['))
keys << '' if key[-2..-1] == '[]' # Have to add it since split will drop empty strings
- return keys
+ keys
else
- return [key]
+ [key]
end
end
def CGIMethods.get_typed_value(value)
- if value.respond_to?(:content_type) && !value.content_type.empty?
+ # test most frequent case first
+ if value.is_a?(String)
+ value
+ elsif value.respond_to?(:content_type) && !value.content_type.empty?
# Uploaded file
value
elsif value.respond_to?(:read)
@@ -88,7 +93,7 @@ def CGIMethods.get_typed_value(value)
elsif value.class == Array
value.collect { |v| CGIMethods.get_typed_value(v) }
else
- # Standard value (not a multipart request)
+ # other value (neither string nor a multipart request)
value.to_s
end
end
View
60 actionpack/lib/action_controller/cgi_ext/cookie_performance_fix.rb
@@ -23,28 +23,32 @@ class Cookie < DelegateClass(Array)
# servers.
#
# These keywords correspond to attributes of the cookie object.
- def initialize(name = "", *value)
- options = if name.kind_of?(String)
- { "name" => name, "value" => value }
- else
- name
- end
- unless options.has_key?("name")
+ def initialize(name = '', *value)
+ if name.kind_of?(String)
+ @name = name
+ @value = Array(value)
+ @domain = nil
+ @expires = nil
+ @secure = false
+ @path = nil
+ else
+ @name = name['name']
+ @value = Array(name['value'])
+ @domain = name['domain']
+ @expires = name['expires']
+ @secure = name['secure'] || false
+ @path = name['path']
+ end
+
+ unless @name
raise ArgumentError, "`name' required"
end
- @name = options["name"]
- @value = Array(options["value"])
# simple support for IE
- if options["path"]
- @path = options["path"]
- else
- %r|^(.*/)|.match(ENV["SCRIPT_NAME"])
- @path = ($1 or "")
+ unless @path
+ %r|^(.*/)|.match(ENV['SCRIPT_NAME'])
+ @path = ($1 or '')
end
- @domain = options["domain"]
- @expires = options["expires"]
- @secure = options["secure"] == true ? true : false
super(@value)
end
@@ -102,20 +106,20 @@ def to_s
#
def self.parse(raw_cookie)
cookies = Hash.new([])
- return cookies unless raw_cookie
-
- raw_cookie.split(/; /).each do |pairs|
- name, values = pairs.split('=',2)
- next unless name and values
- name = CGI::unescape(name)
- values ||= ""
- values = values.split('&').collect{|v| CGI::unescape(v) }
- unless cookies.has_key?(name)
- cookies[name] = new({ "name" => name, "value" => values })
+
+ if raw_cookie
+ raw_cookie.split(/; /).each do |pairs|
+ name, values = pairs.split('=',2)
+ next unless name and values
+ name = CGI::unescape(name)
+ values = values.split('&').collect!{|v| CGI::unescape(v) }
+ unless cookies.has_key?(name)
+ cookies[name] = new(name, *values)
+ end
end
end
cookies
end
end # class Cookie
-end
+end
View
51 actionpack/lib/action_controller/cgi_ext/raw_post_data_fix.rb
@@ -6,14 +6,21 @@ module QueryExtension
# Handles multipart forms (in particular, forms that involve file uploads).
# Reads query parameters in the @params field, and cookies into @cookies.
def initialize_query()
- @cookies = CGI::Cookie::parse((env_table['HTTP_COOKIE'] || env_table['COOKIE']))
+ @cookies = CGI::Cookie::parse(env_table['HTTP_COOKIE'] || env_table['COOKIE'])
- if boundary = multipart_form_boundary
+ #fix some strange request environments
+ if method = env_table['REQUEST_METHOD']
+ method = method.to_s.downcase.intern
+ else
+ method = :get
+ end
+
+ if method == :post && (boundary = multipart_form_boundary)
@multipart = true
@params = read_multipart(boundary, Integer(env_table['CONTENT_LENGTH']))
else
@multipart = false
- @params = CGI::parse(read_query_params || "")
+ @params = CGI::parse(read_query_params(method) || "")
end
end
@@ -22,21 +29,23 @@ def initialize_query()
MULTIPART_FORM_BOUNDARY_RE = %r|\Amultipart/form-data.*boundary=\"?([^\";,]+)\"?|n #"
end
- def multipart_form_boundary
- if env_table['REQUEST_METHOD'] == 'POST'
- MULTIPART_FORM_BOUNDARY_RE.match(env_table['CONTENT_TYPE']).to_a.pop
- end
+ def multipart_form_boundary
+ MULTIPART_FORM_BOUNDARY_RE.match(env_table['CONTENT_TYPE']).to_a.pop
end
- def read_params_from_query
- if defined? MOD_RUBY
+ if defined? MOD_RUBY
+ def read_params_from_query
Apache::request.args || ''
- else
- # fixes CGI querystring parsing for POSTs
- if env_table['QUERY_STRING'].blank? && !env_table['REQUEST_URI'].blank?
- env_table['QUERY_STRING'] = env_table['REQUEST_URI'].split('?', 2)[1] || ''
+ end
+ else
+ def read_params_from_query
+ # fixes CGI querystring parsing for lighttpd
+ env_qs = env_table['QUERY_STRING']
+ if env_qs.blank? && !(uri=env_table['REQUEST_URI']).blank?
+ env_qs.replace(uri.split('?', 2)[1] || '')
+ else
+ env_qs
end
- env_table['QUERY_STRING']
end
end
@@ -46,13 +55,15 @@ def read_params_from_post
env_table['RAW_POST_DATA'] = content.split("&_").first.to_s.freeze # &_ is a fix for Safari Ajax postings that always append \000
end
- def read_query_params
- case env_table['REQUEST_METHOD'].to_s.upcase
- when 'CMD'
- read_from_cmdline
- when 'POST', 'PUT'
+ def read_query_params(method)
+ case method
+ when :get
+ read_params_from_query
+ when :post, :put
read_params_from_post
- else # when 'GET', 'HEAD', 'DELETE', 'OPTIONS'
+ when :cmd
+ read_from_cmdline
+ else # when :head, :delete, :options
read_params_from_query
end
end
View
18 actionpack/lib/action_controller/cgi_process.rb
@@ -52,19 +52,19 @@ def initialize(cgi, session_options = {})
end
def query_string
- return @cgi.query_string unless @cgi.query_string.nil? || @cgi.query_string.empty?
- unless env['REQUEST_URI'].nil?
- parts = env['REQUEST_URI'].split('?')
+ if (qs = @cgi.query_string) && !qs.empty?
+ qs
+ elsif uri = env['REQUEST_URI']
+ parts = uri.split('?')
+ parts.shift
+ parts.join('?')
else
- return env['QUERY_STRING'] || ''
- end
- parts.shift
- return parts.join('?')
+ env['QUERY_STRING'] || ''
+ end
end
def query_parameters
- qs = self.query_string
- qs.empty? ? {} : CGIMethods.parse_query_parameters(query_string)
+ (qs = self.query_string).empty? ? {} : CGIMethods.parse_query_parameters(qs)
end
def request_parameters
View
16 actionpack/lib/action_controller/filters.rb
@@ -157,7 +157,7 @@ def append_before_filter(*filters, &block)
conditions = extract_conditions!(filters)
filters << block if block_given?
add_action_conditions(filters, conditions)
- append_filter_to_chain("before", filters)
+ append_filter_to_chain('before', filters)
end
# The passed <tt>filters</tt> will be prepended to the array of filters that's run _before_ actions
@@ -166,7 +166,7 @@ def prepend_before_filter(*filters, &block)
conditions = extract_conditions!(filters)
filters << block if block_given?
add_action_conditions(filters, conditions)
- prepend_filter_to_chain("before", filters)
+ prepend_filter_to_chain('before', filters)
end
# Short-hand for append_before_filter since that's the most common of the two.
@@ -178,7 +178,7 @@ def append_after_filter(*filters, &block)
conditions = extract_conditions!(filters)
filters << block if block_given?
add_action_conditions(filters, conditions)
- append_filter_to_chain("after", filters)
+ append_filter_to_chain('after', filters)
end
# The passed <tt>filters</tt> will be prepended to the array of filters that's run _after_ actions
@@ -272,8 +272,8 @@ def extract_conditions!(filters)
def add_action_conditions(filters, conditions)
return unless conditions
included, excluded = conditions[:only], conditions[:except]
- write_inheritable_hash("included_actions", condition_hash(filters, included)) && return if included
- write_inheritable_hash("excluded_actions", condition_hash(filters, excluded)) if excluded
+ write_inheritable_hash('included_actions', condition_hash(filters, included)) && return if included
+ write_inheritable_hash('excluded_actions', condition_hash(filters, excluded)) if excluded
end
def condition_hash(filters, *actions)
@@ -322,7 +322,7 @@ def call_filters(filters)
else
raise(
ActionControllerError,
- "Filters need to be either a symbol, proc/method, or class implementing a static filter method"
+ 'Filters need to be either a symbol, proc/method, or class implementing a static filter method'
)
end
@@ -334,11 +334,11 @@ def call_filters(filters)
end
def filter_block?(filter)
- filter.respond_to?("call") && (filter.arity == 1 || filter.arity == -1)
+ filter.respond_to?('call') && (filter.arity == 1 || filter.arity == -1)
end
def filter_class?(filter)
- filter.respond_to?("filter")
+ filter.respond_to?('filter')
end
def action_exempted?(filter)
View
2 actionpack/lib/action_controller/layout.rb
@@ -210,7 +210,7 @@ def render_with_a_layout(options = {}, deprecated_status = nil, deprecated_layou
@content_for_layout = render_with_no_layout(options.merge(:layout => false))
erase_render_results
- add_variables_to_assigns
+ @assigns["content_for_layout"] = @content_for_layout
render_with_no_layout(options.merge({ :text => @template.render_file(layout, true), :status => options[:status] || deprecated_status }))
else
render_with_no_layout(options, deprecated_status)
View
49 actionpack/lib/action_controller/request.rb
@@ -51,30 +51,31 @@ def head?
# For backward compatibility, the post format is extracted from the
# X-Post-Data-Format HTTP header if present.
def post_format
- if env['HTTP_X_POST_DATA_FORMAT']
- env['HTTP_X_POST_DATA_FORMAT'].downcase.to_sym
- else
- case env['CONTENT_TYPE'].to_s.downcase
- when 'application/xml', 'text/xml' then :xml
- when 'application/x-yaml', 'text/x-yaml' then :yaml
- else :url_encoded
- end
- end
+ @post_format ||=
+ if env['HTTP_X_POST_DATA_FORMAT']
+ env['HTTP_X_POST_DATA_FORMAT'].downcase.to_sym
+ else
+ case env['CONTENT_TYPE'].to_s.downcase
+ when 'application/xml', 'text/xml' then :xml
+ when 'application/x-yaml', 'text/x-yaml' then :yaml
+ else :url_encoded
+ end
+ end
end
# Is this a POST request formatted as XML or YAML?
def formatted_post?
- [ :xml, :yaml ].include?(post_format) && post?
+ post? && (post_format == :xml || post_format == :yaml)
end
# Is this a POST request formatted as XML?
def xml_post?
- post_format == :xml && post?
+ post? && post_format == :xml
end
# Is this a POST request formatted as YAML?
def yaml_post?
- post_format == :yaml && post?
+ post? && post_format == :yaml
end
# Returns true if the request's "X-Requested-With" header contains
@@ -102,7 +103,7 @@ def remote_ip
return remote_ips.first.strip unless remote_ips.empty?
end
- return env['REMOTE_ADDR']
+ env['REMOTE_ADDR']
end
# Returns the domain part of a host, such as rubyonrails.org in "www.rubyonrails.org". You can specify
@@ -127,25 +128,27 @@ def raw_post
end
def request_uri
- unless env['REQUEST_URI'].nil?
- (%r{^\w+\://[^/]+(/.*|$)$} =~ env['REQUEST_URI']) ? $1 : env['REQUEST_URI'] # Remove domain, which webrick puts into the request_uri.
+ if uri = env['REQUEST_URI']
+ (%r{^\w+\://[^/]+(/.*|$)$} =~ uri) ? $1 : uri # Remove domain, which webrick puts into the request_uri.
else # REQUEST_URI is blank under IIS - get this from PATH_INFO and SCRIPT_NAME
- script_filename = env["SCRIPT_NAME"].to_s.match(%r{[^/]+$})
- request_uri = env["PATH_INFO"]
- request_uri.sub!(/#{script_filename}\//, '') unless script_filename.nil?
- request_uri += '?' + env["QUERY_STRING"] unless env["QUERY_STRING"].nil? || env["QUERY_STRING"].empty?
- return request_uri
+ script_filename = env['SCRIPT_NAME'].to_s.match(%r{[^/]+$})
+ uri = env['PATH_INFO']
+ uri = uri.sub(/#{script_filename}\//, '') unless script_filename.nil?
+ unless (env_qs = env['QUERY_STRING']).nil? || env_qs.empty?
+ uri << '?' << env_qs
+ end
+ uri
end
end
# Return 'https://' if this is an SSL request and 'http://' otherwise.
def protocol
- env["HTTPS"] == "on" ? 'https://' : 'http://'
+ ssl? ? 'https://' : 'http://'
end
# Is this an SSL request?
def ssl?
- protocol == 'https://'
+ env['HTTPS'] == 'on'
end
# Returns the interpreted path to requested resource after all the installation directory of this application was taken into account
@@ -168,7 +171,7 @@ def relative_url_root
# Returns the port number of this request as an integer.
def port
- env['SERVER_PORT'].to_i
+ @port_as_int ||= env['SERVER_PORT'].to_i
end
# Returns a port suffix like ":8080" if the port number of this request
View
1 actionpack/lib/action_controller/test_process.rb
@@ -35,6 +35,7 @@ def reset_session
def port=(number)
@env["SERVER_PORT"] = number.to_i
+ @port_as_int = nil
end
def action=(action_name)
View
13 actionpack/lib/action_controller/url_rewriter.rb
@@ -20,8 +20,10 @@ def to_str
private
def rewrite_url(path, options)
rewritten_url = ""
- rewritten_url << (options[:protocol] || @request.protocol) unless options[:only_path]
- rewritten_url << (options[:host] || @request.host_with_port) unless options[:only_path]
+ unless options[:only_path]
+ rewritten_url << (options[:protocol] || @request.protocol)
+ rewritten_url << (options[:host] || @request.host_with_port)
+ end
rewritten_url << @request.relative_url_root.to_s unless options[:skip_relative_url_root]
rewritten_url << path
@@ -53,8 +55,11 @@ def build_query_string(hash, only_keys = nil)
only_keys.each do |key|
value = hash[key]
key = CGI.escape key.to_s
- key << '[]' if value.class == Array
- value = [ value ] unless value.class == Array
+ if value.class == Array
+ key << '[]'
+ else
+ value = [ value ]
+ end
value.each { |val| elements << "#{key}=#{Routing.extract_parameter_value(val)}" }
end
View
9 actionpack/test/controller/new_render_test.rb
@@ -92,7 +92,7 @@ def accessing_params_in_template
end
def accessing_params_in_template_with_layout
- render :inline => "Hello: <%= params[:name] %>", :layout => nil
+ render :layout => nil, :inline => "Hello: <%= params[:name] %>"
end
def render_with_explicit_template
@@ -201,15 +201,22 @@ def test_private_methods
end
def test_access_to_request_in_view
+ view_internals_old_value = ActionController::Base.view_controller_internals
+
ActionController::Base.view_controller_internals = false
+ ActionController::Base.protected_variables_cache = nil
get :hello_world
assert_nil(assigns["request"])
ActionController::Base.view_controller_internals = true
+ ActionController::Base.protected_variables_cache = nil
get :hello_world
assert_kind_of ActionController::AbstractRequest, assigns["request"]
+
+ ActionController::Base.view_controller_internals = view_internals_old_value
+ ActionController::Base.protected_variables_cache = nil
end
def test_render_xml
View
2 actionpack/test/controller/redirect_test.rb
@@ -97,4 +97,4 @@ def test_module_redirect_using_options
assert_redirected_to :controller => 'redirect', :action => "hello_world"
end
end
-end
+end
View
9 actionpack/test/controller/render_test.rb
@@ -140,17 +140,24 @@ def test_private_methods
end
def test_access_to_request_in_view
+ view_internals_old_value = ActionController::Base.view_controller_internals
+
ActionController::Base.view_controller_internals = false
+ ActionController::Base.protected_variables_cache = nil
@request.action = "hello_world"
response = process_request
assert_nil response.template.assigns["request"]
ActionController::Base.view_controller_internals = true
+ ActionController::Base.protected_variables_cache = nil
@request.action = "hello_world"
response = process_request
- assert_kind_of ActionController::AbstractRequest, response.template.assigns["request"]
+ assert_kind_of ActionController::AbstractRequest, response.template.assigns["request"]
+
+ ActionController::Base.view_controller_internals = view_internals_old_value
+ ActionController::Base.protected_variables_cache = nil
end
def test_render_xml

0 comments on commit b366dbd

Please sign in to comment.
Something went wrong with that request. Please try again.