Skip to content

Fewer allocated objects on each request #737

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rkh merged 1 commit into from
Oct 2, 2014
Merged

Fewer allocated objects on each request #737

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions lib/rack.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,16 @@ def self.version
def self.release
"1.5"
end
PATH_INFO = 'PATH_INFO'.freeze
REQUEST_METHOD = 'REQUEST_METHOD'.freeze
SCRIPT_NAME = 'SCRIPT_NAME'.freeze
QUERY_STRING = 'QUERY_STRING'.freeze
CACHE_CONTROL = 'Cache-Control'.freeze
CONTENT_LENGTH = 'Content-Length'.freeze
CONTENT_TYPE = 'Content-Type'.freeze

GET = 'GET'.freeze
HEAD = 'HEAD'.freeze

autoload :Builder, "rack/builder"
autoload :BodyProxy, "rack/body_proxy"
Expand Down
8 changes: 4 additions & 4 deletions lib/rack/auth/abstract/handler.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,17 @@ def initialize(app, realm=nil, &authenticator)

def unauthorized(www_authenticate = challenge)
return [ 401,
{ 'Content-Type' => 'text/plain',
'Content-Length' => '0',
{ CONTENT_TYPE => 'text/plain',
CONTENT_LENGTH => '0',
'WWW-Authenticate' => www_authenticate.to_s },
[]
]
end

def bad_request
return [ 400,
{ 'Content-Type' => 'text/plain',
'Content-Length' => '0' },
{ CONTENT_TYPE => 'text/plain',
CONTENT_LENGTH => '0' },
[]
]
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rack/auth/digest/request.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ module Auth
module Digest
class Request < Auth::AbstractRequest
def method
@env['rack.methodoverride.original_method'] || @env['REQUEST_METHOD']
@env['rack.methodoverride.original_method'] || @env[REQUEST_METHOD]
end

def digest?
Expand Down
11 changes: 8 additions & 3 deletions lib/rack/body_proxy.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,14 @@ def initialize(body, &block)
@body, @block, @closed = body, block, false
end

def respond_to?(*args)
return false if args.first.to_s =~ /^to_ary$/
super or @body.respond_to?(*args)
def respond_to?(method_name)

@raggi raggi Oct 2, 2014

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In 1.9, respond_to takes 2 arguments.

@schneems schneems Oct 2, 2014

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right you are: http://ruby-doc.org/core-2.1.3/Object.html#method-i-respond_to-3F Looks like 1.8.7 & 2+ also take multiple arguments. I did this because the *args causes us to have an extra array allocation. If we match the method signature respond_to?(string, include_all=false) then the include_all would be an extra object. If I remember this only decreased the overall count by around 100. I'm going to open a new PR to revert these changes, I don't think we can do better than the previous case and still maintain compatibility.

@schneems schneems Oct 2, 2014

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix is here: #739 sorry about that, should have known better. Thanks for the catch!

@raggi raggi Oct 2, 2014

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, but false.object_id == 0, include_all in your example only takes a reference slot, not an RObject structure.

@schneems schneems Oct 3, 2014

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in #742 also can get rid of a string allocation in method_missing

case method_name
when :to_ary
return false
when String
return false if /^to_ary$/ =~ method_name
end
super or @body.respond_to?(method_name)
end

def close
Expand Down
2 changes: 1 addition & 1 deletion lib/rack/cascade.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ module Rack
# status codes).

class Cascade
NotFound = [404, {"Content-Type" => "text/plain"}, []]
NotFound = [404, {CONTENT_TYPE => "text/plain"}, []]

attr_reader :apps

Expand Down
4 changes: 2 additions & 2 deletions lib/rack/chunked.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,11 @@ def call(env)

if ! chunkable_version?(env['HTTP_VERSION']) ||
STATUS_WITH_NO_ENTITY_BODY.include?(status) ||
headers['Content-Length'] ||
headers[CONTENT_LENGTH] ||
headers['Transfer-Encoding']
[status, headers, body]
else
headers.delete('Content-Length')
headers.delete(CONTENT_LENGTH)
headers['Transfer-Encoding'] = 'chunked'
[status, headers, Body.new(body)]
end
Expand Down
8 changes: 4 additions & 4 deletions lib/rack/commonlogger.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ def log(env, status, header, began_at)
env['HTTP_X_FORWARDED_FOR'] || env["REMOTE_ADDR"] || "-",
env["REMOTE_USER"] || "-",
now.strftime("%d/%b/%Y:%H:%M:%S %z"),
env["REQUEST_METHOD"],
env["PATH_INFO"],
env["QUERY_STRING"].empty? ? "" : "?"+env["QUERY_STRING"],
env[REQUEST_METHOD],
env[PATH_INFO],
env[QUERY_STRING].empty? ? "" : "?"+env[QUERY_STRING],
env["HTTP_VERSION"],
status.to_s[0..3],
length,
Expand All @@ -65,7 +65,7 @@ def log(env, status, header, began_at)
end

def extract_content_length(headers)
value = headers['Content-Length'] or return '-'
value = headers[CONTENT_LENGTH] or return '-'
value.to_s == '0' ? '-' : value
end
end
Expand Down
8 changes: 4 additions & 4 deletions lib/rack/conditionalget.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@ def initialize(app)
end

def call(env)
case env['REQUEST_METHOD']
when "GET", "HEAD"
case env[REQUEST_METHOD]
when GET, HEAD
status, headers, body = @app.call(env)
headers = Utils::HeaderHash.new(headers)
if status == 200 && fresh?(env, headers)
status = 304
headers.delete('Content-Type')
headers.delete('Content-Length')
headers.delete(CONTENT_TYPE)
headers.delete(CONTENT_LENGTH)
original_body = body
body = Rack::BodyProxy.new([]) do
original_body.close if original_body.respond_to?(:close)
Expand Down
4 changes: 2 additions & 2 deletions lib/rack/content_length.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def call(env)
headers = HeaderHash.new(headers)

if !STATUS_WITH_NO_ENTITY_BODY.include?(status.to_i) &&
!headers['Content-Length'] &&
!headers[CONTENT_LENGTH] &&
!headers['Transfer-Encoding'] &&
body.respond_to?(:to_ary)

Expand All @@ -28,7 +28,7 @@ def call(env)
obody.close if obody.respond_to?(:close)
end

headers['Content-Length'] = length.to_s
headers[CONTENT_LENGTH] = length.to_s
end

[status, headers, body]
Expand Down
2 changes: 1 addition & 1 deletion lib/rack/content_type.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def call(env)
headers = Utils::HeaderHash.new(headers)

unless STATUS_WITH_NO_ENTITY_BODY.include?(status)
headers['Content-Type'] ||= @content_type
headers[CONTENT_TYPE] ||= @content_type
end

[status, headers, body]
Expand Down
8 changes: 4 additions & 4 deletions lib/rack/deflater.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,20 +54,20 @@ def call(env)
case encoding
when "gzip"
headers['Content-Encoding'] = "gzip"
headers.delete('Content-Length')
headers.delete(CONTENT_LENGTH)
mtime = headers.key?("Last-Modified") ?
Time.httpdate(headers["Last-Modified"]) : Time.now
[status, headers, GzipStream.new(body, mtime)]
when "deflate"
headers['Content-Encoding'] = "deflate"
headers.delete('Content-Length')
headers.delete(CONTENT_LENGTH)
[status, headers, DeflateStream.new(body)]
when "identity"
[status, headers, body]
when nil
message = "An acceptable encoding for the requested resource #{request.fullpath} could not be found."
bp = Rack::BodyProxy.new([message]) { body.close if body.respond_to?(:close) }
[406, {"Content-Type" => "text/plain", "Content-Length" => message.length.to_s}, bp]
[406, {CONTENT_TYPE => "text/plain", CONTENT_LENGTH => message.length.to_s}, bp]
end
end

Expand Down Expand Up @@ -138,7 +138,7 @@ def should_deflate?(env, status, headers, body)
# Skip compressing empty entity body responses and responses with
# no-transform set.
if Utils::STATUS_WITH_NO_ENTITY_BODY.include?(status) ||
headers['Cache-Control'].to_s =~ /\bno-transform\b/ ||
headers[CACHE_CONTROL].to_s =~ /\bno-transform\b/ ||
(headers['Content-Encoding'] && headers['Content-Encoding'] !~ /\bidentity\b/)
return false
end
Expand Down
10 changes: 5 additions & 5 deletions lib/rack/directory.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ def call(env)

def _call(env)
@env = env
@script_name = env['SCRIPT_NAME']
@path_info = Utils.unescape(env['PATH_INFO'])
@script_name = env[SCRIPT_NAME]
@path_info = Utils.unescape(env[PATH_INFO])

if forbidden = check_forbidden
forbidden
Expand All @@ -72,7 +72,7 @@ def check_forbidden
body = "Forbidden\n"
size = Rack::Utils.bytesize(body)
return [403, {"Content-Type" => "text/plain",

@GetSetExecute GetSetExecute Oct 3, 2014

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missed opportunity to replace "Content-Type" with CONTENT_TYPE. Also on line 132

@schneems schneems Oct 3, 2014

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RIght you are. I love open source, thanks for pointing this out! Updated in #742

"Content-Length" => size.to_s,
CONTENT_LENGTH => size.to_s,
"X-Cascade" => "pass"}, [body]]
end

Expand Down Expand Up @@ -101,7 +101,7 @@ def list_directory
@files << [ url, basename, size, type, mtime ]
end

return [ 200, {'Content-Type'=>'text/html; charset=utf-8'}, self ]
return [ 200, { CONTENT_TYPE =>'text/html; charset=utf-8'}, self ]
end

def stat(node, max = 10)
Expand Down Expand Up @@ -130,7 +130,7 @@ def entity_not_found
body = "Entity not found: #{@path_info}\n"
size = Rack::Utils.bytesize(body)
return [404, {"Content-Type" => "text/plain",
"Content-Length" => size.to_s,
CONTENT_LENGTH => size.to_s,
"X-Cascade" => "pass"}, [body]]
end

Expand Down
13 changes: 7 additions & 6 deletions lib/rack/etag.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ module Rack
# used when Etag is absent and a directive when it is present. The first
# defaults to nil, while the second defaults to "max-age=0, private, must-revalidate"
class ETag
ETAG_STRING = 'ETag'.freeze
DEFAULT_CACHE_CONTROL = "max-age=0, private, must-revalidate".freeze

def initialize(app, no_cache_control = nil, cache_control = DEFAULT_CACHE_CONTROL)
Expand All @@ -28,14 +29,14 @@ def call(env)
body = Rack::BodyProxy.new(new_body) do
original_body.close if original_body.respond_to?(:close)
end
headers['ETag'] = %(W/"#{digest}") if digest
headers[ETAG_STRING] = %(W/"#{digest}") if digest
end

unless headers['Cache-Control']
unless headers[CACHE_CONTROL]
if digest
headers['Cache-Control'] = @cache_control if @cache_control
headers[CACHE_CONTROL] = @cache_control if @cache_control
else
headers['Cache-Control'] = @no_cache_control if @no_cache_control
headers[CACHE_CONTROL] = @no_cache_control if @no_cache_control
end
end

Expand All @@ -53,8 +54,8 @@ def etag_body?(body)
end

def skip_caching?(headers)
(headers['Cache-Control'] && headers['Cache-Control'].include?('no-cache')) ||
headers.key?('ETag') || headers.key?('Last-Modified')
(headers[CACHE_CONTROL] && headers[CACHE_CONTROL].include?('no-cache')) ||
headers.key?(ETAG_STRING) || headers.key?('Last-Modified')
end

def digest_body(body)
Expand Down
16 changes: 8 additions & 8 deletions lib/rack/file.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@ def call(env)
F = ::File

def _call(env)
unless ALLOWED_VERBS.include? env["REQUEST_METHOD"]
unless ALLOWED_VERBS.include? env[REQUEST_METHOD]
return fail(405, "Method Not Allowed", {'Allow' => ALLOW_HEADER})
end

path_info = Utils.unescape(env["PATH_INFO"])
path_info = Utils.unescape(env[PATH_INFO])
clean_path_info = Utils.clean_path_info(path_info)

@path = F.join(@root, clean_path_info)
Expand All @@ -58,19 +58,19 @@ def _call(env)

def serving(env)
if env["REQUEST_METHOD"] == "OPTIONS"
return [200, {'Allow' => ALLOW_HEADER, 'Content-Length' => '0'}, []]
return [200, {'Allow' => ALLOW_HEADER, CONTENT_LENGTH => '0'}, []]
end
last_modified = F.mtime(@path).httpdate
return [304, {}, []] if env['HTTP_IF_MODIFIED_SINCE'] == last_modified

headers = { "Last-Modified" => last_modified }
mime = Mime.mime_type(F.extname(@path), @default_mime)
headers["Content-Type"] = mime if mime
headers[CONTENT_TYPE] = mime if mime

# Set custom headers
@headers.each { |field, content| headers[field] = content } if @headers

response = [ 200, headers, env["REQUEST_METHOD"] == "HEAD" ? [] : self ]
response = [ 200, headers, env[REQUEST_METHOD] == "HEAD" ? [] : self ]

# NOTE:
# We check via File::size? whether this file provides size info
Expand All @@ -97,7 +97,7 @@ def serving(env)
size = @range.end - @range.begin + 1
end

response[1]["Content-Length"] = size.to_s
response[1][CONTENT_LENGTH] = size.to_s
response
end

Expand All @@ -122,8 +122,8 @@ def fail(status, body, headers = {})
[
status,
{
"Content-Type" => "text/plain",
"Content-Length" => body.size.to_s,
CONTENT_TYPE => "text/plain",
CONTENT_LENGTH => body.size.to_s,
"X-Cascade" => "pass"
}.merge!(headers),
[body]
Expand Down
2 changes: 1 addition & 1 deletion lib/rack/handler.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def self.default(options = {})
options.delete :Port

Rack::Handler::FastCGI
elsif ENV.include?("REQUEST_METHOD")
elsif ENV.include?(REQUEST_METHOD)
Rack::Handler::CGI
elsif ENV.include?("RACK_HANDLER")
self.get(ENV["RACK_HANDLER"])
Expand Down
2 changes: 1 addition & 1 deletion lib/rack/handler/cgi.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def self.serve(app)
"rack.url_scheme" => ["yes", "on", "1"].include?(ENV["HTTPS"]) ? "https" : "http"
})

env["QUERY_STRING"] ||= ""
env[QUERY_STRING] ||= ""
env["HTTP_VERSION"] ||= env["SERVER_PROTOCOL"]
env["REQUEST_PATH"] ||= "/"

Expand Down
2 changes: 1 addition & 1 deletion lib/rack/handler/fastcgi.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ def self.serve(request, app)
"rack.url_scheme" => ["yes", "on", "1"].include?(env["HTTPS"]) ? "https" : "http"
})

env["QUERY_STRING"] ||= ""
env[QUERY_STRING] ||= ""
env["HTTP_VERSION"] ||= env["SERVER_PROTOCOL"]
env["REQUEST_PATH"] ||= "/"
env.delete "CONTENT_TYPE" if env["CONTENT_TYPE"] == ""
Expand Down
2 changes: 1 addition & 1 deletion lib/rack/handler/lsws.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def self.serve(app)
"rack.url_scheme" => ["yes", "on", "1"].include?(ENV["HTTPS"]) ? "https" : "http"
)

env["QUERY_STRING"] ||= ""
env[QUERY_STRING] ||= ""
env["HTTP_VERSION"] ||= env["SERVER_PROTOCOL"]
env["REQUEST_PATH"] ||= "/"
status, headers, body = app.call(env)
Expand Down
2 changes: 1 addition & 1 deletion lib/rack/handler/mongrel.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ def process(request, response)

"rack.url_scheme" => ["yes", "on", "1"].include?(env["HTTPS"]) ? "https" : "http"
})
env["QUERY_STRING"] ||= ""
env[QUERY_STRING] ||= ""

status, headers, body = @app.call(env)

Expand Down
4 changes: 2 additions & 2 deletions lib/rack/handler/scgi.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ def process_request(request, input_body, socket)
env = Hash[request]
env.delete "HTTP_CONTENT_TYPE"
env.delete "HTTP_CONTENT_LENGTH"
env["REQUEST_PATH"], env["QUERY_STRING"] = env["REQUEST_URI"].split('?', 2)
env["REQUEST_PATH"], env[QUERY_STRING] = env["REQUEST_URI"].split('?', 2)
env["HTTP_VERSION"] ||= env["SERVER_PROTOCOL"]
env["PATH_INFO"] = env["REQUEST_PATH"]
env[PATH_INFO] = env["REQUEST_PATH"]
env["QUERY_STRING"] ||= ""
env["SCRIPT_NAME"] = ""

Expand Down
8 changes: 4 additions & 4 deletions lib/rack/handler/webrick.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,12 +79,12 @@ def service(req, res)
})

env["HTTP_VERSION"] ||= env["SERVER_PROTOCOL"]
env["QUERY_STRING"] ||= ""
unless env["PATH_INFO"] == ""
env[QUERY_STRING] ||= ""
unless env[PATH_INFO] == ""
path, n = req.request_uri.path, env["SCRIPT_NAME"].length
env["PATH_INFO"] = path[n, path.length-n]
env[PATH_INFO] = path[n, path.length-n]
end
env["REQUEST_PATH"] ||= [env["SCRIPT_NAME"], env["PATH_INFO"]].join
env["REQUEST_PATH"] ||= [env["SCRIPT_NAME"], env[PATH_INFO]].join

status, headers, body = @app.call(env)
begin
Expand Down
Loading