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
next 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).