dumping ground for random patches and texts
 help / color / mirror / Atom feed
From: Eric Wong <e@80x24.org>
To: spew@80x24.org
Subject: [PATCH] io.c: avoid kwarg parsing in C API
Date: Tue,  7 Jul 2015 21:57:20 +0000	[thread overview]
Message-ID: <1436306240-27684-1-git-send-email-e@80x24.org> (raw)

rb_scan_args and hash lookups for kwargs in the C API are clumsy and
slow.  Instead of improving the C API for performance, use Ruby
instead :)

Implement IO#read_nonblock and IO#write_nonblock in prelude.rb
to avoid argument parsing via rb_scan_args and hash lookups.

This speeds up IO#write_nonblock and IO#read_nonblock benchmarks
in both cases, including the original non-idiomatic case where
the `exception: false' hash is pre-allocated to avoid GC pressure.

Now, writing the kwargs in natural, idiomatic Ruby is fastest.
I've added the noex2 benchmark to show this.

target 0: a (ruby 2.3.0dev (2015-07-08 trunk 51190) [x86_64-linux]) at "a/ruby"
target 1: b (ruby 2.3.0dev (2015-07-08 nonblock-kwarg 51190) [x86_64-linux]) at "b/ruby"
-----------------------------------------------------------
raw data:

[["io_nonblock_noex",
  [[2.5436805468052626, 2.5724728293716908, 2.4915440678596497],
   [2.478000810369849, 2.4285155069082975, 2.462410459294915]]],
 ["io_nonblock_noex2",
  [[3.012514788657427, 3.034533655270934, 2.9972082190215588],
   [2.135501991957426, 2.146781364455819, 2.0429874528199434]]]]

Elapsed time: 30.348340944 (sec)
-----------------------------------------------------------
benchmark results:
minimum results in each 3 measurements.
Execution time (sec)
name    a       b
io_nonblock_noex        2.492   2.429
io_nonblock_noex2       2.997   2.043

Speedup ratio: compare with the result of `a' (greater is better)
name    b
io_nonblock_noex        1.026
io_nonblock_noex2       1.467

Note: I plan to followup commits for other *_nonblock methods
Eventually, I even wish to deprecate rb_scan_args :D

For what it's worth, I'm more excited about this change than usual
and hope to use prelude.rb more.
---
 benchmark/bm_io_nonblock_noex2.rb |  21 ++++++++
 io.c                              | 109 ++++++++++++++++++++++----------------
 prelude.rb                        |  10 ++++
 3 files changed, 94 insertions(+), 46 deletions(-)
 create mode 100644 benchmark/bm_io_nonblock_noex2.rb

diff --git a/benchmark/bm_io_nonblock_noex2.rb b/benchmark/bm_io_nonblock_noex2.rb
new file mode 100644
index 0000000..56819d0
--- /dev/null
+++ b/benchmark/bm_io_nonblock_noex2.rb
@@ -0,0 +1,21 @@
+nr = 1_000_000
+i = 0
+msg = '.'
+buf = '.'
+begin
+  r, w = IO.pipe
+  while i < nr
+    i += 1
+    w.write_nonblock(msg, exception: false)
+    r.read_nonblock(1, buf, exception: false)
+  end
+rescue ArgumentError # old Rubies
+  while i < nr
+    i += 1
+    w.write_nonblock(msg)
+    r.read_nonblock(1, buf)
+  end
+ensure
+  r.close
+  w.close
+end
diff --git a/io.c b/io.c
index 57ddbb7..5966ee1 100644
--- a/io.c
+++ b/io.c
@@ -2671,55 +2671,49 @@ io_readpartial(int argc, VALUE *argv, VALUE io)
  */
 
 static VALUE
-io_read_nonblock(int argc, VALUE *argv, VALUE io)
-{
-    VALUE ret, opts;
-
-    rb_scan_args(argc, argv, "11:", NULL, NULL, &opts);
-
-    ret = io_getpartial(argc, argv, io, opts, 1);
-
-    if (NIL_P(ret)) {
-	if (no_exception_p(opts))
-	    return Qnil;
-	else
-	    rb_eof_error();
-    }
-    return ret;
-}
-
-static VALUE
-io_write_nonblock(VALUE io, VALUE str, VALUE opts)
+io_read_nonblock(VALUE io, VALUE length, VALUE str, VALUE ex)
 {
     rb_io_t *fptr;
-    long n;
+    long n, len;
+    struct read_internal_arg arg;
 
-    if (!RB_TYPE_P(str, T_STRING))
-	str = rb_obj_as_string(str);
+    if ((len = NUM2LONG(length)) < 0) {
+	rb_raise(rb_eArgError, "negative length %ld given", len);
+    }
 
-    io = GetWriteIO(io);
+    io_setstrbuf(&str,len);
+    OBJ_TAINT(str);
     GetOpenFile(io, fptr);
-    rb_io_check_writable(fptr);
+    rb_io_check_byte_readable(fptr);
 
-    if (io_fflush(fptr) < 0)
-        rb_sys_fail(0);
+    if (len == 0)
+	return str;
 
-    rb_io_set_nonblock(fptr);
-    n = write(fptr->fd, RSTRING_PTR(str), RSTRING_LEN(str));
+    n = read_buffered_data(RSTRING_PTR(str), len, fptr);
+    if (n <= 0) {
+	rb_io_set_nonblock(fptr);
+	io_setstrbuf(&str, len);
+	arg.fd = fptr->fd;
+	arg.str_ptr = RSTRING_PTR(str);
+	arg.len = len;
+	rb_str_locktmp_ensure(str, read_internal_call, (VALUE)&arg);
+	n = arg.len;
+        if (n < 0) {
+            if ((errno == EWOULDBLOCK || errno == EAGAIN)) {
+                if (ex == Qfalse) return sym_wait_readable;
+                rb_readwrite_sys_fail(RB_IO_WAIT_READABLE, "read would block");
+            }
+            rb_sys_fail_path(fptr->pathv);
+        }
+    }
+    io_set_read_length(str, n);
 
-    if (n == -1) {
-        if (errno == EWOULDBLOCK || errno == EAGAIN) {
-	    if (no_exception_p(opts)) {
-		return sym_wait_writable;
-	    }
-	    else {
-		rb_readwrite_sys_fail(RB_IO_WAIT_WRITABLE, "write would block");
-	    }
-	}
-        rb_sys_fail_path(fptr->pathv);
+    if (n == 0) {
+	if (ex == Qfalse) return Qnil;
+	rb_eof_error();
     }
 
-    return LONG2FIX(n);
+    return str;
 }
 
 /*
@@ -2779,15 +2773,38 @@ io_write_nonblock(VALUE io, VALUE str, VALUE opts)
  *  return the symbol :wait_writable instead.
  *
  */
-
 static VALUE
-rb_io_write_nonblock(int argc, VALUE *argv, VALUE io)
+io_write_nonblock(VALUE io, VALUE str, VALUE ex)
 {
-    VALUE str, opts;
+    rb_io_t *fptr;
+    long n;
 
-    rb_scan_args(argc, argv, "10:", &str, &opts);
+    if (!RB_TYPE_P(str, T_STRING))
+	str = rb_obj_as_string(str);
 
-    return io_write_nonblock(io, str, opts);
+    io = GetWriteIO(io);
+    GetOpenFile(io, fptr);
+    rb_io_check_writable(fptr);
+
+    if (io_fflush(fptr) < 0)
+        rb_sys_fail(0);
+
+    rb_io_set_nonblock(fptr);
+    n = write(fptr->fd, RSTRING_PTR(str), RSTRING_LEN(str));
+
+    if (n == -1) {
+        if (errno == EWOULDBLOCK || errno == EAGAIN) {
+	    if (ex == Qfalse) {
+		return sym_wait_writable;
+	    }
+	    else {
+		rb_readwrite_sys_fail(RB_IO_WAIT_WRITABLE, "write would block");
+	    }
+	}
+        rb_sys_fail_path(fptr->pathv);
+    }
+
+    return LONG2FIX(n);
 }
 
 /*
@@ -12246,8 +12263,8 @@ Init_IO(void)
 
     rb_define_method(rb_cIO, "readlines",  rb_io_readlines, -1);
 
-    rb_define_method(rb_cIO, "read_nonblock",  io_read_nonblock, -1);
-    rb_define_method(rb_cIO, "write_nonblock", rb_io_write_nonblock, -1);
+    rb_define_private_method(rb_cIO, "__read_nonblock", io_read_nonblock, 3);
+    rb_define_private_method(rb_cIO, "__write_nonblock", io_write_nonblock, 2);
     rb_define_method(rb_cIO, "readpartial",  io_readpartial, -1);
     rb_define_method(rb_cIO, "read",  io_read, -1);
     rb_define_method(rb_cIO, "write", io_write_m, 1);
diff --git a/prelude.rb b/prelude.rb
index cc24a81..1a2e41b 100644
--- a/prelude.rb
+++ b/prelude.rb
@@ -13,3 +13,13 @@ class Thread
     }
   end
 end
+
+class IO
+  def read_nonblock(len, buf = nil, exception: true)
+    __read_nonblock(len, buf, exception)
+  end
+
+  def write_nonblock(buf, exception: true)
+    __write_nonblock(buf, exception)
+  end
+end
-- 
EW


             reply	other threads:[~2015-07-07 21:57 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-07 21:57 Eric Wong [this message]
  -- strict thread matches above, loose matches on Subject: below --
2015-11-12  1:51 [PATCH] io.c: avoid kwarg parsing in C API Eric Wong

Reply instructions:

You may reply publicly 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

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

  git send-email \
    --in-reply-to=1436306240-27684-1-git-send-email-e@80x24.org \
    --to=e@80x24.org \
    --cc=spew@80x24.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).