* [PATCH 1/3] reflect changes in Rack::Utils::HTTP_STATUS_CODES
2015-06-30 22:51 [PATCH 0/3] reflect changes to Rack::Utils::HTTP_STATUS_CODES Eric Wong
@ 2015-06-30 22:51 ` Eric Wong
2015-06-30 22:51 ` [PATCH 2/3] reduce constants and optimize for Ruby 2.2 Eric Wong
2015-06-30 22:51 ` [PATCH 3/3] http_response: reduce size of multi-line header path Eric Wong
2 siblings, 0 replies; 4+ messages in thread
From: Eric Wong @ 2015-06-30 22:51 UTC (permalink / raw)
To: unicorn-public; +Cc: e
Applications may want to alter the message associated with HTTP
status codes in Rack::Utils::HTTP_STATUS_CODES. Avoid memoizing
status lines ahead-of-time
Note: this introduces a minor performance regression, but ought to
be unnoticeable unless you're running "Hello world"-type apps.
---
lib/unicorn/http_response.rb | 15 +++++++--------
test/unit/test_response.rb | 20 ++++++++++++++++++++
2 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/lib/unicorn/http_response.rb b/lib/unicorn/http_response.rb
index 016dac8..a42303e 100644
--- a/lib/unicorn/http_response.rb
+++ b/lib/unicorn/http_response.rb
@@ -10,15 +10,12 @@
# is the job of Rack, with the exception of the "Date" and "Status" header.
module Unicorn::HttpResponse
- # Every standard HTTP code mapped to the appropriate message.
- CODES = Rack::Utils::HTTP_STATUS_CODES.inject({}) { |hash,(code,msg)|
- hash[code] = "#{code} #{msg}"
- hash
- }
CRLF = "\r\n"
+ # internal API, code will always be common-enough-for-even-old-Rack
def err_response(code, response_start_sent)
- "#{response_start_sent ? '' : 'HTTP/1.1 '}#{CODES[code]}\r\n\r\n"
+ "#{response_start_sent ? '' : 'HTTP/1.1 '}" \
+ "#{code} #{Rack::Utils::HTTP_STATUS_CODES[code]}\r\n\r\n"
end
# writes the rack_response to socket as an HTTP response
@@ -26,9 +23,11 @@ module Unicorn::HttpResponse
response_start_sent=false)
hijack = nil
- http_response_start = response_start_sent ? '' : 'HTTP/1.1 '
if headers
- buf = "#{http_response_start}#{CODES[status.to_i] || status}\r\n" \
+ code = status.to_i
+ msg = Rack::Utils::HTTP_STATUS_CODES[code]
+ start = response_start_sent ? ''.freeze : 'HTTP/1.1 '.freeze
+ buf = "#{start}#{msg ? %Q(#{code} #{msg}) : status}\r\n" \
"Date: #{httpdate}\r\n" \
"Connection: close\r\n"
headers.each do |key, value|
diff --git a/test/unit/test_response.rb b/test/unit/test_response.rb
index 3478288..1e9d74a 100644
--- a/test/unit/test_response.rb
+++ b/test/unit/test_response.rb
@@ -79,4 +79,24 @@ class ResponseTest < Test::Unit::TestCase
headers = out.string.split(/\r\n\r\n/).first.split(/\r\n/)
assert %r{\AHTTP/\d\.\d 666 I AM THE BEAST\z}.match(headers[0])
end
+
+ def test_modified_rack_http_status_codes_late
+ r, w = IO.pipe
+ pid = fork do
+ r.close
+ # Users may want to globally override the status text associated
+ # with an HTTP status code in their app.
+ Rack::Utils::HTTP_STATUS_CODES[200] = "HI"
+ http_response_write(w, 200, {}, [])
+ w.close
+ end
+ w.close
+ assert_equal "HTTP/1.1 200 HI\r\n", r.gets
+ r.read # just drain the pipe
+ pid, status = Process.waitpid2(pid)
+ assert_predicate status, :success?
+ ensure
+ r.close
+ w.close unless w.closed?
+ end
end
--
EW
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 2/3] reduce constants and optimize for Ruby 2.2
2015-06-30 22:51 [PATCH 0/3] reflect changes to Rack::Utils::HTTP_STATUS_CODES Eric Wong
2015-06-30 22:51 ` [PATCH 1/3] reflect changes in Rack::Utils::HTTP_STATUS_CODES Eric Wong
@ 2015-06-30 22:51 ` Eric Wong
2015-06-30 22:51 ` [PATCH 3/3] http_response: reduce size of multi-line header path Eric Wong
2 siblings, 0 replies; 4+ messages in thread
From: Eric Wong @ 2015-06-30 22:51 UTC (permalink / raw)
To: unicorn-public; +Cc: e
Ruby (MRI) 2.1 optimizes allocations away on String#freeze with
literal strings.
Furthermore, Ruby 2.2 optimizes away literal string allocations
when they are used as arguments to Hash#[] and Hash#[]=
Thus we can avoid expensive constant lookups and cache overhead
by taking advantage of advancements in Ruby.
Since Ruby 2.2 has been out for 7 months, now; it ought to be safe
to introduce minor performance regressions for folks using older
Rubies (1.9.3+ remains supported) to benefit folks on the latest
Ruby.
This should recover the performance lost in the
"reflect changes in Rack::Utils::HTTP_STATUS_CODES" change
in synthetic benchmarks.
---
lib/unicorn/http_request.rb | 19 +++++++------------
lib/unicorn/http_response.rb | 6 ++----
2 files changed, 9 insertions(+), 16 deletions(-)
diff --git a/lib/unicorn/http_request.rb b/lib/unicorn/http_request.rb
index 9339bce..0c1f9bb 100644
--- a/lib/unicorn/http_request.rb
+++ b/lib/unicorn/http_request.rb
@@ -21,17 +21,12 @@ class Unicorn::HttpParser
"SERVER_SOFTWARE" => "Unicorn #{Unicorn::Const::UNICORN_VERSION}"
}
- RACK_HIJACK = "rack.hijack".freeze
- RACK_HIJACK_IO = "rack.hijack_io".freeze
NULL_IO = StringIO.new("")
# :stopdoc:
# A frozen format for this is about 15% faster
# Drop these frozen strings when Ruby 2.2 becomes more prevalent,
# 2.2+ optimizes hash assignments when used with literal string keys
- REMOTE_ADDR = 'REMOTE_ADDR'.freeze
- RACK_INPUT = 'rack.input'.freeze
- UNICORN_SOCKET = 'unicorn.socket'.freeze
HTTP_RESPONSE_START = [ 'HTTP', '/1.1 ']
@@input_class = Unicorn::TeeInput
@@check_client_connection = false
@@ -78,7 +73,7 @@ class Unicorn::HttpParser
# identify the client for the immediate request to the server;
# that client may be a proxy, gateway, or other intermediary
# acting on behalf of the actual source client."
- e[REMOTE_ADDR] = socket.kgio_addr
+ e['REMOTE_ADDR'] = socket.kgio_addr
# short circuit the common case with small GET requests first
socket.kgio_read!(16384, buf)
@@ -94,12 +89,12 @@ class Unicorn::HttpParser
HTTP_RESPONSE_START.each { |c| socket.write(c) }
end
- e[RACK_INPUT] = 0 == content_length ?
- NULL_IO : @@input_class.new(socket, self)
+ e['rack.input'] = 0 == content_length ?
+ NULL_IO : @@input_class.new(socket, self)
# for Rack hijacking in Rack 1.5 and later
- e[UNICORN_SOCKET] = socket
- e[RACK_HIJACK] = self
+ e['unicorn.socket'] = socket
+ e['rack.hijack'] = self
e.merge!(DEFAULTS)
end
@@ -107,10 +102,10 @@ class Unicorn::HttpParser
# for rack.hijack, we respond to this method so no extra allocation
# of a proc object
def call
- env[RACK_HIJACK_IO] = env[UNICORN_SOCKET]
+ env['rack.hijack_io'] = env['unicorn.socket']
end
def hijacked?
- env.include?(RACK_HIJACK_IO)
+ env.include?('rack.hijack_io'.freeze)
end
end
diff --git a/lib/unicorn/http_response.rb b/lib/unicorn/http_response.rb
index a42303e..ec8b7c8 100644
--- a/lib/unicorn/http_response.rb
+++ b/lib/unicorn/http_response.rb
@@ -10,8 +10,6 @@
# is the job of Rack, with the exception of the "Date" and "Status" header.
module Unicorn::HttpResponse
- CRLF = "\r\n"
-
# internal API, code will always be common-enough-for-even-old-Rack
def err_response(code, response_start_sent)
"#{response_start_sent ? '' : 'HTTP/1.1 '}" \
@@ -39,7 +37,7 @@ module Unicorn::HttpResponse
# key in Rack < 1.5
hijack = value
else
- if value =~ /\n/
+ if value.include?("\n".freeze)
# avoiding blank, key-only cookies with /\n+/
buf << value.split(/\n+/).map! { |v| "#{key}: #{v}\r\n" }.join
else
@@ -47,7 +45,7 @@ module Unicorn::HttpResponse
end
end
end
- socket.write(buf << CRLF)
+ socket.write(buf << "\r\n".freeze)
end
if hijack
--
EW
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 3/3] http_response: reduce size of multi-line header path
2015-06-30 22:51 [PATCH 0/3] reflect changes to Rack::Utils::HTTP_STATUS_CODES Eric Wong
2015-06-30 22:51 ` [PATCH 1/3] reflect changes in Rack::Utils::HTTP_STATUS_CODES Eric Wong
2015-06-30 22:51 ` [PATCH 2/3] reduce constants and optimize for Ruby 2.2 Eric Wong
@ 2015-06-30 22:51 ` Eric Wong
2 siblings, 0 replies; 4+ messages in thread
From: Eric Wong @ 2015-06-30 22:51 UTC (permalink / raw)
To: unicorn-public; +Cc: e
This should save over 100 bytes of bytecode overhead due to
reduced method dispatch points. The performance difference
when this is actually hit could go either way depending on
how String#<< and realloc(3) interact, but it's uncommon
enough that nobody is expected to notice either way.
---
lib/unicorn/http_response.rb | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/unicorn/http_response.rb b/lib/unicorn/http_response.rb
index ec8b7c8..c1aa738 100644
--- a/lib/unicorn/http_response.rb
+++ b/lib/unicorn/http_response.rb
@@ -39,7 +39,7 @@ module Unicorn::HttpResponse
else
if value.include?("\n".freeze)
# avoiding blank, key-only cookies with /\n+/
- buf << value.split(/\n+/).map! { |v| "#{key}: #{v}\r\n" }.join
+ value.split(/\n+/).each { |v| buf << "#{key}: #{v}\r\n" }
else
buf << "#{key}: #{value}\r\n"
end
--
EW
^ permalink raw reply related [flat|nested] 4+ messages in thread