ruby-changes:70303
From: Samuel <ko1@a...>
Date: Sat, 18 Dec 2021 14:19:45 +0900 (JST)
Subject: [ruby-changes:70303] 42d3231154 (master): Introduce io_result wrapper for passing `[-errno, size]` in VALUE.
https://git.ruby-lang.org/ruby.git/commit/?id=42d3231154 From 42d32311541e58503b885b09b469948922650c66 Mon Sep 17 00:00:00 2001 From: Samuel Williams <samuel.williams@o...> Date: Sat, 18 Dec 2021 18:19:30 +1300 Subject: Introduce io_result wrapper for passing `[-errno, size]` in VALUE. --- include/ruby/fiber/scheduler.h | 54 +++++++++++++++++- io.c | 121 +++++++++++++++++++---------------------- test/fiber/scheduler.rb | 82 ++++++++++++++++++++++++++++ test/fiber/test_io.rb | 32 +++++++++++ test/fiber/test_io_buffer.rb | 114 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 336 insertions(+), 67 deletions(-) create mode 100644 test/fiber/test_io_buffer.rb diff --git a/include/ruby/fiber/scheduler.h b/include/ruby/fiber/scheduler.h index 82944422164..ff587e28c09 100644 --- a/include/ruby/fiber/scheduler.h +++ b/include/ruby/fiber/scheduler.h @@ -11,17 +11,67 @@ https://github.com/ruby/ruby/blob/trunk/include/ruby/fiber/scheduler.h#L11 */ #include "ruby/internal/config.h" +#include <errno.h> + #ifdef STDC_HEADERS #include <stddef.h> /* size_t */ #endif #include "ruby/ruby.h" #include "ruby/internal/dllexport.h" +#include "ruby/internal/arithmetic.h" RBIMPL_SYMBOL_EXPORT_BEGIN() struct timeval; +/** + * Wrap a `ssize_t` and `int errno` into a single `VALUE`. This interface should + * be used to safely capture results from system calls like `read` and `write`. + * + * You should use `rb_fiber_scheduler_io_result_apply` to unpack the result of + * this value and update `int errno`. + * + * You should not directly try to interpret the result value as it is considered + * an opaque representation. However, the general representation is an integer + * in the range of `[-int errno, size_t size]`. Linux generally restricts the + * result of system calls like `read` and `write` to `<= 2^31` which means this + * will typically fit within a single FIXNUM. + * + * @param[in] result The result of the system call. + * @param[in] error The value of `errno`. + * @return A `VALUE` which contains the result and/or errno. + */ +static inline VALUE +rb_fiber_scheduler_io_result(ssize_t result, int error) { + if (result == -1) { + return RB_INT2NUM(-error); + } else { + return RB_SIZE2NUM(result); + } +} + +/** + * Apply an io result to the local thread, returning the value of the orginal + * system call that created it and updating `int errno`. + * + * You should not directly try to interpret the result value as it is considered + * an opaque representation. + * + * @param[in] result The `VALUE` which contains an errno and/or result size. + * @post Updates `int errno` with the value if negative. + * @return The original result of the system call. + */ +static inline ssize_t +rb_fiber_scheduler_io_result_apply(VALUE result) { + if (RB_FIXNUM_P(result) && RB_NUM2INT(result) < 0) { + errno = -RB_NUM2INT(result); + return -1; + } else { + return RB_NUM2SIZE(result); + } +} + /** * Queries the current scheduler of the current thread that is calling this * function. @@ -195,7 +245,7 @@ VALUE rb_fiber_scheduler_io_wait_writable(VALUE scheduler, VALUE io); https://github.com/ruby/ruby/blob/trunk/include/ruby/fiber/scheduler.h#L245 * @param[out] buffer Return buffer. * @param[in] length Requested number of bytes to read. * @retval RUBY_Qundef `scheduler` doesn't have `#io_read`. - * @return otherwise What `scheduler.io_read` returns. + * @return otherwise What `scheduler.io_read` returns `[-errno, size]`. */ VALUE rb_fiber_scheduler_io_read(VALUE scheduler, VALUE io, VALUE buffer, size_t length); @@ -207,7 +257,7 @@ VALUE rb_fiber_scheduler_io_read(VALUE scheduler, VALUE io, VALUE buffer, size_t https://github.com/ruby/ruby/blob/trunk/include/ruby/fiber/scheduler.h#L257 * @param[in] buffer What to write. * @param[in] length Number of bytes to write. * @retval RUBY_Qundef `scheduler` doesn't have `#io_write`. - * @return otherwise What `scheduler.io_write` returns. + * @return otherwise What `scheduler.io_write` returns `[-errno, size]`. */ VALUE rb_fiber_scheduler_io_write(VALUE scheduler, VALUE io, VALUE buffer, size_t length); diff --git a/io.c b/io.c index bc64938648b..7ec63fcf714 100644 --- a/io.c +++ b/io.c @@ -1138,14 +1138,14 @@ rb_read_internal(rb_io_t *fptr, void *buf, size_t count) https://github.com/ruby/ruby/blob/trunk/io.c#L1138 { VALUE scheduler = rb_fiber_scheduler_current(); if (scheduler != Qnil) { - VALUE result = rb_fiber_scheduler_io_read_memory(scheduler, fptr->self, buf, count, 1); + VALUE result = rb_fiber_scheduler_io_read_memory(scheduler, fptr->self, buf, count, count); if (result != Qundef) { - ssize_t length = RB_NUM2SSIZE(result); + ssize_t length = rb_fiber_scheduler_io_result_apply(result); - if (length < 0) rb_sys_fail_path(fptr->pathv); + if (length < 0) rb_sys_fail_path(fptr->pathv); - return length; + return length; } } @@ -1165,14 +1165,10 @@ rb_write_internal(rb_io_t *fptr, const void *buf, size_t count) https://github.com/ruby/ruby/blob/trunk/io.c#L1165 { VALUE scheduler = rb_fiber_scheduler_current(); if (scheduler != Qnil) { - VALUE result = rb_fiber_scheduler_io_write_memory(scheduler, fptr->self, buf, count, count); + VALUE result = rb_fiber_scheduler_io_write_memory(scheduler, fptr->self, buf, count, 0); if (result != Qundef) { - ssize_t length = RB_NUM2SSIZE(result); - - if (length < 0) rb_sys_fail_path(fptr->pathv); - - return length; + return rb_fiber_scheduler_io_result_apply(result); } } @@ -1182,33 +1178,34 @@ rb_write_internal(rb_io_t *fptr, const void *buf, size_t count) https://github.com/ruby/ruby/blob/trunk/io.c#L1178 .capa = count }; - return (ssize_t)rb_thread_io_blocking_region(internal_write_func, &iis, fptr->fd); + if (fptr->write_lock && rb_mutex_owned_p(fptr->write_lock)) + return (ssize_t)rb_thread_call_without_gvl2(internal_write_func2, &iis, RUBY_UBF_IO, NULL); + else + return (ssize_t)rb_thread_io_blocking_region(internal_write_func, &iis, fptr->fd); } +#ifdef HAVE_WRITEV static ssize_t -rb_write_internal2(rb_io_t *fptr, const void *buf, size_t count) +rb_writev_internal(rb_io_t *fptr, const struct iovec *iov, int iovcnt) { - struct io_internal_write_struct iis = { - .fd = fptr->fd, - .buf = buf, - .capa = count - }; + VALUE scheduler = rb_fiber_scheduler_current(); + if (scheduler != Qnil) { + for (int i = 0; i < iovcnt; i += 1) { + VALUE result = rb_fiber_scheduler_io_write_memory(scheduler, fptr->self, iov[i].iov_base, iov[i].iov_len, 0); - return (ssize_t)rb_thread_call_without_gvl2(internal_write_func2, &iis, - RUBY_UBF_IO, NULL); -} + if (result != Qundef) { + return rb_fiber_scheduler_io_result_apply(result); + } + } + } -#ifdef HAVE_WRITEV -static ssize_t -rb_writev_internal(int fd, const struct iovec *iov, int iovcnt) -{ struct io_internal_writev_struct iis = { - .fd = fd, + .fd = fptr->fd, .iov = iov, .iovcnt = iovcnt, }; - return (ssize_t)rb_thread_io_blocking_region(internal_writev_func, &iis, fd); + return (ssize_t)rb_thread_io_blocking_region(internal_writev_func, &iis, fptr->fd); } #endif @@ -1592,7 +1589,7 @@ io_binwrite_string(VALUE arg) https://github.com/ruby/ruby/blob/trunk/io.c#L1589 iov[1].iov_base = (char *)p->ptr; iov[1].iov_len = p->length; - r = rb_writev_internal(fptr->fd, iov, 2); + r = rb_writev_internal(fptr, iov, 2); if (r < 0) return r; @@ -1654,56 +1651,49 @@ io_binwrite(VALUE str, const char *ptr, long len, rb_io_t *fptr, int nosync) https://github.com/ruby/ruby/blob/trunk/io.c#L1651 if ((n = len) <= 0) return n; - VALUE scheduler = rb_fiber_scheduler_current(); - if (scheduler != Qnil) { - VALUE result = rb_fiber_scheduler_io_write_memory(scheduler, fptr->self, ptr, len, len); - - if (result != Qundef) { - ssize_t length = RB_NUM2SSIZE(result); - - if (length < 0) rb_sys_fail_path(fptr->pathv); - - return length; - } - } - if (fptr->wbuf.ptr == NULL && !(!nosync && (fptr->mode & FMODE_SYNC))) { fptr->wbuf.off = 0; fptr->wbuf.len = 0; fptr->wbuf.capa = IO_WBUF_CAPA_MIN; fptr->wbuf.ptr = ALLOC_N(char, fptr->wbuf.capa); fptr->write_lock = rb_mutex_new(); - rb_mutex_allow_trap(fptr->write_lock, 1); + rb_mutex_allow_trap(fptr->write_lock, 1); } + if ((!nosync && (fptr->mode & (FMODE_SYNC|FMODE_TTY))) || (fptr->wbuf.ptr && fptr->wbuf.capa <= fptr->wbuf.len + len)) { - struct binwrite_arg arg; + struct binwrite_arg arg; - arg.fptr = fptr; - arg.str = str; + arg.fptr = fptr; + arg.str = str; retry: - arg.ptr = ptr + offset; - arg.length = n; - if (fptr->write_lock) { + arg.ptr = ptr + offset; + arg.length = n; + + if (fptr->write_lock) { r = rb_mutex_synchronize(fptr->write_lock, io_binwrite_string, (VALUE)&arg); - } - else { - r = io_binwrite_string((VALUE)&arg); - } - /* xxx: other threads may modify given string. */ + } + else { + r = io_binwrite_string((VALUE)&arg); + } + + /* xxx: other threads may modify given string. */ if (r == n) return len; if (0 <= r) { offset += r; n -= r; errno = EAGAIN; - } - if (r == -2L) - return -1L; + } + + if (r == -2L) + return -1L; if (rb_io_maybe_wait_writable(errno, fptr->self, Qnil)) { (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/