Permalink
Browse files

Backport 5c51cd0: #send_file leans on Rack::Sendfile to X-Accel-Redir…

…ect the file's path, so opening the file to set the response body is wasteful. Set a FileBody wrapper instead that responds to to_path and streams the file if needed.
  • Loading branch information...
1 parent 786713a commit bccc35b13e4782bde4a8def9b52c21f1eda0b4b1 @jeremy jeremy committed with Erich Menge Apr 12, 2012
Showing with 28 additions and 2 deletions.
  1. +6 −0 actionpack/CHANGELOG.md
  2. +22 −2 actionpack/lib/action_controller/metal/data_streaming.rb
@@ -7,6 +7,12 @@
*Ravil Bayramgalin*
+* Performance Improvement to send_file: Avoid having to pass an open file handle as the response body. Rack::Sendfile
+ will usually intercept the response and just uses the path directly, so no reason to open the file. This performance
+ improvement also resolves an issue with jRuby encodings, and is the reason for the backport, see issue #6844.
+
+ *Jeremy Kemper & Erich Menge*
+
## Rails 3.2.8 (Aug 9, 2012) ##
@@ -75,7 +75,27 @@ def send_file(path, options = {}) #:doc:
self.status = options[:status] || 200
self.content_type = options[:content_type] if options.key?(:content_type)
- self.response_body = File.open(path, "rb")
+ self.response_body = FileBody.new(path)
+ end
+
+ # Avoid having to pass an open file handle as the response body.
+ # Rack::Sendfile will usually intercept the response and uses
+ # the path directly, so there is no reason to open the file.
+ class FileBody #:nodoc:
+ attr_reader :to_path
+
+ def initialize(path)
+ @to_path = path
+ end
+
+ # Stream the file's contents if Rack::Sendfile isn't present.
+ def each
+ File.open(to_path, 'rb') do |file|
+ while chunk = file.read(16384)
+ yield chunk
+ end
+ end
+ end
end
# Sends the given binary data to the browser. This method is similar to
@@ -115,7 +135,7 @@ def send_data(data, options = {}) #:doc:
private
def send_file_headers!(options)
type_provided = options.has_key?(:type)
-
+
options.update(DEFAULT_SEND_FILE_OPTIONS.merge(options))
[:type, :disposition].each do |arg|
raise ArgumentError, ":#{arg} option required" if options[arg].nil?

0 comments on commit bccc35b

Please sign in to comment.