Re: Unexpectedly low read/write performance of open-pipe

[Chris Vine]

On Tue, 09 Apr 2019 04:35:38 -0400
Mark H Weaver <address@hidden> wrote:
> Hi Rob,
> 
> Rob Browning <address@hidden> writes:
> 
> > Mark H Weaver <address@hidden> writes:
> >
> >> See below for a draft reimplementation of the OPEN_BOTH mode of
> >> open-pipe* based on R6RS custom binary input/output.  On my machine it
> >> increases the speed of your test by a factor of ~1k.
> >
> > Hah, I was about to report that I'd tested something along similar lines
> > (though much more a quick hack to just replace make-rw-port and see what
> > happened), and that I had seen substantial improvements:
> >
> >   (define (make-rw-bin-port read-port write-port)
> >     (define (read! dest offset count)
> >       (let ((result (get-bytevector-n! read-port dest offset count)))
> >         (if (eof-object? result) 0 result)))
> >     (define (write! src offset count)
> >       (put-bytevector write-port src offset count)
> >       count)
> >     (define (close x)
> >       (close-port read-port)
> >       (close-port write-port))
> >     (make-custom-binary-input/output-port "open-bin-pipe-port"
> >                                           read! write! #f #f
> >                                           close))
> 
> Hah, we had the same idea! :-)
> 
> FYI, the reason I didn't use 'get-bytevector-n!', although it leads to
> the much simpler code above, is that it has the wrong semantics
> w.r.t. blocking behavior.  'get-bytevector-n!' blocks until the entire
> requested count is read, returning less than the requested amount only
> if EOF is reached.
> 
> In this case, 'read!' is free to return less than 'count' bytes, and
> should block _only_ as needed to ensure that at least one byte is
> returned (or EOF).  Of course, an effort should be made to return
> additional bytes, and preferably a sizeable buffer, but only if it can
> be done without blocking unnecessarily.
> 
> Guile has only one I/O primitive capable of doing this job efficiently:
> 'get-bytevector-some'.  Internally, it works by simply returning the
> entire existing port read buffer if it's non-empty.  If the read buffer
> is empty, then read(2) is used to refill the read buffer, which is then
> returned to the user.
> 
> The reason for the extra complexity in my reimplementation is that
> there's no way to specify an upper bound on the size of the bytevector
> returned by 'get-bytevector-some', so in general we may need to preserve
> the remaining bytes more future 'read!' calls.
> 
> >> Let me know how it works for you.
> >
> > For a first quick test of your patch using the original program I was
> > working on, I see about ~1.4MiB/s without the patch, and about 150MiB/s
> > with it, measured by pv.
> 
> It's interesting that I saw a much larger improvement than you're
> seeing.  In my case, the numbers reported by your test program went from
> ~0.35 mb/s to ~333 mb/s, on a Thinkpad X200.
> 
> > (If the patch holds up, it'd be nice to have in 2.2, but I suppose that
> >  might not be appropriate.)
> 
> I think it's probably fine for 2.2, although a more careful check should
> be made for differences in behavior between the old and new
> implementations, and tests should be added.  I'll try to get to it soon.

If it is going in 2.2 (or 3.0) it would be nice if the ports could be
suspendable.  put-bytevector (used by write!) is suspendable;
get-bytevector-some (used by read!) is not.

Chris