yahns Ruby server user/dev discussion
 help / color / Atom feed
From: Eric Wong <yahns-public@yhbt.net>
To: yahns-public@yhbt.net
Subject: [PATCH 2/2] drop writev support
Date: Mon,  6 Mar 2017 03:30:11 +0000
Message-ID: <20170306033011.26938-3-yahns-public@yhbt.net> (raw)
In-Reply-To: <20170306033011.26938-1-yahns-public@yhbt.net>

Since we're getting rid of kgio, we'll need to remove
writev support as it's not in mainline Ruby.  I've never
checked if writev support was ever worth it for performance,
and benchmarks will be coming.

At least this simplifies req_res.rb quite a bit.
---
 lib/yahns/chunk_body.rb          | 15 ++++--------
 lib/yahns/http_response.rb       |  6 ++---
 lib/yahns/openssl_client.rb      |  4 ----
 lib/yahns/proxy_http_response.rb | 16 +++++++------
 lib/yahns/req_res.rb             | 51 +++++++++++++++++++---------------------
 lib/yahns/tmpio.rb               |  1 -
 lib/yahns/wbuf.rb                | 11 +++------
 test/test_tmpio.rb               |  6 ++---
 8 files changed, 46 insertions(+), 64 deletions(-)

diff --git a/lib/yahns/chunk_body.rb b/lib/yahns/chunk_body.rb
index aab803b..7153ad9 100644
--- a/lib/yahns/chunk_body.rb
+++ b/lib/yahns/chunk_body.rb
@@ -1,24 +1,19 @@
 # -*- encoding: binary -*-
 # Copyright (C) 2016 all contributors <yahns-public@yhbt.net>
 # License: GPL-3.0+ <https://www.gnu.org/licenses/gpl-3.0.txt>
-# frozen_string_literal: true
+# frozen_string_literal: false
 
 class Yahns::ChunkBody # :nodoc:
-  def initialize(body, vec)
+  def initialize(body)
     @body = body
-    @vec = vec
   end
 
   def each
-    vec = @vec
-    vec[2] = "\r\n".freeze
     @body.each do |chunk|
-      vec[0] = "#{chunk.bytesize.to_s(16)}\r\n"
-      vec[1] = chunk
-      # vec[2] never changes: "\r\n" above
-      yield vec
+      chunk = "#{chunk.bytesize.to_s(16)}\r\n#{chunk}\r\n"
+      yield chunk
+      chunk.clear
     end
-    vec.clear
     yield "0\r\n\r\n".freeze
   end
 
diff --git a/lib/yahns/http_response.rb b/lib/yahns/http_response.rb
index 86b7f56..7fa70ff 100644
--- a/lib/yahns/http_response.rb
+++ b/lib/yahns/http_response.rb
@@ -167,7 +167,7 @@ def http_response_write(res, opt)
 
     if !term && chunk_ok && !hdr_only
       term = true
-      body = Yahns::ChunkBody.new(body, opt)
+      body = Yahns::ChunkBody.new(body)
       buf << "Transfer-Encoding: chunked\r\n".freeze
     end
     alive &&= (term || hdr_only)
@@ -205,10 +205,10 @@ def http_response_write(res, opt)
       if wbuf
         rv = wbuf.wbuf_write(self, x)
       else
-        case rv = String === x ? kgio_trywrite(x) : kgio_trywritev(x)
+        case rv = kgio_trywrite(x)
         when nil # all done, likely and good!
           break
-        when String, Array
+        when String
           x = rv # hope the skb grows when we loop into the trywrite
         when :wait_writable, :wait_readable
           if self.class.output_buffering
diff --git a/lib/yahns/openssl_client.rb b/lib/yahns/openssl_client.rb
index a86d701..cf4941c 100644
--- a/lib/yahns/openssl_client.rb
+++ b/lib/yahns/openssl_client.rb
@@ -51,10 +51,6 @@ def kgio_trywrite(buf)
     rv
   end
 
-  def kgio_trywritev(buf)
-    kgio_trywrite(buf.join)
-  end
-
   def kgio_syssend(buf, flags)
     kgio_trywrite(buf)
   end
diff --git a/lib/yahns/proxy_http_response.rb b/lib/yahns/proxy_http_response.rb
index d9c878e..9f75fec 100644
--- a/lib/yahns/proxy_http_response.rb
+++ b/lib/yahns/proxy_http_response.rb
@@ -1,7 +1,7 @@
 # -*- encoding: binary -*-
 # Copyright (C) 2015-2016 all contributors <yahns-public@yhbt.net>
 # License: GPL-3.0+ (https://www.gnu.org/licenses/gpl-3.0.txt)
-# frozen_string_literal: true
+# frozen_string_literal: false
 
 require_relative 'wbuf_lite'
 
@@ -31,9 +31,9 @@ def wbuf_alloc(req_res)
   def proxy_write(wbuf, buf, req_res)
     unless wbuf
       # no write buffer, try to write directly to the client socket
-      case rv = String === buf ? kgio_trywrite(buf) : kgio_trywritev(buf)
+      case rv = kgio_trywrite(buf)
       when nil then return # done writing buf, likely
-      when String, Array # partial write, hope the skb grows
+      when String # partial write, hope the skb grows
         buf = rv
       when :wait_writable, :wait_readable
         wbuf = req_res.resbuf ||= wbuf_alloc(req_res)
@@ -157,16 +157,18 @@ def proxy_read_body(tip, kcar, req_res)
     when String
       if len
         kcar.body_bytes_left -= tmp.size # progress for body_eof? => true
+        clr = nil
       elsif chunk
         kcar.filter_body(chunk, rbuf = tmp) # progress for body_eof? => true
         next if chunk.empty? # call req_res.read_nonblock for more
-        tmp = chunk_out(chunk)
+        clr = tmp = chunk_out(chunk)
+        chunk.clear
       elsif alive # HTTP/1.0 upstream, HTTP/1.1 client
-        tmp = chunk_out(tmp)
+        clr = tmp = chunk_out(tmp)
       # else # HTTP/1.0 upstream, HTTP/1.0 client, do nothing
       end
       wbuf = proxy_write(wbuf, tmp, req_res)
-      chunk.clear if chunk
+      clr.clear if clr
       if Yahns::WbufLite === wbuf
         req_res.proxy_trailers = [ rbuf.dup, tip ] if chunk && kcar.body_eof?
         return proxy_unbuffer(wbuf)
@@ -296,7 +298,7 @@ def proxy_busy_mod(wbuf, req_res)
   # of String#bytesize because all the IO read methods return a binary
   # string when given a maximum read length
   def chunk_out(buf)
-    [ "#{buf.size.to_s(16)}\r\n", buf, "\r\n".freeze ]
+    "#{buf.size.to_s(16)}\r\n#{buf}\r\n"
   end
 
   def trailer_out(tlr)
diff --git a/lib/yahns/req_res.rb b/lib/yahns/req_res.rb
index 0fa4285..958015a 100644
--- a/lib/yahns/req_res.rb
+++ b/lib/yahns/req_res.rb
@@ -1,7 +1,7 @@
 # -*- encoding: binary -*-
 # Copyright (C) 2013-2016 all contributors <yahns-public@yhbt.net>
 # License: GPL-3.0+ (https://www.gnu.org/licenses/gpl-3.0.txt)
-# frozen_string_literal: true
+# frozen_string_literal: false
 # Only used by Yahns::ProxyPass
 require 'kcar' # gem install kcar
 require 'kgio'
@@ -64,7 +64,7 @@ def yahns_step # yahns event loop entry point
 
       end while true # case @resbuf
 
-    when Array # [ (str|vec), rack.input, chunked? ]
+    when Array # [ str, rack.input, chunked? ]
       send_req_body(req) # returns nil or :wait_writable
     when String # buffered request header
       send_req_buf(req)
@@ -80,11 +80,14 @@ def yahns_step # yahns event loop entry point
   end
 
   def send_req_body_chunk(buf)
-    case rv = String === buf ? kgio_trywrite(buf) : kgio_trywritev(buf)
-    when String, Array
+    case rv = kgio_trywrite(buf)
+    when nil
+      return rv # done
+    when String
       buf.replace(rv) # retry loop on partial write
-    when :wait_writable, nil
+    when :wait_writable
       # :wait_writable = upstream is reading slowly and making us wait
+      @rrstate[0] = buf # for the next time we're called
       return rv
     else
       abort "BUG: #{rv.inspect} from kgio_trywrite*"
@@ -92,39 +95,33 @@ def send_req_body_chunk(buf)
   end
 
   # returns :wait_readable if complete, :wait_writable if not
-  def send_req_body(req) # @rrstate == [ (str|vec), rack.input, chunked? ]
+  def send_req_body(req) # @rrstate == [ str, rack.input, chunked? ]
     buf, input, chunked = req
 
     # send the first buffered chunk or vector
     rv = send_req_body_chunk(buf) and return rv # :wait_writable
 
     # yay, sent the first chunk, now read the body!
-    rbuf = buf
     if chunked
-      if String === buf # initial body
-        req[0] = buf = []
-      else
-        # try to reuse the biggest non-frozen buffer we just wrote;
-        rbuf = buf.max_by(&:size)
-        rbuf = ''.dup if rbuf.frozen? # unlikely...
+      # Note: input (env['rack.input']) is fully-buffered by default so
+      # we should not be waiting on a slow network resource when reading
+      # input.  However, some weird configs may disable this on LANs
+      # and we may wait indefinitely on input.read here...
+
+      rbuf = buf
+      while input.read(0x2000, rbuf)
+        buf = "#{rbuf.size.to_s(16)}\r\n#{rbuf}\r\n"
+        rbuf.clear
+        rv = send_req_body_chunk(buf) and return rv # :wait_writable
+        buf.clear
       end
-    end
-
-    # Note: input (env['rack.input']) is fully-buffered by default so
-    # we should not be waiting on a slow network resource when reading
-    # input.  However, some weird configs may disable this on LANs
-    # and we may wait indefinitely on input.read here...
-    while input.read(0x2000, rbuf)
-      if chunked
-        buf[0] = "#{rbuf.size.to_s(16)}\r\n".freeze
-        buf[1] = rbuf
-        buf[2] = "\r\n".freeze
+    else
+      while input.read(0x2000, buf)
+        rv = send_req_body_chunk(buf) and return rv # :wait_writable
       end
-      rv = send_req_body_chunk(buf) and return rv # :wait_writable
+      buf.clear # all done, clear the big buffer
     end
 
-    rbuf.clear # all done, clear the big buffer
-
     # we cannot use respond_to?(:close) here since Rack::Lint::InputWrapper
     # tries to prevent that (and hijack means all Rack specs go out the door)
     case input
diff --git a/lib/yahns/tmpio.rb b/lib/yahns/tmpio.rb
index 9e36d93..c14dc8a 100644
--- a/lib/yahns/tmpio.rb
+++ b/lib/yahns/tmpio.rb
@@ -8,7 +8,6 @@
 # well with unlinked files.  This one is much shorter, easier
 # to understand, and slightly faster (no delegation).
 class Yahns::TmpIO < File # :nodoc:
-  include Kgio::PipeMethods
 
   # creates and returns a new File object.  The File is unlinked
   # immediately, switched to binary mode, and userspace output
diff --git a/lib/yahns/wbuf.rb b/lib/yahns/wbuf.rb
index 3abc5f9..0eb7267 100644
--- a/lib/yahns/wbuf.rb
+++ b/lib/yahns/wbuf.rb
@@ -40,16 +40,11 @@ def initialize(body, persist)
     @busy = false
   end
 
-  def wbuf_writev(buf)
-    @tmpio.kgio_writev(buf)
-    buf.inject(0) { |n, s| n += s.size }
-  end
-
   def wbuf_write(c, buf)
     # try to bypass the VFS layer and write directly to the socket
     # if we're all caught up
-    case rv = String === buf ? c.kgio_trywrite(buf) : c.kgio_trywritev(buf)
-    when String, Array
+    case rv = c.kgio_trywrite(buf)
+    when String
       buf = rv # retry in loop
     when nil
       return # yay! hopefully we don't have to buffer again
@@ -59,7 +54,7 @@ def wbuf_write(c, buf)
 
     @tmpio ||= Yahns::TmpIO.new(c.class.output_buffer_tmpdir)
     # n.b.: we rely on O_APPEND in TmpIO, here
-    @sf_count += String === buf ? @tmpio.write(buf) : wbuf_writev(buf)
+    @sf_count += @tmpio.write(buf)
 
     # we spent some time copying to the FS, try to write to
     # the socket again in case some space opened up...
diff --git a/test/test_tmpio.rb b/test/test_tmpio.rb
index 3bcf3ca..3b2ca88 100644
--- a/test/test_tmpio.rb
+++ b/test/test_tmpio.rb
@@ -9,12 +9,10 @@ def setup
     skip 'sendfile missing' unless IO.instance_methods.include?(:sendfile)
   end
 
-  def test_writev
+  def test_write_and_sendfile
     a, b = UNIXSocket.pair
-    a.extend Kgio::PipeMethods
     tmpio = Yahns::TmpIO.new(Dir.tmpdir)
-    ary = [ "hello\n".freeze, "world\n".freeze ].freeze
-    tmpio.kgio_trywritev(ary)
+    tmpio.write("hello\nworld\n")
     a.trysendfile(tmpio, 0, 12)
     assert_equal "hello\nworld\n", b.read(12)
   ensure
-- 
EW


      parent reply index

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-06  3:30 [PATCH 0/2] WIP remove-kgio branch pushed branch Eric Wong
2017-03-06  3:30 ` [PATCH 1/2] remove kgio read and wait dependencies Eric Wong
2017-03-06  3:30 ` Eric Wong [this message]

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://yhbt.net/yahns/README

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170306033011.26938-3-yahns-public@yhbt.net \
    --to=yahns-public@yhbt.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

yahns Ruby server user/dev discussion

Archives are clonable:
	git clone --mirror https://yhbt.net/yahns-public
	git clone --mirror http://ou63pmih66umazou.onion/yahns-public

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.lang.ruby.yahns
	nntp://ou63pmih66umazou.onion/inbox.comp.lang.ruby.yahns

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox