* [PATCH 0/3] reflect changes to Rack::Utils::HTTP_STATUS_CODES
@ 2015-06-30 22:51 Eric Wong
2015-06-30 22:51 ` [PATCH 1/3] reflect changes in Rack::Utils::HTTP_STATUS_CODES Eric Wong
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Eric Wong @ 2015-06-30 22:51 UTC (permalink / raw)
To: unicorn-public; +Cc: e
A user privately reported (to unicorn@bogomips.org) that they wanted
status text reflected in their responses for an HTTP status code not
included with Rack::Utils::HTTP_STATUS_CODES.
They tried to modify Rack::Utils::HTTP_STATUS_CODES hash directly in
their app, but unicorn loads before their app and already memoized the
status codes internally in the CODES hash to reduce GC pressure, thus
their change was never reflected.
With the first change, users can change Rack::Utils::HTTP_STATUS_CODES
as many times as they want (perhaps even based on time-of-day, weather,
server load, whatever) and the changes will be reflected
instantaneously in responses.
Of course, this slows unicorn down slightly due to increased GC
pressure, but I doubt anybody would notice in Real World usage.
In case they do, patch 2/3 will recover the lost performance
if they're using Ruby 2.2+
I figure anybody who cares about micro-benchmark performance with
unicorn would be using the latest Rubies anyways...
That user is Bcc:-ed for these patches, and can follow any public
discussion at: http://bogomips.org/unicorn-public/
(including the Atom feed at http://bogomips.org/unicorn-public/atom.xml )
* [PATCH 1/3] reflect changes in Rack::Utils::HTTP_STATUS_CODES
introduces a performance regression
* [PATCH 2/3] reduce constants and optimize for Ruby 2.2
recover lost performance from [PATCH 1/3] (on Ruby 2.2+),
further regressions for 2.1+
* [PATCH 3/3] http_response: reduce size of multi-line header path
bonus reduce code size for common cases, should
lib/unicorn/http_request.rb | 19 +++++++------------
lib/unicorn/http_response.rb | 23 ++++++++++-------------
test/unit/test_response.rb | 20 ++++++++++++++++++++
3 files changed, 37 insertions(+), 25 deletions(-)
^ permalink raw reply [flat|nested] 4+ messages in thread
* [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
end of thread, other threads:[~2015-06-30 22:52 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 ` [PATCH 3/3] http_response: reduce size of multi-line header path Eric Wong
Code repositories for project(s) associated with this public inbox
https://yhbt.net/unicorn.git/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).