From 0adbeeb0c92c6de2e4a148e4b54d56cd4a325800 Mon Sep 17 00:00:00 2001 From: Yehuda Katz Date: Mon, 10 Aug 2009 11:40:41 -0400 Subject: 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. --- actionpack/lib/action_controller/metal/compatibility.rb | 5 +++-- actionpack/lib/action_controller/metal/streaming.rb | 3 ++- actionpack/lib/action_controller/testing/process.rb | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) (limited to 'actionpack/lib/action_controller') diff --git a/actionpack/lib/action_controller/metal/compatibility.rb b/actionpack/lib/action_controller/metal/compatibility.rb index f94d1c669c..a008b53d45 100644 --- a/actionpack/lib/action_controller/metal/compatibility.rb +++ b/actionpack/lib/action_controller/metal/compatibility.rb @@ -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 diff --git a/actionpack/lib/action_controller/metal/streaming.rb b/actionpack/lib/action_controller/metal/streaming.rb index 57318e8747..4761763a26 100644 --- a/actionpack/lib/action_controller/metal/streaming.rb +++ b/actionpack/lib/action_controller/metal/streaming.rb @@ -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 diff --git a/actionpack/lib/action_controller/testing/process.rb b/actionpack/lib/action_controller/testing/process.rb index d32d5562e8..147a7e7631 100644 --- a/actionpack/lib/action_controller/testing/process.rb +++ b/actionpack/lib/action_controller/testing/process.rb @@ -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 -- cgit v1.2.3 From 4bf516e072f5279bdb462c6592e17b195fd9cf05 Mon Sep 17 00:00:00 2001 From: Yehuda Katz Date: Mon, 10 Aug 2009 15:49:33 -0700 Subject: More perf work: * Move #set_cookie and #delete_cookie inline to optimize. These optimizations should almost certainly be sent back upstream to Rack. The optimization involves using an ivar for cookies instead of indexing into the headers each time. * Was able to use a bare Hash for headers now that cookies have their own joining semantics (some code assumed that the raw cookies were an Array). * Cache blankness of body on body= * Improve expand_cache_key for Arrays of a single element (common in our case) * Use a simple layout condition check unless conditions are used * Cache visible actions * Lazily load the UrlRewriter * Make etag an ivar that is set on prepare! --- actionpack/lib/action_controller/metal/compatibility.rb | 7 +++---- actionpack/lib/action_controller/metal/hide_actions.rb | 14 +++++++++++++- actionpack/lib/action_controller/metal/url_for.rb | 10 +--------- actionpack/lib/action_controller/testing/process.rb | 2 +- actionpack/lib/action_controller/testing/test_case.rb | 1 - 5 files changed, 18 insertions(+), 16 deletions(-) (limited to 'actionpack/lib/action_controller') diff --git a/actionpack/lib/action_controller/metal/compatibility.rb b/actionpack/lib/action_controller/metal/compatibility.rb index a008b53d45..5b0165f0e7 100644 --- a/actionpack/lib/action_controller/metal/compatibility.rb +++ b/actionpack/lib/action_controller/metal/compatibility.rb @@ -102,11 +102,10 @@ module ActionController options[:template].sub!(/^\//, '') end - options[:text] = nil if options[:nothing] == true + options[:text] = nil if options.delete(:nothing) == true + options[:text] = " " if options.key?(:text) && options[:text].nil? - body = super - body = [' '] if body.blank? - body + super || " " end def _handle_method_missing diff --git a/actionpack/lib/action_controller/metal/hide_actions.rb b/actionpack/lib/action_controller/metal/hide_actions.rb index af68c772b1..cdacdc40a6 100644 --- a/actionpack/lib/action_controller/metal/hide_actions.rb +++ b/actionpack/lib/action_controller/metal/hide_actions.rb @@ -13,7 +13,9 @@ module ActionController # Overrides AbstractController::Base#action_method? to return false if the # action name is in the list of hidden actions. def action_method?(action_name) - !hidden_actions.include?(action_name) && super + self.class.visible_action?(action_name) do + !hidden_actions.include?(action_name) && super + end end module ClassMethods @@ -25,6 +27,16 @@ module ActionController hidden_actions.merge(args.map! {|a| a.to_s }) end + def inherited(klass) + klass.instance_variable_set("@visible_actions", {}) + super + end + + def visible_action?(action_name) + return @visible_actions[action_name] if @visible_actions.key?(action_name) + @visible_actions[action_name] = yield + end + # Overrides AbstractController::Base#action_methods to remove any methods # that are listed as hidden methods. def action_methods diff --git a/actionpack/lib/action_controller/metal/url_for.rb b/actionpack/lib/action_controller/metal/url_for.rb index 7119c14cd3..14c6523045 100644 --- a/actionpack/lib/action_controller/metal/url_for.rb +++ b/actionpack/lib/action_controller/metal/url_for.rb @@ -4,15 +4,6 @@ module ActionController include RackConvenience - def process_action(*) - initialize_current_url - super - end - - def initialize_current_url - @url = UrlRewriter.new(request, params.clone) - end - # Overwrite to implement a number of default options that all url_for-based methods will use. The default options should come in # the form of a hash, just like the one you would use for url_for directly. Example: # @@ -40,6 +31,7 @@ module ActionController when String options when Hash + @url ||= UrlRewriter.new(request, params) @url.rewrite(rewrite_options(options)) else polymorphic_url(options) diff --git a/actionpack/lib/action_controller/testing/process.rb b/actionpack/lib/action_controller/testing/process.rb index 147a7e7631..09b1a59254 100644 --- a/actionpack/lib/action_controller/testing/process.rb +++ b/actionpack/lib/action_controller/testing/process.rb @@ -52,7 +52,7 @@ module ActionController #:nodoc: class TestResponse < ActionDispatch::TestResponse def recycle! @status = 200 - @header = ActionDispatch::Response::SimpleHeaderHash.new + @header = {} @writer = lambda { |x| @body << x } @block = nil @length = 0 diff --git a/actionpack/lib/action_controller/testing/test_case.rb b/actionpack/lib/action_controller/testing/test_case.rb index a11755b517..b66a4c15ff 100644 --- a/actionpack/lib/action_controller/testing/test_case.rb +++ b/actionpack/lib/action_controller/testing/test_case.rb @@ -179,7 +179,6 @@ module ActionController if @controller @controller.request = @request @controller.params = {} - @controller.send(:initialize_current_url) end end -- cgit v1.2.3