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

[Mark H Weaver]

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.

      Regards,
        Mark