metajack / libstrophe

The libstrophe repository has moved to https://github.com/strophe/libstrophe
http://strophe.im/libstrophe
Other
135 stars 49 forks source link

Event system refactorizations to allow for optional external event loops #7

Closed anoek closed 3 years ago

anoek commented 13 years ago

Hey metajack,

Love the library.

I moved some code around and added a few functions to allow people to tie into your library but use their own event loop. I found your library very useful when putting together some stress tests, but i needed to beyond what a select loop could offer, so I made these modifications, tied in to libevent (which uses epoll/kqueue/etc), and was able to spin up tens of thousands of connections with ease.

The API for your built in event loop remains intact and functions exactly as it did before.

I also made a couple of changes to the autotools build files to enable builds on FreeBSD and to make the dynamic and static versions of the library, and the two header files, more easily installable.

naquad commented 12 years ago

Will this be ever integrated? Really need that.

metajack commented 12 years ago

I don't understand why libtool is needed. My experiences with that in the past have been nothing but painful, and I avoided using it for that reason. Does it do something you absolutely need here?

Also, if you want to make the select loop pluggable, why not make it pluggable similar to the XML parsing?

anoek commented 12 years ago

sock_t was renamed to xmpp_sock_t to match your naming convention of having all global identifiers prefixed with xmpp_. Prior to this patch, sock_t was only used internally, but with the addition of xmpp_conn_get_socket in strophe.h (allowing external event loops can obtain the underlying socket), xmpp_sock_t became a global identifier, and hence needed to be prefixed with xmpp_

xmpp_conn_write and xmpp_conn_read are exposed so that you can have an external event loop drive those actions when there is data to read/write. Did I miss another way of doing this?

The write callback exists so that when the library enqueus data to be sent, we can notify our external event loop that there is data to be written. (This isn't called when data is written to the pipe, this is called when data is queued up to be written, or when there is still data to be written after a partial write)

I don't think libtool is vital.. I've had the opposite experience, libtool has always treated me well. I don't think there was a strong reason for me changing it, I think it made my life easier linking to another project, I imagine others would benefit similarly, and since it doesn't change the .a's generated it shouldn't break existing builds using libstrophe.

Re making it pluggable similar to XML parsing - I thought I was :) The xmpp_conn_set_write_callback function was added as a way of binding what gets called when there is data to be written, similar to the handler functions you specified for the xml parsing. Perhaps a different name would have been more appropriate?

metajack commented 12 years ago

I figured it would work using an event.c which delegated to one of the plugins, of which the built in one is event_select.c. Then you'd make an event_kqueue.c, etc. This is the same way that there is a parser.c and a parser_expat.c and a parser_libxml2.c.

Is it common for people to want to write the kqueue event loop themselves? I've not encountered that in practice, but it's been a while since I've used tons of C libraries.

That said, I can see that on an embedded system you might have something more exotic for an eventloop. Perhaps it would work will to put these functions into the context. This would prevent them from leaking the abstraction, but allow exotic event loops to customize the behavior. This would be similar to how memory management works with pluggable allocators.

anoek commented 12 years ago

Oh I see what you mean. That would certainly be cleaner in the sense that the read/write details could be hidden again, and for supported event libraries integration would be easier. The downside of course is that libstrophe would have to explicitly support your event handling library, or you would have to live with maintaining a modified copy of libstrophe if you wanted to add your own.

I don't think it's too common for people to write their own kqueue/epoll event loops, but there is a good selection of libraries out there that people do use: libevent, libev, libaio, boost's asio, intel's libicaio, and undoubtedly others... and then of course there are plain old threads too I suppose.

Moving the functionality into the context is an interesting idea - so you're thinking basically have a user-replaceable set of socket functions, along with whatever support functions that may be necessary to glue the event loops together?

metajack commented 12 years ago

I was thinking specifically of moving your new functions into the context instead of making them public parts of the API.

sodabrew commented 12 years ago

Hi! I was just lurking by, and want to voice support for pluggable event loops. People would be coming in with their own libraries, often with instances of the event base coming from the code into which libstrophe is being linked. So libstrophe needs to be careful not to assume "ownership" of the event loop in this case. libevent, libev, libaio, ... yep @anoek has a good list there.

You wouldn't want to have event_kqueue. Maybe event_libevent -- but again note that the code pulling in libstrophe will have a better idea of how it wants to share it's event pointers. Callbacks are best.