mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Got overhead down from 127 to 85. All tests pass:
* Tentatively replaced HeaderHash with SimpleHeaderHash, which does not preserve case but does handle converting Arrays to Strings in to_hash. This requires further discussion. * Moved default_charset to ActionDispatch::Response to avoid having to hop over to ActionController. Ideally, this would be a constant on AD::Response, but some tests expect to be able to change it dynamically and I didn't want to change them yet. * Completely override #initialize from Rack::Response. Previously, it was creating a HeaderHash, and then we were creating an entirely new one. There is no way to call super without incurring the overhead of creating a HeaderHash. * Override #write from Rack::Response. Its implementation tracks Content-Length, and doing so adds additional overhead that could be mooted if other middleware changes the body. It is more efficiently done at the top-level server. * Change sending_file to an instance_variable instead of header inspection. In general, if a state is important, it should be set as a property of the response not reconstructed later. * Set the Etag to @body instead of .body. AS::Cache.expand_cache_key handles Arrays fine, and it's more efficient to let it handle the body parts, since it is not forced to create a joined String. * If we detect the default cache control case, just set it, rather than setting the constituent parts and then running the normal (expensive) code to generate the string.
This commit is contained in:
parent
9e62d6d1c0
commit
0adbeeb0c9
5 changed files with 66 additions and 43 deletions
|
@ -25,8 +25,9 @@ module ActionController
|
|||
cattr_accessor :relative_url_root
|
||||
self.relative_url_root = ENV['RAILS_RELATIVE_URL_ROOT']
|
||||
|
||||
cattr_accessor :default_charset
|
||||
self.default_charset = "utf-8"
|
||||
class << self
|
||||
delegate :default_charset=, :to => "ActionDispatch::Response"
|
||||
end
|
||||
|
||||
# cattr_reader :protected_instance_variables
|
||||
cattr_accessor :protected_instance_variables
|
||||
|
|
|
@ -145,7 +145,6 @@ module ActionController #:nodoc:
|
|||
def send_data(data, options = {}) #:doc:
|
||||
logger.info "Sending data #{options[:filename]}" if logger
|
||||
send_file_headers! options.merge(:length => data.bytesize)
|
||||
@performed_render = false
|
||||
render :status => options[:status], :text => data
|
||||
end
|
||||
|
||||
|
@ -175,6 +174,8 @@ module ActionController #:nodoc:
|
|||
'Content-Transfer-Encoding' => 'binary'
|
||||
)
|
||||
|
||||
response.sending_file = true
|
||||
|
||||
# Fix a problem with IE 6.0 on opening downloaded files:
|
||||
# If Cache-Control: no-cache is set (which Rails does by default),
|
||||
# IE removes the file it just downloaded from its cache immediately
|
||||
|
|
|
@ -52,7 +52,7 @@ module ActionController #:nodoc:
|
|||
class TestResponse < ActionDispatch::TestResponse
|
||||
def recycle!
|
||||
@status = 200
|
||||
@header = Rack::Utils::HeaderHash.new
|
||||
@header = ActionDispatch::Response::SimpleHeaderHash.new
|
||||
@writer = lambda { |x| @body << x }
|
||||
@block = nil
|
||||
@length = 0
|
||||
|
|
|
@ -32,18 +32,42 @@ module ActionDispatch # :nodoc:
|
|||
# end
|
||||
# end
|
||||
class Response < Rack::Response
|
||||
class SimpleHeaderHash < Hash
|
||||
def to_hash
|
||||
result = {}
|
||||
each do |k,v|
|
||||
v = v.join("\n") if v.is_a?(Array)
|
||||
result[k] = v
|
||||
end
|
||||
result
|
||||
end
|
||||
end
|
||||
|
||||
attr_accessor :request
|
||||
attr_reader :cache_control
|
||||
|
||||
attr_writer :header
|
||||
attr_writer :header, :sending_file
|
||||
alias_method :headers=, :header=
|
||||
|
||||
delegate :default_charset, :to => 'ActionController::Base'
|
||||
|
||||
def initialize
|
||||
super
|
||||
@status = 200
|
||||
@header = SimpleHeaderHash.new
|
||||
@cache_control = {}
|
||||
@header = Rack::Utils::HeaderHash.new
|
||||
|
||||
@writer = lambda { |x| @body << x }
|
||||
@block = nil
|
||||
@length = 0
|
||||
|
||||
@body = []
|
||||
@sending_file = false
|
||||
|
||||
yield self if block_given?
|
||||
end
|
||||
|
||||
def write(str)
|
||||
s = str.to_s
|
||||
@writer.call s
|
||||
str
|
||||
end
|
||||
|
||||
def status=(status)
|
||||
|
@ -128,20 +152,20 @@ module ActionDispatch # :nodoc:
|
|||
end
|
||||
end
|
||||
|
||||
def sending_file?
|
||||
headers["Content-Transfer-Encoding"] == "binary"
|
||||
end
|
||||
CONTENT_TYPE = "Content-Type"
|
||||
|
||||
cattr_accessor(:default_charset) { "utf-8" }
|
||||
|
||||
def assign_default_content_type_and_charset!
|
||||
return if !headers["Content-Type"].blank?
|
||||
return if !headers[CONTENT_TYPE].blank?
|
||||
|
||||
@content_type ||= Mime::HTML
|
||||
@charset ||= default_charset
|
||||
@charset ||= self.class.default_charset
|
||||
|
||||
type = @content_type.to_s.dup
|
||||
type << "; charset=#{@charset}" unless sending_file?
|
||||
type << "; charset=#{@charset}" unless @sending_file
|
||||
|
||||
headers["Content-Type"] = type
|
||||
headers[CONTENT_TYPE] = type
|
||||
end
|
||||
|
||||
def prepare!
|
||||
|
@ -168,17 +192,6 @@ module ActionDispatch # :nodoc:
|
|||
str
|
||||
end
|
||||
|
||||
def set_cookie(key, value)
|
||||
if value.has_key?(:http_only)
|
||||
ActiveSupport::Deprecation.warn(
|
||||
"The :http_only option in ActionController::Response#set_cookie " +
|
||||
"has been renamed. Please use :httponly instead.", caller)
|
||||
value[:httponly] ||= value.delete(:http_only)
|
||||
end
|
||||
|
||||
super(key, value)
|
||||
end
|
||||
|
||||
# Returns the response cookies, converted to a Hash of (name => value) pairs
|
||||
#
|
||||
# assert_equal 'AuthorOfNewPage', r.cookies['author']
|
||||
|
@ -201,7 +214,7 @@ module ActionDispatch # :nodoc:
|
|||
if etag? || last_modified? || !cache_control.empty?
|
||||
set_conditional_cache_control!
|
||||
elsif nonempty_ok_response?
|
||||
self.etag = body
|
||||
self.etag = @body
|
||||
|
||||
if request && request.etag_matches?(etag)
|
||||
self.status = 304
|
||||
|
@ -214,30 +227,37 @@ module ActionDispatch # :nodoc:
|
|||
end
|
||||
end
|
||||
|
||||
EMPTY_RESPONSE = [" "]
|
||||
|
||||
def nonempty_ok_response?
|
||||
ok = !@status || @status == 200
|
||||
ok && string_body?
|
||||
ok && string_body? && @body != EMPTY_RESPONSE
|
||||
end
|
||||
|
||||
def string_body?
|
||||
!body_parts.respond_to?(:call) && body_parts.any? && body_parts.all? { |part| part.is_a?(String) }
|
||||
end
|
||||
|
||||
def set_conditional_cache_control!
|
||||
if cache_control.empty?
|
||||
cache_control.merge!(:public => false, :max_age => 0, :must_revalidate => true)
|
||||
end
|
||||
DEFAULT_CACHE_CONTROL = "max-age=0, private, must-revalidate"
|
||||
|
||||
public_cache, max_age, must_revalidate, extras =
|
||||
cache_control.values_at(:public, :max_age, :must_revalidate, :extras)
|
||||
def set_conditional_cache_control!
|
||||
control = cache_control
|
||||
|
||||
if control.empty?
|
||||
headers["Cache-Control"] = DEFAULT_CACHE_CONTROL
|
||||
else
|
||||
extras = control[:extras]
|
||||
max_age = control[:max_age]
|
||||
|
||||
options = []
|
||||
options << "max-age=#{max_age}" if max_age
|
||||
options << (public_cache ? "public" : "private")
|
||||
options << "must-revalidate" if must_revalidate
|
||||
options << (control[:public] ? "public" : "private")
|
||||
options << "must-revalidate" if control[:must_revalidate]
|
||||
options.concat(extras) if extras
|
||||
|
||||
headers["Cache-Control"] = options.join(", ")
|
||||
end
|
||||
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -46,11 +46,12 @@ class Class
|
|||
end # end
|
||||
" unless options[:instance_writer] == false } # # instance writer above is generated unless options[:instance_writer] == false
|
||||
EOS
|
||||
self.send("#{sym}=", yield) if block_given?
|
||||
end
|
||||
end
|
||||
|
||||
def cattr_accessor(*syms)
|
||||
def cattr_accessor(*syms, &blk)
|
||||
cattr_reader(*syms)
|
||||
cattr_writer(*syms)
|
||||
cattr_writer(*syms, &blk)
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in a new issue