[PATCH 0/6] misc minor updates

[PATCH 0/6] misc minor updates

[Eric Wong]

Most of this are minor cleanups to reduce code size.  I'll probably
make the sendfile dependency optional as well.  Not everybody needs
static file serving, and the pread-style offset-agnosticism is
unlikely to be useful unless we feel the need need to implement open
file caching for static files.

Eric Wong (6):
      http_response: remove arg for Array#join
      remove unused client_max_header_size config
      config: use literal symbol array for now
      http_response: reduce constants for 100 responses
      favor Array#map! for freshly-split arrays
      sendfile_compat: remove dependency on pread

 INSTALL                      |  4 +---
 lib/yahns/config.rb          |  3 +--
 lib/yahns/http_context.rb    |  1 -
 lib/yahns/http_response.rb   | 11 +++++------
 lib/yahns/sendfile_compat.rb | 12 +++++-------
 lib/yahns/server.rb          |  2 +-
 lib/yahns/tmpio.rb           |  2 +-
 7 files changed, 14 insertions(+), 21 deletions(-)

[PATCH 1/6] http_response: remove arg for Array#join

[Eric Wong]

Our kv_str method already fails if `$,' is a non-empty string.
Rack::Chunked and likely other middlewares fails when `$,' is
not empty, too, so supporting apps which set `$,' is probably
not feasible.
---
 lib/yahns/http_response.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/yahns/http_response.rb b/lib/yahns/http_response.rb
index 33ae93e..e48e57c 100644
--- a/lib/yahns/http_response.rb
+++ b/lib/yahns/http_response.rb
@@ -27,7 +27,7 @@ module Yahns::HttpResponse # :nodoc:
   CONN_CLOSE = "Connection: close\r\n\r\n"
   Z = ""
   CCC_RESPONSE_START = [ 'HTTP', '/1.1 ' ]
-  RESPONSE_START = CCC_RESPONSE_START.join('')
+  RESPONSE_START = CCC_RESPONSE_START.join
   R100_RAW = "HTTP/1.1 100 Continue\r\n\r\n"
   R100_CCC = "100 Continue\r\n\r\nHTTP/1.1 "
   HTTP_EXPECT = "HTTP_EXPECT"
-- 
EW

[PATCH 2/6] remove unused client_max_header_size config

[Eric Wong]

We may not be able to support this in a more performant
way just yet.  Since this was never documented, we'll remove
the the current knobs for silently setting and ignoring it.

Users should use Unicorn::HttpParser.max_header_len= for now,
instead.  We may change Unicorn::HttpParser in the future if enough
people care about making this functionality per-app.
---
 lib/yahns/config.rb       | 1 -
 lib/yahns/http_context.rb | 1 -
 2 files changed, 2 deletions(-)

diff --git a/lib/yahns/config.rb b/lib/yahns/config.rb
index 4ea51af..3055539 100644
--- a/lib/yahns/config.rb
+++ b/lib/yahns/config.rb
@@ -363,7 +363,6 @@ class Yahns::Config # :nodoc:
     # config name, minimum value
     client_body_buffer_size: 1,
     client_header_buffer_size: 1,
-    client_max_header_size: 1,
   }.each do |_v,minval|
     eval(
     %Q(def #{_v}(val);) <<
diff --git a/lib/yahns/http_context.rb b/lib/yahns/http_context.rb
index 349e129..73bb49a 100644
--- a/lib/yahns/http_context.rb
+++ b/lib/yahns/http_context.rb
@@ -9,7 +9,6 @@ module Yahns::HttpContext # :nodoc:
   attr_accessor :client_body_buffer_size
   attr_accessor :client_header_buffer_size
   attr_accessor :client_max_body_size
-  attr_accessor :client_max_header_size
   attr_accessor :input_buffering  # :lazy, true, false
   attr_accessor :output_buffering # true, false
   attr_accessor :persistent_connections # true or false only
-- 
EW

[PATCH 3/6] config: use literal symbol array for now

[Eric Wong]

Until we drop 1.9.3 support, we'll save some bytecode by using
[ :literal, :symbols, :in, :arrays ]

In 2.0.0 and later, we may use %i(terser syntax)
---
 lib/yahns/config.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/yahns/config.rb b/lib/yahns/config.rb
index 3055539..f354961 100644
--- a/lib/yahns/config.rb
+++ b/lib/yahns/config.rb
@@ -402,7 +402,7 @@ class Yahns::Config # :nodoc:
       val.close_on_exec = val.sync = true
       val.binmode
     else
-      rt = %w(puts write flush).map(&:to_sym) # match Rack::Lint
+      rt = [ :puts, :write, :flush ] # match Rack::Lint
       rt.all? { |m| val.respond_to?(m) } or raise ArgumentError,
                    "`#{var}' destination must respond to all of: #{rt.inspect}"
     end
-- 
EW

[PATCH 4/6] http_response: reduce constants for 100 responses

[Eric Wong]

Ruby 2.1 optimizes String#freeze by deduplicating string literal
calls to freeze.  Ruby 2.2 _may_ also optimize away allocations to
Hash#delete in the future.  In any case, this is uncommon code and
not worth trading permanent space to reduce temporal garbage.

While this favors Ruby 2.1 and later, it remains completely
compatible with Ruby 1.9.3 and 2.0.0.
---
 lib/yahns/http_response.rb | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/lib/yahns/http_response.rb b/lib/yahns/http_response.rb
index e48e57c..0b0296f 100644
--- a/lib/yahns/http_response.rb
+++ b/lib/yahns/http_response.rb
@@ -28,9 +28,6 @@ module Yahns::HttpResponse # :nodoc:
   Z = ""
   CCC_RESPONSE_START = [ 'HTTP', '/1.1 ' ]
   RESPONSE_START = CCC_RESPONSE_START.join
-  R100_RAW = "HTTP/1.1 100 Continue\r\n\r\n"
-  R100_CCC = "100 Continue\r\n\r\nHTTP/1.1 "
-  HTTP_EXPECT = "HTTP_EXPECT"
   REQUEST_METHOD = "REQUEST_METHOD"
   HEAD = "HEAD"
 
@@ -258,8 +255,10 @@ module Yahns::HttpResponse # :nodoc:
   # returns nil on success
   # returns :close, :wait_writable, or :wait_readable
   def http_100_response(env)
-    env.delete(HTTP_EXPECT) =~ /\A100-continue\z/i or return nil
-    buf = @response_start_sent ? R100_CCC : R100_RAW
+    env.delete("HTTP_EXPECT") =~ /\A100-continue\z/i or return
+    buf = @response_start_sent ? "100 Continue\r\n\r\nHTTP/1.1 ".freeze
+                               : "HTTP/1.1 100 Continue\r\n\r\n".freeze
+
     case rv = kgio_trywrite(buf)
     when String
       buf = rv
-- 
EW

[PATCH 5/6] favor Array#map! for freshly-split arrays

[Eric Wong]

This barely reduces garbage objects at startup, but less
garbage is usually better.
---
 lib/yahns/server.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/yahns/server.rb b/lib/yahns/server.rb
index b28e741..1196d2d 100644
--- a/lib/yahns/server.rb
+++ b/lib/yahns/server.rb
@@ -313,7 +313,7 @@ class Yahns::Server # :nodoc:
     # because that can completely break the non-blocking one.
     # Unfortunately, there is no one-off MSG_DONTWAIT-like flag for
     # accept4(2).
-    inherited = ENV['YAHNS_FD'].to_s.split(',').map do |fd|
+    inherited = ENV['YAHNS_FD'].to_s.split(',').map! do |fd|
       io = Socket.for_fd(fd.to_i)
       set_server_sockopt(io, sock_opts(io))
       @logger.info "inherited addr=#{sock_name(io)} fd=#{fd}"
-- 
EW

[PATCH 6/6] sendfile_compat: remove dependency on pread

[Eric Wong]

We only need to open files with O_APPEND to allow appending to the
temporary buffer while leaving the read offset unchanged.
---
 INSTALL                      |  4 +---
 lib/yahns/sendfile_compat.rb | 12 +++++-------
 lib/yahns/tmpio.rb           |  2 +-
 3 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/INSTALL b/INSTALL
index 87acc53..fcc92f2 100644
--- a/INSTALL
+++ b/INSTALL
@@ -10,6 +10,4 @@ more details.
 Debian GNU/kFreeBSD:
 
 For now, you will need to define SENDFILE_BROKEN=1 in the env before
-running yahns, and also install the "io-extra" RubyGem (tested on 1.2.7)
-
-	gem install -v 1.2.7 io-extra
+running yahns.
diff --git a/lib/yahns/sendfile_compat.rb b/lib/yahns/sendfile_compat.rb
index e3f53d1..f324075 100644
--- a/lib/yahns/sendfile_compat.rb
+++ b/lib/yahns/sendfile_compat.rb
@@ -1,25 +1,23 @@
 # -*- encoding: binary -*-
 # Copyright (C) 2009-2014, Eric Wong <normalperson@yhbt.net> et. al.
 # License: GPLv3 or later (https://www.gnu.org/licenses/gpl-3.0.txt)
-require 'io/extra' # gem install io-extra
 
 module Yahns::SendfileCompat
   def trysendfile(io, offset, count)
     return 0 if count == 0
     count = 0x4000 if count > 0x4000
-    str = IO.pread(io.fileno, count, offset)
-    if count > str.bytesize
-      raise EOFError, "end of file reached"
-    end
+    buf = Thread.current[:yahns_sfbuf] ||= ''
+    io.pos = offset
+    str = io.read(count, buf) or return # nil for EOF
     n = 0
     case rv = kgio_trywrite(str)
     when String # partial write, keep trying
-      n += (str.bytesize - rv.bytesize)
+      n += (str.size - rv.size)
       str = rv
     when :wait_writable, :wait_readable
       return n > 0 ? n : rv
     when nil
-      return n + str.bytesize # yay!
+      return n + str.size # yay!
     end while true
   end
 end
diff --git a/lib/yahns/tmpio.rb b/lib/yahns/tmpio.rb
index 19da658..f0ddd2f 100644
--- a/lib/yahns/tmpio.rb
+++ b/lib/yahns/tmpio.rb
@@ -14,7 +14,7 @@ class Yahns::TmpIO < File # :nodoc:
   def self.new(tmpdir = Dir.tmpdir)
     retried = false
     begin
-      fp = super("#{tmpdir}/#{rand}", RDWR|CREAT|EXCL, 0600)
+      fp = super("#{tmpdir}/#{rand}", RDWR|CREAT|EXCL|APPEND, 0600)
     rescue Errno::EEXIST
       retry
     rescue Errno::EMFILE, Errno::ENFILE
-- 
EW