socketry / async-io

Concurrent wrappers for native Ruby IO & Sockets.
MIT License
208 stars 28 forks source link

Improve TCPSocket buffering #11

Closed ioquatix closed 5 years ago

ioquatix commented 6 years ago

As outlined in https://github.com/socketry/async-io/pull/10, there are issues with TCPSocket#gets, #puts, #read and #write.

ioquatix commented 6 years ago

@jjyr Here is something which seems to work, but I think we need more specs to cover the behaviour of TCPSocket, and specifically documentation explaining the situation https://github.com/socketry/async-io/pull/10#issuecomment-421762652 so that people generally avoid using TCPSocket unless required.

jjyr commented 6 years ago

I believe we can reuse some test cases from https://github.com/socketry/lightio/blob/master/spec/lightio/library/io_spec.rb#L179

ioquatix commented 6 years ago

That makes sense, in the end, I'm not entirely sure how, say, #syswrite, #sync=, #write, and #puts interact with each other. It seems like the only option is to make specs to ensure the behaviour matches up with ::TCPSocket.

jjyr commented 6 years ago

So I think maybe we should keep the IO interface simple, to offer some basic IO operations like: #write #read #gets #puts is enough, is hard to maintain compatible in so many related IO operations( for example, how to implement the readchar method?).

By the way, we have no reason to maintain 100% compatible if users should aware the difference between Async::IO and std IO(or shouldn't they?).

ioquatix commented 6 years ago

These are all good points and things I have thought about.

The point of Async::IO::TCPSocket and ...::TCPServer was to be drop in (injectable) replacements to their native equivalents. The API should be identical in this respect. You are right, it's almost impossible to implement it, the surface area is huge.

Additionally, I'm not sure anyone could possibly use some of those methods to implement any kind of efficient IO with Ruby. So, some of them are just plain irrelevant.

Async::IO is not perfect and I think I expected some level of issues like this. The point was to have a good API: Socket/Stream/Endpoint and a drop-in compatible API (e.g. TCPSocket/UDPSocket).

When I was making Async::IO, I had a happy dream to ignore TCPSocket and related classes. It would be nice to simply remove it. But I think it's a good proof of concept and the actual code... isn't too complex/long. But as discussed earlier, it might be confusing to new users - which API to use?

I feel like as long as we can support the main parts of the API, it's a useful starting point, and a useful testing implementation (e.g. injecting into Net::HTTP).

Long term though, I'm not even sure if the design for Async::IO is good. What I mean by that, is that the design of Async::IO was limited by the design of Ruby's existing IO for which I wanted to provide an (async) interface. So, Async::IO is limited to some of the design choices of Ruby's native IO implementation. That's by design, but it's a limitation none the less. It affects the API and the separation of concerns. I tried to add some things which I think make sense (e.g. Async::IO::Stream) but as you can clearly see it's tricky.

I want to see the direction Ruby 3.0 goes in before I make any major changes. I suspect that Async::IO will always try to provide 1-1 wrappers for ::IO and that's basically the extent of it.

Longer term, I think I'd like to explore a new model for IO, including using FFI to directly access POSIX sockets/file descriptor methods. This would work better for JRuby too which currently has a very poor implementation of Ruby IO. But I can't really go down this path yet.

ioquatix commented 6 years ago

I made some minor compatibility improvements https://github.com/socketry/async-io/commit/de51e2b7a95d7cafcb9e7af25e094635892556d1

chuckremes commented 6 years ago

Longer term, I think I'd like to explore a new model for IO, including using FFI to directly access POSIX sockets/file descriptor methods. This would work better for JRuby too which currently has a very poor implementation of Ruby IO. But I can't really go down this path yet.

I like the sound of that. :)

ioquatix commented 5 years ago

I believe these issues are all fixed, that being said the implementation could probably be improved.