make byte order types IO wrappers

[simonbyrne]

Converts the ByteOrder types introduced in #3633 to IO wrappers.

Works quite well, requiring no changes for existing read/write methods.

However, I seem to be getting some ambiguous dispatch errors that I can't seem to fix,

[StefanKarpinski]

I'm much, much happier with this API. cc: @JeffBezanson

[simonbyrne]

Ah, I think it's because I'm missing some Bool and Char definitions. Is there an easier way to specify these without enumerating all combinations?

Also, does Char behave the same on both big and little-endian architectures?

[StefanKarpinski]

Char is a weird case since writing it generally means encoding something as a string, so you generally don't want to actually write Chars. What you do want to do is make sure that string data for which byte ordering is meaningful is ordered correctly. That doesn't mean anything for UTF-8 since it's byte-oriented, but for UTF-16 and UTF-32 have both big and little endian variants.

[timholy]

I agree that it's better associated with the stream. Good observation, @StefanKarpinski.

[kmsquire]

Thoughts:

• In the tests right now, each read and write are creating a new ByteOrderedIO object.
• If you create one object at the beginning and continue to reference it, you still may need to pass the wrapped IO instance to some functions
• This will probably break with zlib compressed io, since zlib already handles endian conversion. The only place this matters right now is with the GZip.jl, where GZipStream <: IO.

[simonbyrne]

Okay, I've added the extra methods, though the Char methods might be wrong. I've also changed the new types to be immutable, but this now causes a segmentation error in test-spawn.

[JeffBezanson]

I'm starting to wonder whether we should do all I/O through a type like IOBuffer, with a low-level API for concrete streams that only uses unix-style read/write of chunks of bytes. Similar to @kmsquire 's BufferedReader, which I initially dismissed, but if we're going to start wrapping streams we might be better off going all the way. In particular, we would be able to implement features like byte order, different text encodings, mark/seekmark etc. very efficiently against a known representation (uint8 array), and only once. The low-level API would only consist of a way to fill and consume the buffer (and would probably be quite distinct from the user-level API). This could eliminate some confusion about what to define, and avoid the performance pitfall we currently have where everything is ultimately defined in terms of reading single bytes. I'm worried about ending up with a bunch of I/O wrappers, like BufferedReader{NetworkByteOrder{MyStream}}.

[kmsquire]

Just to note, the BufferedReader pull request (which is still open #3656) doesn't actually have a BufferedReader any more, and instead just modifies IOBuffer and IOStream. IOBuffer is already used by UV streams in a number of places.

I agree that that's probably the way to go, with one caveat that the current IOBuffer implementation moves around data too much. A better implementation would probably be a ring buffer or bucket brigade (I like that name!), although they're somewhat more complicated.

[simonbyrne]

That's strange, the tests seem to now be passing on my machine, but Travis has hit a different error.

[vtjnash]

To take @JeffBezanson's idea in a slightly different direction, what if the I/O wrappers were implemented as functions instead of types?

type FilteredIO
    io::IO
    rfilters::Array{Function,1}
    wfilters::Array{Function,1}
    FilteredIO(io) = new(io, Function[], Function[])
end
write(io::FilteredIO, x) = write(io.io, reduce((x,f)->f(x), x, io.wfilters)

However, it's not immediately clear to me how one would implement reading in this scheme.
read(io::FilteredIO, T) = reduce((x,f)->f(x), read(io.io, T), io.rfilters) is not correct.

read(io::FilteredIO, T) = io.rfilters[1](io.rfilters[2:], io, T) might be closer?

[JeffBezanson]

see #3887