{"action":"created","issue":{"url":"https://api.github.com/repos/tokio-rs/tokio-core/issues/35","repository_url":"https://api.github.com/repos/tokio-rs/tokio-core","labels_url":"https://api.github.com/repos/tokio-rs/tokio-core/issues/35/labels{/name}","comments_url":"https://api.github.com/repos/tokio-rs/tokio-core/issues/35/comments","events_url":"https://api.github.com/repos/tokio-rs/tokio-core/issues/35/events","html_url":"https://github.com/tokio-rs/tokio-core/pull/35","id":177221345,"number":35,"title":"Io streams","user":{"login":"rrichardson","id":124171,"avatar_url":"https://avatars.githubusercontent.com/u/124171?v=3","gravatar_id":"","url":"https://api.github.com/users/rrichardson","html_url":"https://github.com/rrichardson","followers_url":"https://api.github.com/users/rrichardson/followers","following_url":"https://api.github.com/users/rrichardson/following{/other_user}","gists_url":"https://api.github.com/users/rrichardson/gists{/gist_id}","starred_url":"https://api.github.com/users/rrichardson/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rrichardson/subscriptions","organizations_url":"https://api.github.com/users/rrichardson/orgs","repos_url":"https://api.github.com/users/rrichardson/repos","events_url":"https://api.github.com/users/rrichardson/events{/privacy}","received_events_url":"https://api.github.com/users/rrichardson/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":13,"created_at":"2016-09-15T16:16:10Z","updated_at":"2016-09-29T14:32:30Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/tokio-rs/tokio-core/pulls/35","html_url":"https://github.com/tokio-rs/tokio-core/pull/35","diff_url":"https://github.com/tokio-rs/tokio-core/pull/35.diff","patch_url":"https://github.com/tokio-rs/tokio-core/pull/35.patch"},"body":"OK.  There is a lot going on here, I doubt that the code is in the state where it can be merged, but it makes for decent talking points. \r\n\r\nA couple things:  \r\n**1. Pools and Contiguous memory:**\r\nI don't think we should attempt to guarantee contiguous memory between reads of sockets in the stream.  If someone has that specialized of a need, which seems unlikely, they can implement their own buffer pool to do so.  We could probably add a stream-id parameter to BufferPool::get to allow the pool to try to ensure contiguous pools.  \r\n\r\n**2.  Naming.**  \r\nMy recommendation is that we change the name of tokio-core::net::TcpStream to TcpSocket.  There isn't really anything special about it that makes it a stream anyways. (I know, probably a long shot for many reasons. )\r\nThat aside, I have put the futures::streams implementation into ``` net::stream::{Tcp, Udp} ``` \r\nThat seemed like a decent spot for them from a namespace point of view. \r\n\r\n**3. Read to completion.**  \r\nThis is something I'm not entirely clear on.  If we don't explicitly say that there isn't any data ready for an io source, will we attempt to read from it at the next poller tick?  I was taking the presence of calls to io.need_read(); to indicate that if that isn't set explicitly, Io::poll_read would continue to indicate that data is waiting.  \r\n\r\n This influences the design pretty heavily because if we have to read to completion for a very fast moving data stream,  there are two issues :  \r\n1. We'll need to be able to return arrays of buffers from stream::poll(). \r\n2. If we continue to read and produce buffers in a loop, we can starve the other futures. \r\n\r\nI implemented poll for both Tcp and Udp under the optimistic assumption that we can fill only a single buffer at a time and come back to it at the next poll tick. \r\n\r\n"},"comment":{"url":"https://api.github.com/repos/tokio-rs/tokio-core/issues/comments/250483709","html_url":"https://github.com/tokio-rs/tokio-core/pull/35#issuecomment-250483709","issue_url":"https://api.github.com/repos/tokio-rs/tokio-core/issues/35","id":250483709,"user":{"login":"rrichardson","id":124171,"avatar_url":"https://avatars.githubusercontent.com/u/124171?v=3","gravatar_id":"","url":"https://api.github.com/users/rrichardson","html_url":"https://github.com/rrichardson","followers_url":"https://api.github.com/users/rrichardson/followers","following_url":"https://api.github.com/users/rrichardson/following{/other_user}","gists_url":"https://api.github.com/users/rrichardson/gists{/gist_id}","starred_url":"https://api.github.com/users/rrichardson/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rrichardson/subscriptions","organizations_url":"https://api.github.com/users/rrichardson/orgs","repos_url":"https://api.github.com/users/rrichardson/repos","events_url":"https://api.github.com/users/rrichardson/events{/privacy}","received_events_url":"https://api.github.com/users/rrichardson/received_events","type":"User","site_admin":false},"created_at":"2016-09-29T14:32:30Z","updated_at":"2016-09-29T14:32:30Z","body":"I have to disagree here and say that contiguous buffers are not generally\nachievable in practice.  If you need contiguous buffers,  I'd recommend\nreading requests into smaller buffers, then copying the payloads into\nsecondary buffers.   This will likely end up being more efficient than\ntrying to manage the provision of slabs in sequential order, or allocating\nvery large chunks.\nYou would be using the secondary buffers less frequently, and if you pick a\npower of 2 as the buffer size, and always use that, then any allocator\nshould be able to effectively re-use the buffers.\n\nThe other option would be to provide a Read-esque interface over a buffer\nchain, where each buffer has offsets into the actual payload.\nThis would mean, of course, that the downstream ecosystem would expect to\noperate on the same Read-esque interface, it is a bit too much to ask at\nthis point, but in the long run this is probably the correct answer.  I say\nRead-esque, because it would need to support string and char\nsearching/scanning style functionality to be performant for parsing and\ngeneral purpose use.\n\nAs an example,  this isn't the prettiest code, its still a POC, but I\nprovide a Read interface over a buffer chain, where each buffer has offsets\ninto its data, I then stream the data through 2 sets of parsers, each of\nwhich expects a read interface.\nhttps://github.com/rrichardson/lout/blob/master/src/gelf.rs#L68,\nhttps://github.com/rrichardson/lout/blob/master/src/gelf.rs#L155\n\n\nOn Thu, Sep 29, 2016 at 2:03 AM Paul Colomiets <notifications@github.com>\nwrote:\n\n> tokio-core may actually pick up a dependency on bytes after all\n>\n> While I'm not against bytes crate at all. I strongly believe that\n> contiguous buffers should be at least possible if not default. I think this\n> makes parsers simpler and faster.\n>\n> So is the text above means FramedIo will go and tokio-core depend on\n> Framed and BlockBuf ?\n>\n> —\n> You are receiving this because you were mentioned.\n> Reply to this email directly, view it on GitHub\n> <https://github.com/tokio-rs/tokio-core/pull/35#issuecomment-250410891>,\n> or mute the thread\n> <https://github.com/notifications/unsubscribe-auth/AAHlC_uUFjU3QMyGCFQn9F4Nvh6jTKpyks5qu37hgaJpZM4J-DyB>\n> .\n>\n"}}
{"actor":{"display_login":"rrichardson"}}