From d613f0aaa2758e294e440766c02243a2023b599c Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Sun, 3 May 2009 22:11:11 -0700
Subject: Speed up the worker accept loop

First, reduce no-op fchmod syscalls under heavy traffic.
gettimeofday(2) is a cheaper syscall than fchmod(2).  Since
ctime resolution is only in seconds on most filesystems (and
Ruby can only get to seconds AFAIK), we can avoid fchmod(2)
happening within the same second.  This allows us to cheat on
synthetic benchmarks where performance is measured in
requests-per-second and not seconds-per-request :)

Secondly, cleanup the acceptor loop and avoid nested
begins/loops as much as possible.  If we got ECONNABORTED, then
there's no way the client variable would've been set correctly,
either.  If there was something there, then it is at the mercy
of the garbage collector because a method can't both return a
value and raise an exception.
---
 lib/unicorn.rb | 81 +++++++++++++++++++++++++++-------------------------------
 1 file changed, 37 insertions(+), 44 deletions(-)

diff --git a/lib/unicorn.rb b/lib/unicorn.rb
index a539960..aadd7af 100644
--- a/lib/unicorn.rb
+++ b/lib/unicorn.rb
@@ -469,7 +469,7 @@ module Unicorn
       nr = 0 # this becomes negative if we need to reopen logs
       alive = worker.tempfile # tempfile is our lifeline to the master process
       ready = LISTENERS
-      client = nil
+      t = ti = Time.now.to_i
 
       # closing anything we IO.select on will raise EBADF
       trap(:USR1) { nr = -65536; SELF_PIPE.first.close rescue nil }
@@ -477,8 +477,10 @@ module Unicorn
       [:TERM, :INT].each { |sig| trap(sig) { exit!(0) } } # instant shutdown
       @logger.info "worker=#{worker.nr} ready"
 
-      while alive
-        reopen_worker_logs(worker.nr) if nr < 0
+      begin
+        nr < 0 and reopen_worker_logs(worker.nr)
+        nr = 0
+
         # we're a goner in @timeout seconds anyways if alive.chmod
         # breaks, so don't trap the exception.  Using fchmod() since
         # futimes() is not available in base Ruby and I very strongly
@@ -487,50 +489,41 @@ module Unicorn
         # changes with chmod doesn't update ctime on all filesystems; so
         # we change our counter each and every time (after process_client
         # and before IO.select).
-        alive.chmod(nr = 0)
+        t == (ti = Time.now.to_i) or alive.chmod(t = ti)
+
+        ready.each do |sock|
+          begin
+            process_client(sock.accept_nonblock)
+            nr += 1
+            t == (ti = Time.now.to_i) or alive.chmod(t = ti)
+          rescue Errno::EAGAIN, Errno::ECONNABORTED
+          end
+          break if nr < 0
+        end
 
+        # make the following bet: if we accepted clients this round,
+        # we're probably reasonably busy, so avoid calling select()
+        # and do a speculative accept_nonblock on every listener
+        # before we sleep again in select().
+        redo unless nr == 0 # (nr < 0) => reopen logs
+
+        master_pid == Process.ppid or return
+        t == (ti = Time.now.to_i) or alive.chmod(t = ti)
         begin
-          ready.each do |sock|
-            begin
-              client = begin
-                sock.accept_nonblock
-              rescue Errno::EAGAIN
-                next
-              end
-              process_client(client)
-              alive.chmod(nr += 1)
-            rescue Errno::ECONNABORTED
-              # client closed the socket even before accept
-              client.close rescue nil
-            end
-            break if nr < 0
-          end
-          client = nil
-
-          # make the following bet: if we accepted clients this round,
-          # we're probably reasonably busy, so avoid calling select()
-          # and do a speculative accept_nonblock on every listener
-          # before we sleep again in select().
-          if nr == 0 # (nr < 0) => reopen logs
-            master_pid == Process.ppid or return
-            alive.chmod(nr += 1)
-            begin
-              # timeout used so we can detect parent death:
-              ret = IO.select(LISTENERS, nil, SELF_PIPE, @timeout) or next
-              ready = ret.first
-            rescue Errno::EINTR
-              ready = LISTENERS
-            rescue Errno::EBADF => e
-              nr < 0 or exit(alive ? 1 : 0)
-            end
-          end
-        rescue Object => e
-          if alive
-            logger.error "Unhandled listen loop exception #{e.inspect}."
-            logger.error e.backtrace.join("\n")
-          end
+          # timeout used so we can detect parent death:
+          ret = IO.select(LISTENERS, nil, SELF_PIPE, @timeout) or redo
+          ready = ret.first
+        rescue Errno::EINTR
+          ready = LISTENERS
+        rescue Errno::EBADF
+          nr < 0 or return
         end
-      end
+      rescue Object => e
+        if alive
+          logger.error "Unhandled listen loop exception #{e.inspect}."
+          logger.error e.backtrace.join("\n")
+        end
+      end while alive
     end
 
     # delivers a signal to a worker and fails gracefully if the worker
-- 
cgit v1.2.3-24-ge0c7