Re: [PATCH] Add a `read' method for ports

[Ludovic Courtès]

Hi Neil,

"Neil Jerram" <address@hidden> writes:

> So, one way of solving this would be for an `unbuffered' port
> temporarily to get a buffer of the right size, and then to use the
> fill_input logic as it currently stands.  That would achieve your
> objective of mapping onto a single N-byte read(2) call.

Surely this would work, but I can think of several issues:

  1. Thread safety.  OK, ports are not thread-safe and it'd take more
     than this to make them thread-safe, but passing arguments
     explicitly (as the BUFFER and SIZE arguments of `read') is a step
     forward, while mutating the buffer pointer and size in the port is
     a step backward.

  2. Performance.  Another case where the `read' method improves on
     performance: when more data is requested than can fit in the port's
     buffer, the port buffer is completely bypassed.  This allows the
     data to be read in one shot, avoiding memcpys from the port's
     buffer to the caller's buffer.

  3. Aesthetics.  The `read' method looks more natural to me, since it's
     just like `read(2)'.  Also, as mentioned earlier, `read' methods
     don't have to be aware of the port's internal buffer: they just
     need to fill in the caller-supplied buffer.  When `scm_fill_input ()' 
     is called, it just invokes `read', passing it the port's internal
     buffer and using its return value to update `pt->read_end'.

The downside with the `read' method is that it requires changes in port
implementations to take advantage of it.

Overall, I'd prefer `read', at least for the longer term.  Changing
`pt->read_buf' et al. right before calling `fill_input ()' looks to me
like a hack to pass arguments implicitly.

> But that advantage is also (in some cases) the disadvantage of having
> to do an additional memcpy of the data.

There's no additional memcpy, as mentioned above.

> I don't think it's too bad for callers to be fiddling with read_buf -
> although we could encapsulate that a bit more clearly.

Indeed.  :-)

Thanks,
Ludovic.