about summary refs log tree commit homepage
diff options
context:
space:
mode:
authorEric Wong <normalperson@yhbt.net>2009-08-15 01:02:03 -0700
committerEric Wong <normalperson@yhbt.net>2009-08-15 02:38:48 -0700
commite184dfc9e6f0e8f142c3fbb0e9d98e8c57f82469 (patch)
tree0fde534669961cb690972715d3deed422d803266
parent8c2e83f98ec8ba9dd2e02f9579b6b2cdefc1b8af (diff)
downloadunicorn-e184dfc9e6f0e8f142c3fbb0e9d98e8c57f82469.tar.gz
The code I was _about_ to commit to support them was too ugly
and the performance benefit in real applications/clients is
unproven.

Support for these things also had the inevitable side effect of
adding overhead to non-persistent requests.  Worst of all,
interaction with people tweaking TCP_CORK/TCP_NOPUSH suddenly
becomes an order of magnitude more complex because of the
_need_ to flush the socket out in between requests (for
most apps).

Aggressive pipelining can actually help a lot with a direct
connection to Unicorn, not via proxy.  But the applications (and
clients) that would benefit from it are very few and moving
those applications away from HTTP entirely would be an even
bigger benefit.

The C/Ragel parser will maintain keepalive support for now since
I've always intended for that to be used in more general-purpose
HTTP servers than Unicorn.

For documentation purposes, the keep-alive/pipelining patch is
included below in its entirety.  I don't have the
TCP_CORK/TCP_NOPUSH parts in there because that's when I finally
gave up and decided it would not be worth supporting.

  diff --git a/lib/unicorn.rb b/lib/unicorn.rb
  index b185b25..cc58997 100644
  --- a/lib/unicorn.rb
  +++ b/lib/unicorn.rb
  @@ -439,22 +439,24 @@ module Unicorn
       # once a client is accepted, it is processed in its entirety here
       # in 3 easy steps: read request, call app, write app response
       def process_client(client)
  +      response = nil
         client.fcntl(Fcntl::F_SETFD, Fcntl::FD_CLOEXEC)
  -      response = app.call(env = REQUEST.read(client))
  -
  -      if 100 == response.first.to_i
  -        client.write(Const::EXPECT_100_RESPONSE)
  -        env.delete(Const::HTTP_EXPECT)
  -        response = app.call(env)
  -      end
  +      begin
  +        response = app.call(env = REQUEST.read(client))

  -      HttpResponse.write(client, response)
  +        if 100 == response.first.to_i
  +          client.write(Const::EXPECT_100_RESPONSE)
  +          env.delete(Const::HTTP_EXPECT)
  +          response = app.call(env)
  +        end
  +      end while HttpResponse.write(client, response, HttpRequest::PARSER.keepalive?)
  +      client.close
       # if we get any error, try to write something back to the client
       # assuming we haven't closed the socket, but don't get hung up
       # if the socket is already closed or broken.  We'll always ensure
       # the socket is closed at the end of this function
  -    rescue EOFError,Errno::ECONNRESET,Errno::EPIPE,Errno::EINVAL,Errno::EBADF
  -      client.write_nonblock(Const::ERROR_500_RESPONSE) rescue nil
  +    rescue EOFError,Errno::ECONNRESET,Errno::EPIPE,Errno::EINVAL,
  +           Errno::EBADF,Errno::ENOTCONN
         client.close rescue nil
       rescue HttpParserError # try to tell the client they're bad
         client.write_nonblock(Const::ERROR_400_RESPONSE) rescue nil
  diff --git a/lib/unicorn/http_request.rb b/lib/unicorn/http_request.rb
  index 1358ccc..f73bdd0 100644
  --- a/lib/unicorn/http_request.rb
  +++ b/lib/unicorn/http_request.rb
  @@ -42,6 +42,7 @@ module Unicorn
       # to handle any socket errors (e.g. user aborted upload).
       def read(socket)
         REQ.clear
  +      pipelined = PARSER.keepalive? && 0 < BUF.size
         PARSER.reset

         # From http://www.ietf.org/rfc/rfc3875:
  @@ -55,7 +56,9 @@ module Unicorn
                       TCPSocket === socket ? socket.peeraddr.last : LOCALHOST

         # short circuit the common case with small GET requests first
  -      PARSER.headers(REQ, socket.readpartial(Const::CHUNK_SIZE, BUF)) and
  +      PARSER.headers(REQ,
  +                     pipelined ? BUF :
  +                     socket.readpartial(Const::CHUNK_SIZE, BUF)) and
             return handle_body(socket)

         data = BUF.dup # socket.readpartial will clobber data
  diff --git a/lib/unicorn/http_response.rb b/lib/unicorn/http_response.rb
  index 5602a43..e22b5f9 100644
  --- a/lib/unicorn/http_response.rb
  +++ b/lib/unicorn/http_response.rb
  @@ -20,12 +20,15 @@ module Unicorn
     # to most clients since they can close their connection immediately.

     class HttpResponse
  +    include Unicorn::SocketHelper

       # Every standard HTTP code mapped to the appropriate message.
       CODES = Rack::Utils::HTTP_STATUS_CODES.inject({}) { |hash,(code,msg)|
         hash[code] = "#{code} #{msg}"
         hash
       }
  +    CLOSE = "close".freeze # :nodoc
  +    KEEPALIVE = "keep-alive".freeze # :nodoc

       # Rack does not set/require a Date: header.  We always override the
       # Connection: and Date: headers no matter what (if anything) our
  @@ -34,7 +37,7 @@ module Unicorn
       OUT = [] # :nodoc

       # writes the rack_response to socket as an HTTP response
  -    def self.write(socket, rack_response)
  +    def self.write(socket, rack_response, keepalive = false)
         status, headers, body = rack_response
         status = CODES[status.to_i] || status
         OUT.clear
  @@ -50,6 +53,10 @@ module Unicorn
           end
         end

  +      if keepalive && (0 == HttpRequest::BUF.size)
  +        keepalive = IO.select([socket], nil, nil, 0.0)
  +      end
  +
         # Rack should enforce Content-Length or chunked transfer encoding,
         # so don't worry or care about them.
         # Date is required by HTTP/1.1 as long as our clock can be trusted.
  @@ -57,10 +64,10 @@ module Unicorn
         socket.write("HTTP/1.1 #{status}\r\n" \
                      "Date: #{Time.now.httpdate}\r\n" \
                      "Status: #{status}\r\n" \
  -                   "Connection: close\r\n" \
  +                   "Connection: #{keepalive ? KEEPALIVE : CLOSE}\r\n" \
                      "#{OUT.join(Z)}\r\n")
         body.each { |chunk| socket.write(chunk) }
  -      socket.close # flushes and uncorks the socket immediately
  +      keepalive
         ensure
           body.respond_to?(:close) and body.close rescue nil
       end
-rw-r--r--TODO2
1 files changed, 0 insertions, 2 deletions
diff --git a/TODO b/TODO
index 01e6596..0b73934 100644
--- a/TODO
+++ b/TODO
@@ -9,5 +9,3 @@
 * Rainbows!
 
 * Rubinius support
-
-* keepalive (1 second max) + pipelining support (Ragel bits already done)