Added a simple Tcp Server wrapper.#17
Conversation
|
I have also removed the |
|
Hey Clark, This PR is trying to do too much for me to untangle. It is doing many things and changing original design decisions. It would be easier to tackle by opening issues w/ design proposals for discussion. Some of these changes go against the original design principles of MIO. I added some notes in the readme about what I am trying to achieve:
For example, the reason I went with the Token strategy vs. using boxed handlers was specifically to avoid requiring allocations. It would be not be hard to write a small shim on top of the token system for boxed handlers at a higher level. You can also ping me in IRC if you want to chat. There is definitely good stuff as well. |
|
I just want to point out that boxed handlers don't necessarily have to be run-time allocations, they can be whenever the user decides to create them. The token approach is definitely optimized towards the stream approach where you are waiting only on a socket in order to read data. I'd call this the stream driven approach. The other approach, which, I'll call the event driven approach, is more general, and that is the API is focused towards generic event registration/callback. There is nothing stopping the stream driven approach from being implemented directly on top of this with no additional overhead. In the event driven approach using a token, one will want to map a function to an event. to do so, one would have to build their own mapping of token to handler, which introduces a run-time lookup cost (of N log(N)) where N is the number of registered sockets. In summary: For the general case (i.e. not stream read/write), the handler approach gives the option of 0 runtime overhead, while the token approach does not. I am for the event driven approach. P.S. FWIW I do think that this is an incredibly ambitious pull request and should probably be broken into a few key ideas. |
|
Is there a particular IRC channel y'all hang out on? I'd love to talk about this with you. Aligning our objectives and breaking up this patch into smaller, more usable pieces is definitely something I'd like to work on. Sorry for not contacting you guys ahead of time before this "incredibly ambitious" patch. I had a last minute "weekend hackathon", and communication overhead would've stalled my work. Of course, now that it's done, I have all the time in the world for communication. :) |
Could you say more about how this is possible? epoll only allows 1 pointer worth of data, so you can either store the FD or the a pointer (box). If the pointer is used for a boxed handler, there needs to be 1 allocated handler per FD.
Using the token based approach and preallocating a slab for handlers, the runtime cost is O(1) on new socket AND socket lookup using the token based approach |
|
@cgaebel no worries, I'm on both the mozilla IRC server and freenode as @carllerche. |
|
I see that we are all on #rust@irc.mozilla.org should we make/use a new On Mon, Sep 22, 2014 at 1:52 PM, Clark Gaebel notifications@github.com
“Science is the great antidote to the poison of enthusiasm and |
|
Maybe if we all joined |
|
In. On Mon, Sep 22, 2014 at 2:02 PM, Clark Gaebel notifications@github.com
“Science is the great antidote to the poison of enthusiasm and |
That seems reasonable.
“Science is the great antidote to the poison of enthusiasm and |
Having to implement readable and writable is a huge pain. I tried to wrap up
the ugly bits in
server.rs, and ported the existing tests to use the newframework.
I'll spend a bit of time this week adding a bunch of documentation to the things
I've touched, but this should be ready for some review work now.
test_echo_server.rsis probably the best demonstration of what the new APIlooks like.
I was going to make the test spawn a whole bunch of clients to see how high I
could get the concurrency, but unfortuantely POSIX
connectlikes to returnEISCONNif a socket is non-blocking and I'm already connected to it. I thinkwe might need to do
connectin a thread pool to get lots of connections fromand to the same places.
This PR includes the Iobuf pull request. Feel free to close that one if you think
there's a good chance of this patch getting merged.