async: puts '' inside Async block causes crash

I merged a fix in https://bugs.ruby-lang.org/issues/19640

I will remove the io_write hook in Async on versions of Ruby where it causes a crash.

Work went into the 6.4 kernel to improve this situation a bit, most notably this merge:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b9dff2195f8a5847fad801046b26955e05670d31

which converts single segment imports into ITER_UBUF, and Linus himself authored some commits improving the import speed of iovecs in general. Most for x86, but also this generic one:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=487c20b016dc48230367a7be017f40313e53e3bd

Doesn’t change the advice I gave above, but it does reduce the overhead on the kernel side a bit in 6.4+ for that particular use case.

Just the iovec itself, not the data. io_uring has to ensure that any “meta data” related to the IO is stable beyond submit, so it has to copy in and store the vec as it cannot reimport it safely later on without violating that requirement.

writev is always worse than write, regardless of whether you use io_uring or not. writev needs to copy an iovec, which write does not. Internally in the kernel, outside of the copy overhead, this means that (until recently) it would also use an ITER_IOVEC when passed down the stack, which are more expensive to advance and iterate.

For io_uring specifically, a vectored write also means that io_uring now has to maintain that state. If things complete inline (eg without needing poll deferral), the cost overhead is the same as writev(2) in that it has to do that extra copy. If io_uring does not need defer the request, then it now has to allocate memory to store this iovec state, and free it at the end. This all adds up to more overhead.

IOW - don’t use vectored IO, unless you have to. Lots of applications seem to happily just use vectored IO by default even if they are just using a single segment, this is a bad idea. Obviously there are cases where you’d want to use single segment vectored IO, for things like sendmsg() for example. But if that’s not the case, don’t do it.

I think I found the issue.

Haha that’s super weird lol, after RubyKaigi this week, I will take a look.

The semantics of writev are (unexpectedly) more complex than write.

I heard some implementations of writev just make a new buffer, copy everything together, and call write. So some developer may overestimate the value of writev. I asked @axboe about this and he mentioned, IIRC, that writev is worse than write when using io_uring. However, I’m sure it’s difficult problem to generalise about.

In any case, I agree “good god does a lot go into a single call of rb_io_puts” and perhaps we should try to simplify the code - we could consider removing writev.

Ahh, I see. Now that I look at the patch, and reread io_binwritev_internal as well, I understand why the loop wasn’t necessary, but good god does a lot go into a single call of rb_io_puts.

Okay, yeah. That’s an oops. I assume you’re talking about the loop.

Edit:

Assuming my understanding of this section is correct:

--- work/ruby-heads-master/io.c	2023-05-13 02:59:15
+++ io.c	2023-05-13 11:27:31
@@ -1329,13 +1329,15 @@
 {
     VALUE scheduler = rb_fiber_scheduler_current();
     if (scheduler != Qnil) {
+        ssize_t retval = 0;
         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);
 
             if (!UNDEF_P(result)) {
-                return rb_fiber_scheduler_io_result_apply(result);
+                retval += rb_fiber_scheduler_io_result_apply(result);
             }
         }
+        return retval;
     }
 
     struct io_internal_writev_struct iis = {

Upstream’s responsibility though.

I believe that’s because nil coerces to "". You can also crash it with:

require 'async'

class Foo
  def to_s
    ""
  end
end

tmp = Foo.new

Async { puts tmp }

puts nil also cause crash.