Re: [Libevent-users] Implementing a deadline timer
On Tue, Sep 09, 2008 at 04:48:40PM -0500, Andrew Troschinetz wrote: Hi all, I'm just getting started with libevent and I was wondering how I might implement a simple deadline timer with it. I'd like to be able to use do something like: void do_something(...) ... deadline_timer_wait(5, do_something); /* _non-blocking_, call do_something() in 5 seconds */ ... perhaps less than 5 seconds later ... deadline_timer_nevermind(); /* whatever we're waiting on, stop waiting, don't call do_something() */ However I'm a bit confused as to how event_dispatch() is supposed to work. Do I need to fork and then have the child call event_dispatch() in a loop: Definitely not. In this simple case, perform the initial timeout_set() and timeout_add(). Then call event_dispatch(). When the callback is called, do_something() in this case, do what you need to do, then re-add the timeout event if you want it to fire again. In the nevermind case, use timeout_del() to remove the timer. The callback for it will then never be called. However, please be sure to remember that if you have no events for libevent to handle (e.g. none added, or all removed), event_dispatch() will return. This means if you remove the timer from the event queue, and there are no other events being handled, event_dispatch() will definitely return. In an event loop paradigm like this, it is standard procedure to structure things as if you were inside of event_dispatch(), with the assumption that it will return only when your application is exiting. This means using things like queues, interval timers, etc. to make sure work is done. At first it may seem foreign, but remember that if one doesn't have anything to actually wait for, there's no reason for an event loop to even exist. In your case, what you want to wait for is the timer's consistent firing. In the case of cancellation, you're then either waiting for input from some other source to rearm this timer or you're exiting the application. -cl ___ Libevent-users mailing list Libevent-users@monkey.org http://monkeymail.org/mailman/listinfo/libevent-users
[Libevent-users] [PATCH] relative timer with EV_PERSIST support + move event_del logic into event_active + regression tests
1. Make EV_PERSIST reschedule timeouts automatically. 2. New function: timeout_schedule: (nothing really new within it, just modular wrapping of timeout rescheduling). 3. New macro: evutil_timercpy: tv_dst = tv_src in one line type deal; 3. Regression tests for persistent timeouts, include read/write, signals, and timers. 4. Another regression test for signal handler restores (no problem, just added another one). So what this means is that if you do the following: event_set(ev, fd, EV_READ | EV_PERSIST, read_cb, obj); event_add(ev, timeout); read_cb() will be called whenever a read event happens, and it's timeout as passed to event_add() will be reset to the original value you passed. You do not have to call event_add() within the handler. event_set(ev, -1, EV_TIMEOUT | EV_PERSIST, timer_cb, obj); event_add(ev, cycle); timer_cb() will be called when timeout (as passed via cycle) expires. It will then reschedule itself with it's original timeout, e.g. periodic timer. You do not have to call event_add() within the handler. For the event_del() changes, it's just moving event_del() into event_active(), when an event occurs. There is a feature request on sourceforge for this, and this couples nicely with the EV_PERSIST change. It also allows us to rexamine the logic tests within the various event dispatchers themselves, as event_active() will only delete the same event once. -cl $ test/regress Testing Priorities 1: OK Testing Priorities 2: OK Testing Priorities 3: OK Testing Evbuffer: OK Testing evbuffer_find 1: OK Testing evbuffer_find 2: OK Testing evbuffer_find 3: OK Bufferevent: OK Free active base: OK Testing HTTP Server Event Base: OK Testing HTTP Header filtering: OK Testing Basic HTTP Server: OK Testing Request Connection Pipeline : OK Testing Request Connection Pipeline (persistent): OK Testing Connection Close Detection: OK Testing HTTP POST Request: OK Testing Bad HTTP Request: OK Testing HTTP Server with high port: OK Testing HTTP Dispatcher: OK Testing Basic RPC Support: OK Testing Good RPC Post: OK Testing RPC Client: OK Testing RPC (Queued) Client: OK Testing RPC Client Timeout: OK DNS server support: OK Simple DNS resolve: type: 1, count: 1, ttl: 300: 152.160.49.201 OK IPv6 DNS resolve: type: 3, count: 1, ttl: 922: 2610:a0:c779:b::d1ad:35b4 OK Simple read: OK Simple write: OK Multiple read/write: OK Persist read/write: OK Combined read/write: OK Simple timeout: OK Persistent timeout: OK Persistent read/write timeout: OK Persistent signal timeout: OK Simple signal: OK Immediate signal: OK Loop exit: OK Multiple events for same fd: OK Want read only once: OK Testing Tagging: encoded 0x0af0 with 2 bytes encoded 0x1000 with 3 bytes encoded 0x0001 with 1 bytes encoded 0xdeadbeef with 5 bytes encoded 0x with 1 bytes encoded 0x00bef000 with 4 bytes evtag_int_test: OK evtag_fuzz: OK OK Testing RPC: (1.9 us/add) OK Signal dealloc: OK Signal pipeloss: OK Signal switchbase: OK Signal handler restore: OK Signal handler spread restore: OK Signal handler assert: OK $ make verify cd test make verify make[1]: Entering directory `/home/clayne/project/libevent.build/test' Running tests: KQUEUE Skipping test DEVPOLL Skipping test POLL test-eof: OKAY test-weof: OKAY test-time: OKAY regress: type: 1, count: 1, ttl: 300: 152.160.49.201 type: 3, count: 1, ttl: 238: 2610:a0:c779:b::d1ad:35b4 (1.9 us/add) OKAY SELECT test-eof: OKAY test-weof: OKAY test-time: OKAY regress: type: 1, count: 1, ttl: 300: 152.160.49.201 type: 3, count: 1, ttl: 1800: 2610:a0:c779:b::d1ad:35b4 (1.9 us/add) OKAY EPOLL test-eof: OKAY test-weof: OKAY test-time: OKAY regress: type: 1, count: 1, ttl: 300: 152.160.49.201 type: 3, count: 1, ttl: 1800: 2610:a0:c779:b::d1ad:35b4 (1.8 us/add) OKAY EVPORT Skipping test Index: event.c === --- event.c (revision 526) +++ event.c (working copy) @@ -125,4 +125,6 @@ static int timeout_next(struct event_bas static voidtimeout_process(struct event_base *); static voidtimeout_correct(struct event_base *, struct timeval *); +static int timeout_schedule(struct event_base *, struct event *, +struct timeval *); static void @@ -615,4 +617,5 @@ event_add(struct event *ev, struct timev const struct eventop *evsel = base-evsel; void *evbase = base-evbase; + min_heap_t *mh = base-timeheap; event_debug(( @@ -627,12 +630,4 @@ event_add(struct event *ev, struct timev if (tv != NULL) { - struct timeval now; - - if (ev-ev_flags EVLIST_TIMEOUT) - event_queue_remove(base, ev, EVLIST_TIMEOUT); - else if (min_heap_reserve(base-timeheap, - 1 + min_heap_size(base-timeheap)) == -1) -
Re: [Libevent-users] [PATCH] relative timer with EV_PERSIST support + move event_del logic into event_active + regression tests
On Tue, Nov 13, 2007 at 06:27:58PM -0800, William Ahern wrote: event_set(ev, fd, EV_READ | EV_PERSIST, read_cb, obj); event_add(ev, timeout); read_cb() will be called whenever a read event happens, and it's timeout as passed to event_add() will be reset to the original value you passed. You do not have to call event_add() within the handler. event_set(ev, -1, EV_TIMEOUT | EV_PERSIST, timer_cb, obj); event_add(ev, cycle); timer_cb() will be called when timeout (as passed via cycle) expires. It will then reschedule itself with it's original timeout, e.g. periodic timer. You do not have to call event_add() within the handler. What was the original behavior? Did the timer disappear? If not, I fail to see how the new behavior is more justificed than the old; meaning, instead of changing it, why not add it as an option? If waiting to read an entire line, but you are trickled one byte at a time, do you really want to reset the timeout between bytes, or is the timeout meant to limit the time spent on reading the line? The original behaviour with EV_PERSIST was this: 1. Implicitly not delete our event, but do not reschedule the timer. 2. Merrily read/write/signal/etc. on an event and then out of the blue receive EV_TIMEOUT because the timeout was never rescheduled unless you explicitly did it (which one probably won't inutitively expect with EV_PERSIST). We went over this one before though. The issue is that the attacker crafting 1 byte data to keep connection open for eternity thing, while valid - I don't really see how it's a libevent issue. That's an application issue. Otherwise, with EV_PERSIST, the timeout added to the event no longer plays the role of a timeout once a single event happens on said event, and the timeout is not rescheduled manually. The concept of a timeout being tied to if we get a single unit of y bytes means the timeout is then a sideband module within said event now. However, receive 1 byte == event occured - and based on - that timeout should no longer be relevant to the situation that already occured. The example you bring up of wanting to read y units of bytes in a given time is still possible without EV_PERSIST, or by using an associated timer event (probably not preferred), etc. It's just that the previous example is so much an exception to the standard idiom of: wait_until_event_or_timeout(); do_stuff() || delete_myself(); reset_timeout(); With EV_PERSIST as it is, it's only convenient for exceptional cases rather than the norm - and that's the focus of this patch - to reverse that. -cl For the event_del() changes, it's just moving event_del() into event_active(), when an event occurs. There is a feature request on sourceforge for this, and this couples nicely with the EV_PERSIST change. It also allows us to rexamine the logic tests within the various event dispatchers themselves, as event_active() will only delete the same event once. Will this cause incompatabilities? What if you call event_add() anyhow? Will it return failure? This could silently break code. Maybe the user set EV_PERSIST, realized it didn't do what he wanted, but never removed it. event_add() will then reschedule the event based on the timeout you passed to it. But since it's already implicit with EV_PERSIST, it would just be a redudant but safe thing to do. -cl ___ Libevent-users mailing list Libevent-users@monkey.org http://monkeymail.org/mailman/listinfo/libevent-users
[Libevent-users] [PATCH] test/Makefile.am: out of srcdir fix
Index: test/Makefile.am === --- test/Makefile.am(revision 515) +++ test/Makefile.am(working copy) @@ -31,6 +31,6 @@ test: test-init test-eof test-weof test-time regress verify: test - @./test.sh + @$(srcdir)/test.sh bench test-init test-eof test-weof test-time: ../libevent.la ___ Libevent-users mailing list Libevent-users@monkey.org http://monkeymail.org/mailman/listinfo/libevent-users
[Libevent-users] [PATCH] event.c: timeout_process(): let event_del() handle queue management
event_del() calls event_queue_remove(base, ev, EVLIST_TIMEOUT). -cl Index: event.c === --- event.c (revision 515) +++ event.c (working copy) @@ -815,7 +815,6 @@ while ((ev = min_heap_top(base-timeheap))) { if (evutil_timercmp(ev-ev_timeout, now, )) break; - event_queue_remove(base, ev, EVLIST_TIMEOUT); /* delete this event from the I/O queues */ event_del(ev); ___ Libevent-users mailing list Libevent-users@monkey.org http://monkeymail.org/mailman/listinfo/libevent-users
Re: [Libevent-users] [OT] craigslist: libevent programmer wanted
On Mon, Nov 12, 2007 at 06:38:33AM +0100, Marc Lehmann wrote: On Fri, Nov 09, 2007 at 12:39:37AM +0100, Hannah Schroeter [EMAIL PROTECTED] wrote: I see less problems with the writing away of the data sucked from the web servers, as most Unix like systems write stuff asynchronously, so the open(..., O_CREAT...), write() and close() calls won't be too slow. Most unix systems cache data for quite long, butwhen they write, usually user mode apps also halt. For throughput this is of little concern, but in a game server I wrote, even an fsync could freeze the server for 15-20 seconds(!) when another sync was in progress at the same time, or when some othe rprogram geenrated lots of I/O (for example a backup/restore). BTW: This isn't a global Linux issue, it's specifically an issue with ext3 and the way it handles fsync() on a global scale. http://kerneltrap.org/node/14148 Personally, I use XFS (awesome design). -cl ___ Libevent-users mailing list Libevent-users@monkey.org http://monkeymail.org/mailman/listinfo/libevent-users
[Libevent-users] [PATCH] signal.c: debug cleanups
1. Fix a debugging call with wrong format, (we should probably use __attr__((format(printf))) eventually). 2. Add additional debugging calls for sanity. -cl Index: signal.c === --- signal.c(revision 507) +++ signal.c(working copy) @@ -141,7 +141,7 @@ * a dynamic array is used to keep footprint on the low side. */ if (evsignal = sig-sh_old_max) { - event_debug((%s: evsignal sh_old_max, resizing array, + event_debug((%s: evsignal (%d) = sh_old_max (%d), resizing, __func__, evsignal, sig-sh_old_max)); sig-sh_old_max = evsignal + 1; p = realloc(sig-sh_old, sig-sh_old_max * sizeof *sig-sh_old); @@ -159,8 +159,9 @@ return (-1); } + /* setup new handler */ + event_debug((%s: %p: changing signal handler, __func__, ev)); #ifdef HAVE_SIGACTION - /* setup new handler */ memset(sa, 0, sizeof(sa)); sa.sa_handler = evsignal_handler; sa.sa_flags |= SA_RESTART; @@ -207,6 +208,7 @@ evsignal = EVENT_SIGNAL(ev); /* restore previous handler */ + event_debug((%s: %p: restoring signal handler, __func__, ev)); sh = sig-sh_old[evsignal]; sig-sh_old[evsignal] = NULL; #ifdef HAVE_SIGACTION ___ Libevent-users mailing list Libevent-users@monkey.org http://monkeymail.org/mailman/listinfo/libevent-users
[Libevent-users] [PATCH] event.c: make internal signal event count as active
[ WARNING: Long and ardorous - difficult to trace behaviour ] Behaviour described is without EV_PERSIST on internal signal handler (as it's been for months): 1. signal comes in, we jump to our internal signal handler, it writes to pipe. 2. epoll_wait() returns -1/EINTR because a signal handler was called. 3. we do not process epoll events here, we call evsignal_process() and return from dispatch. 4. our user-side event is deleted from queues, marked active, and processed normally BY evsignal_process(). 5. user-side event then adds itself back normally to handle signals again. 6. we head back into epoll_dispatch(), where epoll_wait() immediately returns because signal pipe is ready for reading (from step 1). 7. we implicitly delete the internal signal event (remember, this is without EV_PERSIST on it) and mark is as active. however, since it is an internal event, we do not increment event_count_active. 8. we return to our main event loop within event.c, and do not call event_process_active() because event_count_active == 0. this is where the problems really start. 9. we head back into epoll_dispatch() and wait normally. however, since we previously deleted our internal signal pipe fd from the epoll set, and did NOT call it from event_process_active() in the previous step - the pipe never ends up being read. but since the user callback was executed properly, everything looks normal. 10. we send the same signal again. epoll_wait() returns, and we call evsignal_process() again. 11. here's the funny part: evsignal_process() increments event_count_active - but since our internal signal event is actually first on the active queue at this point, when we go to process events in the main loop - we actually end up processing our internal signal event here. it reads from the pipe, and then re-adds itself back into epoll's watch set. but since it's internal, we do not decrement event_count_active - guaranteeing we'll get to our user event when we process the active events from the main loop. this is the flip-flop. 12. repeat. Now with EV_PERSIST on the internal signal event, steps 7-12 do not happen the first time the signal is called. So not only is the bug still there, it's super bad when it happens as epoll_wait() returns immediately. By sending a signal again - we can flip it from spin to wait. So this is a bug that needs to be fixed anyways. The EV_PERSIST change just made it more visible. I'm not so sure how to make a regression test for this other than a specific test to make sure that our signal pipe is empty AFTER returning from event_process_active(). -- test case: #include signal.h #include stdlib.h #include unistd.h #include event.h void scb(int sig, short event, void *a) { write(2, .\n, 2); event_add(a, NULL); return; } int main(void) { struct event sig; event_init(); event_set(sig, SIGTSTP, EV_SIGNAL, scb, sig); event_add(sig, NULL); event_dispatch(); return 0; } -- debug log (unpatched): $ ./sigtest [debug] event_add: event: 0x7fff762c0500,call 0x400768 [debug] event_queue_insert: 0x7fff762c0500: docount == 1, event_count == 1, queue == 4 [debug] evsignal_add: evsignal sh_old_max, s == 20, max == 0 [debug] event_add: event: 0x602050, EV_READ call 0x2ac034a166d4 [debug] event_queue_insert: 0x602050: docount == 0, event_count == 1, queue == 2 wait, signal [debug] evsignal_handler: wake up [debug] epoll_dispatch: epoll_wait() res == -1, errno = 4 [debug] evsignal_process: processing signals, caught == 1 [debug] event_del: 0x7fff762c0500, callback 0x400768 [debug] evsignal_del: 0x7fff762c0500: restoring previous handler [debug] event_active: 0x7fff762c0500: adding to active queue, ev_res == 0, res == 8 [debug] event_queue_insert: 0x7fff762c0500: docount == 1, event_count == 1, queue == 8 [debug] event_base_loop: event_count_active == 1 . [debug] event_add: event: 0x7fff762c0500,call 0x400768 [debug] event_queue_insert: 0x7fff762c0500: docount == 1, event_count == 1, queue == 4 [debug] epoll_dispatch: epoll_wait reports 1 [debug] event_del: 0x602050, callback 0x2ac034a166d4 [debug] event_active: 0x602050: adding to active queue, ev_res == 0, res == 2 [debug] event_queue_insert: 0x602050: docount == 0, event_count == 1, queue == 8 [debug] event_base_loop: event_count_active == 0 wait, signal (notice how we immediately call our internal event from event_base_loop?) [debug] evsignal_handler: wake up [debug] epoll_dispatch: epoll_wait() res == -1, errno = 4 [debug] evsignal_process: processing signals, caught == 1 [debug] event_del: 0x7fff762c0500, callback 0x400768 [debug] evsignal_del: 0x7fff762c0500: restoring previous handler [debug] event_active: 0x7fff762c0500: adding to active queue, ev_res == 8, res == 8 [debug] event_queue_insert: 0x7fff762c0500: docount == 1, event_count == 1, queue == 8 [debug] event_base_loop: event_count_active == 1 [debug] evsignal_cb: n == 2
Re: [Libevent-users] [PATCH] event.c: make internal signal event count as active
On Sat, Nov 10, 2007 at 06:44:12PM -0800, Christopher Layne wrote: 11. here's the funny part: evsignal_process() increments event_count_active - but since our should be: evsignal_process() adds the internal event to the active queue as normal. before we call event_process_active(), event_count_active is 1 at that point (user event). but since our ... internal signal event is actually first on the active queue at this point, when we go to process events in the main loop - we actually end up processing our internal signal event here. it reads from the pipe, and then re-adds itself back into epoll's watch set. but since it's internal, we do not decrement event_count_active - guaranteeing we'll get to our user event when we process the active events from the main loop. this is the flip-flop. -cl ___ Libevent-users mailing list Libevent-users@monkey.org http://monkeymail.org/mailman/listinfo/libevent-users
Re: [Libevent-users] [PATCH] event.c: make internal signal event count as active
On Sat, Nov 10, 2007 at 06:49:58PM -0800, Christopher Layne wrote: On Sat, Nov 10, 2007 at 06:44:12PM -0800, Christopher Layne wrote: 11. here's the funny part: evsignal_process() increments event_count_active - but since our Here's a better patch: This removes docount entirely. docount is used to determine if the event being added or removed from the queue should influence base-event_count. The internal signal event should not be counted as an event to wait for - such that when one deletes all their events the event loop will not count the internal signal event as something to wait around for (nothing changes here). However, based on the previous discussion, it still needs to be processed as a normal active event, hence we change base-event_count_active regardless of if it's internal or not. -cl Index: event.c === --- event.c (revision 507) +++ event.c (working copy) @@ -829,23 +829,17 @@ void event_queue_remove(struct event_base *base, struct event *ev, int queue) { - int docount = 1; - if (!(ev-ev_flags queue)) event_errx(1, %s: %p(fd %d) not on queue %x, __func__, ev, ev-ev_fd, queue); - if (ev-ev_flags EVLIST_INTERNAL) - docount = 0; - - if (docount) + if (~ev-ev_flags EVLIST_INTERNAL) base-event_count--; ev-ev_flags = ~queue; switch (queue) { case EVLIST_ACTIVE: - if (docount) - base-event_count_active--; + base-event_count_active--; TAILQ_REMOVE(base-activequeues[ev-ev_pri], ev, ev_active_next); break; @@ -866,8 +860,6 @@ void event_queue_insert(struct event_base *base, struct event *ev, int queue) { - int docount = 1; - if (ev-ev_flags queue) { /* Double insertion is possible for active events */ if (queue EVLIST_ACTIVE) @@ -877,17 +869,13 @@ ev, ev-ev_fd, queue); } - if (ev-ev_flags EVLIST_INTERNAL) - docount = 0; - - if (docount) + if (~ev-ev_flags EVLIST_INTERNAL) base-event_count++; ev-ev_flags |= queue; switch (queue) { case EVLIST_ACTIVE: - if (docount) - base-event_count_active++; + base-event_count_active++; TAILQ_INSERT_TAIL(base-activequeues[ev-ev_pri], ev,ev_active_next); break; ___ Libevent-users mailing list Libevent-users@monkey.org http://monkeymail.org/mailman/listinfo/libevent-users
Re: [Libevent-users] [PATCH] signal.c, evsignal.h: properly save/restore previous signal handlers and fix memory stomp
On Fri, Nov 09, 2007 at 04:03:46PM -0800, Scott Lamb wrote: Christopher Layne wrote: +/* save previous handler setup */ +if (sigaction(evsignal, NULL, sig-sa_old[evsignal]) == -1 +|| sigaction(evsignal, sa, NULL) == -1) Not worth changing unless you're redoing the patch anyway, but is there some reason you aren't doing this in a single call? I.e., if (sigaction(evsignal, sa, sig-sa_old[evsignal]) == -1) { Good idea. I'll change it and resubmit it with the regress.c patch also included (that Nick mentioned). ... -if (!base-sig.ev_signal_added) { -base-sig.ev_signal_added = 1; -event_add(base-sig.ev_signal, NULL); +if (!sig-ev_signal_added) { +sig-ev_signal_added = 1; +event_add(sig-ev_signal, NULL); } There's a bug here (that predates your change): this code should handle event_add() failure. (E.g., epoll_ctl() returning ENOMEM.) Fix in same, or sweep up in a later patch? How many other places are there where we're not currently checking the return value of event_add()? If there are more than this, we might as well just do it separately. -cl ___ Libevent-users mailing list Libevent-users@monkey.org http://monkeymail.org/mailman/listinfo/libevent-users
[Libevent-users] [PATCH] sample/Makefile.am: additional include
Separate build dir issue. Index: sample/Makefile.am === --- sample/Makefile.am (revision 506) +++ sample/Makefile.am (working copy) @@ -1,7 +1,7 @@ AUTOMAKE_OPTIONS = foreign no-dependencies LDADD = ../libevent.la -AM_CFLAGS = -I$(top_srcdir)/compat +AM_CFLAGS = -I$(top_srcdir) -I$(top_srcdir)/compat noinst_PROGRAMS = event-test time-test signal-test ___ Libevent-users mailing list Libevent-users@monkey.org http://monkeymail.org/mailman/listinfo/libevent-users
Re: [Libevent-users] sensible thread-safe signal handling proposal
On Sun, Nov 04, 2007 at 12:15:56PM -0800, Steven Grimm wrote: On Nov 4, 2007, at 8:13 AM, Marc Lehmann wrote: This would create additional loops (event_bases). The difference is that these cannot handle signals (or child watchers) at all, with the default loop being the only one to do signal handling. This seems like a totally sane approach to me. Having multiple loops is a big performance win for some applications (e.g., memcached in multithreaded mode), so making the behavior a bit more consistent is a good thing. It's only a performance win when the number of context switches and cache stomping, as a result of multiple threads cycling within their own context does not outweigh the latency of a model using less or even 1 thread. Consider a room with 20 people in it and a single door. The goal is to hand them a football as a new football is dropped off the assembly line and have them exit the door. You could throw them all a new football right as it comes off the line and have them immediately rush for the door - resulting in a log jam that one has to stop tending the assembly line to handle. You then head back to the line and begin the patterened task of throwing footballs to workers as fast as you can - only to have the log jam repeat itself. The only way to solve this efficiently is to have less people try and exit the door at once, or add more doors (CPUs). Now if only there were a way to wake just one thread up when input arrives on a descriptor being monitored by multiple threads... But that isn't supported by any of the underlying poll mechanisms as far as I can tell. -Steve It isn't typically supported because it's not a particularly useful or efficient path to head down in the first place. Thread pools being what they are, incredibly useful and pretty much the de facto in threaded code, do have their own abstraction limits as well. Setting up a thread pool, an inherently asynchronous and unordered collection of contexts, to asynchronously process an ordered stream of data (unless your protocol has no sequence, which I doubt), which I presume to somehow be in the name of performance, is way more complex and troublesome design than it needs to be. It's anchored somewhat to the every thread can do anything school of thought which has many hidden costs. The issue in itself is having multiple threads monitor the *same* fd via any kind of wait mechanism. It's short circuiting application layers, so that a thread (*any* thread in that pool) can immediately process new data. I think it would be much more structured, less complex (i.e. better performance in the long run anyways), and a cleaner design to have a set number (or even 1) thread handle the controller task of tending to new network events, push them onto a per-connection PDU queue, or pre-process in some form or fashion, condsig, and let previously mentioned thread pool handle it in an ordered fashion. Having a group of threads listening to the same fd has now just thrown our football manager out entirely and become a smash-and-grab for new footballs. There's still the door to get through. -cl ___ Libevent-users mailing list Libevent-users@monkey.org http://monkey.org/mailman/listinfo/libevent-users
Re: [Libevent-users] announcing libev, towards a faster and more featureful libevent
On Mon, Nov 05, 2007 at 02:42:16AM +0100, Marc Lehmann wrote: On Sun, Nov 04, 2007 at 05:19:36PM -0800, Christopher Layne [EMAIL PROTECTED] wrote: Not to put on my O-face, but binary heap insert is *average* O(1). There should be a performance win for libevent, when it comes to timer checking, as using a heap will also be O(1) for heap_min() - which will benefit pending timer calculation code. However, early delete w/ pending timer will need some rigging or tombstone games. I wouldn't be surprised that, in a case where one is consistently resetting timers (think read/write before x time passes) and re-adding said event, that in the end it will be the same amount of CPU time. No, because a red-black tree is much more complex in management, so even if both operations are O(log n), the heap usually wins hands down. Both insertion and removal are of the same complexity, on average, in a heap, of the data is random. However, libev has an interface (ev_timer_again) that incrementally updates the heap. Also, for repeating timers in general, there is no removal/insertion but only an incremental update. Right, which is not due to an inherent advantage of heap vs rbtree - but due to our luck in time always going in one direction. I believe similar code was present in libevent as it was. This in itself should be no different. Regarding pending events, this is solved in the same way for all events (not unlike how libevent does it): There is only one place where a pending event can be, and that is on its associated pending list. When an event gets stopped, and is found pending, it will be removed form the pending queue by nulling out its pointer. My point was that an event_del() on an event which has been called before it's timer has expired *or* an event_add() with a new timer will require reheapifying atleast part of the timer heap. Having an intrinsically sorted collection of elements and then altering a value within the middle of said collection before it has sifted to the top will require a reheap from that point on. Which isn't really that big a deal as similar time is spent in the present RB implementation as it is. I'm all for a binary-heap rather than a RB-tree personally. I think the performance will benefit primarily for heap_min() (which is done before every re-entry into the event backend to reset the max-wait timer for epoll/poll/select, etc). -cl ___ Libevent-users mailing list Libevent-users@monkey.org http://monkey.org/mailman/listinfo/libevent-users
Re: [Libevent-users] announcing libev, towards a faster and more featureful libevent
On Mon, Nov 05, 2007 at 02:46:36AM +0100, Marc Lehmann wrote: On Sun, Nov 04, 2007 at 04:09:08PM -0800, Scott Lamb [EMAIL PROTECTED] wrote: Have you seen the new Linux timerfd API? Where available, you can wait for CLOCK_MONOTONIC and CLOCK_REALTIME events independently. Beats heuristics, How? I still need to detect time jumps. If my ev_periodic is to be scheduled every minute, on the minute, and somebody resets the time the timer needs to be rescheduled. With timerfd I would need to detetc that and remove/insert the timer again. (You might have no use for periodics for timeouts, but they are designed to solve this very problem :) timerfd() has good and redundant points... as far as I can tell, it's an inversion of userkernel code that results in the same goal. http://lwn.net/Articles/245688/ -cl ___ Libevent-users mailing list Libevent-users@monkey.org http://monkey.org/mailman/listinfo/libevent-users
Re: [Libevent-users] [PATCH] TAILQ_ENTRY missing in evhttp.h on linux
On Sat, Nov 03, 2007 at 04:56:07PM -0700, Niels Provos wrote: Try #include sys/queue.h before including evhttp.h Niels. Why is this a usercode issue? Shouldn't evhttp.h be more interested in handling it's dependencies than non-event parent code? It's similar to the sys/time.h being a usercode, but event.h dependency as well. I mean why not make the parent code just handle all includes for event.h and evhttp.h - down to stdint.h while we're at it? Because that would be ridiculous - and the child header should handle it's own dependencies. I know Rob Pike may have thought at one time one shouldn't include other include files - but that was also 1989. -cl ___ Libevent-users mailing list Libevent-users@monkey.org http://monkey.org/mailman/listinfo/libevent-users
Re: [Libevent-users] [PATCH] TAILQ_ENTRY missing in evhttp.h on linux
On Sat, Nov 03, 2007 at 05:53:42PM -0700, Niels Provos wrote: Well, Rob Pike has many great insights. He also does not like conditional compilation. However, you are right, evhttp.h could resolve the dependency on queue.h internally. I will look into it. Agreed, Rob Pike is true blue - and I also respect him greatly - but what he wrote back then (including the grumble about not using include guards) just doesn't hold today. If one thinks about it in an abstract sense, it's inverting the child-module dependency maintenance onto the parent module. It was also somewhat hypocritical in that it was a form of compiler lexical analyzer pre-optimization in it's own right. -cl ___ Libevent-users mailing list Libevent-users@monkey.org http://monkey.org/mailman/listinfo/libevent-users
Re: [Libevent-users] [PATCH] Add autoconf/make functionality for --disable-dns, --disable-http, and --disable-bevents
On Thu, Sep 27, 2007 at 08:31:32AM -0700, Niels Provos wrote: Hi Christopher, I am not sure if this is necessarily the right way to go for a library, esp if it can impact backwards compatibility for bufferevents. As for reducing the size of the library, do you really think that 30K make a difference these days? Niels. It won't impact backwards compatibility unless someone explictly removed support for bufferevents. Additionally, where do we draw the line on what makes a difference these days, when it comes down to it? 1M? 2M? On a typical *bsd/linux platform, I agree, it won't make a significant difference at the end of the day. I also agree that using autoconf and automake to manipulate things is a hack in itself (they always feel like a hack unfortunately). I also don't know how many people are using libevent on smaller platforms (embedded, etc) but I thought that perhaps it could have had some kind of benefit. Part of my impetus was based on this post which directly identifies some concerns and ideas: -- From [EMAIL PROTECTED] Thu Feb 8 11:31:50 2007 From: [EMAIL PROTECTED] (Niels Provos) Date: Thu Feb 8 11:31:53 2007 Subject: [Libevent-users] [Patch] Third attempt: Add support for DNS servers to evdns.c (This time with regression tests!) In-Reply-To: [EMAIL PROTECTED] References: [EMAIL PROTECTED] [EMAIL PROTECTED] Message-ID: [EMAIL PROTECTED] Status: RO Content-Length: 720 Lines: 16 On 1/30/07, Wouter Wijngaards [EMAIL PROTECTED] wrote: Please, why put these really big http and dns protocols into an event handling library? I would prefer libevent to stay focused on providing portable select() and alternatives wrappers. The http and evdns are pretty big compared to the rest of libevent. They can be put in their own library(or -ies) perhaps? Some sort of libevent-driven application support? There has been some talk about libevent creating two different libraries during compile. One would be the traditional libevent and the other one would layer on top of it. Still thinking about the best way of doing this, but you are not the only one with concerns about bloat. Niels. -- So based on that, I went and wrote a patch. -cl ___ Libevent-users mailing list Libevent-users@monkey.org http://monkey.org/mailman/listinfo/libevent-users
[Libevent-users] [PATCH] Add autoconf/make functionality for --disable-dns, --disable-http, and --disable-bevents
$ ./configure --help [...] Optional Features: [...] --enable-dnsbuild with support for dns layer [default=yes] --enable-http build with support for http layer [default=yes] --enable-beventsbuild with support for buffer events [default=yes] Changes: 1. This required me to move buffer_* and evbuffer_* function declarations that were in event.h to a new file, evbuffer.h. Also, it looks like buffer.c contains ev_buffer_* functions and evbuffer.c contains buffer_* functions. This was like this before so I just tried to keep things consistent overall when creating a new header file and went with evbuffer.h. The flip-flop can be changed later. Also added evbuffer.h include to various http and bufferevents specific modules/headers that required them. 2. This required creating test/regress_buffer.c and moving bevents regress specific code into that module. Calls to higher level regress suite collection functions are conditionally ifdef'd via HAVE_EVENT_DNS, HAVE_EVENT_HTTP, etc. (which configure takes care of). 3. In both cases of above, the actual conditional building of modules is handled via ifdef's within Makefile.am that configure takes care of. 4. Modified configure.in to not bother doing Fortran, C++, or ObjC specific tests. Libtool likes to try it's own checks as well, but I just redefined those with simple null macros. 5. Throughout this all: substantially increased my derision for GNU auto* tools. Pros: 1. Significantly smaller libevent library footprint but the ability to include everything normally. 2. Default, nothing changes, all API layers are enabled per normal. 3. Couple of useful macros added to configure.in which should make adding any other enable/disable and build checks quicker. Cons: 1. http code is highly dependent on bufferevents code. As such, one cannot use --enable-bufferevents=no w/ --enable-http=no. If bufferevent code is disabled, http code must be disabled. 2. rpc code (which AFAICT is regress testing specific) is tied with http code and shares the same characteristics of above. Builds+size+make verify checks: $ set=( --disable-none \ --disable-dns \ --disable-http \ --disable-dns --disable-http \ --disable-bevents --disable-http \ --disable-bevents --disable-http --disable-dns ); \ a=${#set[*]}; i=0; while [[ $i -lt $a ]]; do f=${set[i++]}; echo $f; \ (./configure $f make clean make) 1/tmp/error.out 21 \ || (cat /tmp/error.out break); \ size .libs/libevent.so; \ make verify 21 | egrep FAIL break; \ printf \n; done --disable-none textdata bss dec hex filename 6711511206904 75139 12583 .libs/libevent.so --disable-dns textdata bss dec hex filename 4880410406328 56172db6c .libs/libevent.so --disable-http textdata bss dec hex filename 45589 840 708 47137b821 .libs/libevent.so --disable-dns --disable-http textdata bss dec hex filename 26603 728 132 274636b47 .libs/libevent.so --disable-bevents --disable-http textdata bss dec hex filename 37882 744 704 3933099a2 .libs/libevent.so --disable-bevents --disable-http --disable-dns textdata bss dec hex filename 18969 628 128 197254d0d .libs/libevent.so This patch is 50k, but is mostly just -+ of moving buffer-specific code from one file to another and the such. Attached as bzip2 here, and also raw at this url: http://www.anodized.com/~clayne/libevent.2007092600.diff -cl libevent.2007092600.diff.bz2 Description: Binary data ___ Libevent-users mailing list Libevent-users@monkey.org http://monkey.org/mailman/listinfo/libevent-users
Re: [Libevent-users] [PATCH] Do not call gettime in timeout_process if the timetree is empty
On Thu, Sep 20, 2007 at 01:52:31PM +0200, Trond Norbye wrote: The result of gettime is not used if the timetree is empty (and might cause a context switch to get the system clock). --Trond Index: event.c === --- event.c (revision 440) +++ event.c (working copy) @@ -804,24 +804,26 @@ void timeout_process(struct event_base *base) { - struct timeval now; - struct event *ev, *next; + if (!RB_EMPTY(base-timetree)) { + struct timeval now; + struct event *ev, *next; Why not just return if RB_EMPTY() is true rather than adding another level of indention? Other than that, I also noticed this but didn't consider it as anything significant. It has never once showed up as any significant source of cycles in profiling either. -cl ___ Libevent-users mailing list Libevent-users@monkey.org http://monkey.org/mailman/listinfo/libevent-users
[Libevent-users] [PATCH] test/Makefile.am: Correct a relative path issue
1. event_rpcgen.py will fail to open regress.rpc if building outside the source directory. -cl -- Index: Makefile.am === --- Makefile.am (revision 433) +++ Makefile.am (working copy) @@ -18,7 +18,7 @@ bench_SOURCES = bench.c regress.gen.c regress.gen.h: regress.rpc - $(top_srcdir)/event_rpcgen.py regress.rpc || echo No Python installed + $(top_srcdir)/event_rpcgen.py ${srcdir}/regress.rpc || echo No Python installed DISTCLEANFILES = *~ CLEANFILES = regress.gen.h regress.gen.c ___ Libevent-users mailing list Libevent-users@monkey.org http://monkey.org/mailman/listinfo/libevent-users
[Libevent-users] [PATCH] http.c build errors and a segfault (USE_DEBUG only)
Compilation errors in http.c and a segfault during make verify when USE_DEBUG is defined. Index: libevent/http.c === --- libevent/http.c (revision 429) +++ libevent/http.c (working copy) @@ -1174,7 +1174,7 @@ if (strchr(value, '\r') != NULL || strchr(value, '\n') != NULL || strchr(key, '\r') != NULL || strchr(key, '\n') != NULL) { /* drop illegal headers */ - event_debug((%s: dropping illegal header\n)); + event_debug((%s: dropping illegal header\n, __func__)); return (-1); } @@ -1499,7 +1499,7 @@ evcon-fd = bind_socket(evcon-bind_address, 0); if (evcon-fd == -1) { event_debug((%s: failed to bind to \%s\, - __func__, bind_address)); + __func__, evcon-bind_address)); return (-1); } @@ -2439,7 +2439,7 @@ if (ai == NULL) { event_debug((%s: make_addrinfo: \%s:%d\, - __func__, evcon-address, evcon-port)); + __func__, address, port)); return (-1); } ___ Libevent-users mailing list Libevent-users@monkey.org http://monkey.org/mailman/listinfo/libevent-users
Re: [Libevent-users] [PATCH] initialize ev_res
On Thu, Jul 19, 2007 at 01:14:58PM -0700, Scott Lamb wrote: When I use valgrind --tool=memcheck on a libevent-based program, it gives the following complaint: ==15442== Conditional jump or move depends on uninitialised value(s) ==15442==at 0x4C0F2D3: event_add (event.c:632) ==15442==by 0x405EE4: main_loop (net.c:356) ==15442==by 0x411853: main (tincd.c:329) Here's the relevant portion of event.c: 632if ((ev-ev_flags EVLIST_ACTIVE) 633(ev-ev_res EV_TIMEOUT)) { I've looked through the libevent code and verified that ev_res is always initialized when (ev_flags EVLIST_ACTIVE) is true, so there's no real bug here. You wouldn't happen to have a copy of the code or atleast code segment which was tickling this would you? When valgrind complains about uninitialised values, it's not in the fashion of gcc in that this could be uninitialised - so watch out type of noise. It really is an uninitalised area being read from. I also looked through event.c, and couldn't find a particular logic path that would cause it, nor could I get valgrind to duplicate it with simple libevent test code. Sometimes when valgrind indicates this error, I split the logic into two separate conditionals as it's not always apparent which particular object is at fault. It seems pretty obvious to me it wasn't ev_flags, as that was explicitly initialised, leaving only ev_res - but as I said I couldn't track it down here. Is it possible you were using an event struct which hadn't been passed to event_set() yet (just a sanity check for a simple bug)? However, it's useful to suppress noise in valgrind output, and there's no real cost to initializing ev_res at event_set time. Thus the attached patch. Certainly agreed, except for the noise part (http://www.valgrind.org/docs/manual/mc-manual.html#mc-manual.uninitvals). -cl ___ Libevent-users mailing list Libevent-users@monkey.org http://monkey.org/mailman/listinfo/libevent-users
Re: [Libevent-users] EV_PERSIST behavior
On Tue, May 08, 2007 at 06:38:51PM -0700, William Ahern wrote: received a non-timeout event. In a way this is absolute, not in the epoch sense, but that it's tied to the time event_add() was called and not relative to when the last valid event was received. That may or may not make sense the majority of the time. Most of the time you're reading logical data atoms, not bytes. Say you set a timeout for 10 seconds, but a socket polled as ready once every 9 seconds. Maybe there's a single byte available, maybe there's none (because the event notification was spurious). You're line buffering, and maybe you've set an upper bound on line length of 1000 bytes (e.g. SMTP). You could be polling for almost 3 hours if the peer trickled data byte-by-byte. If the peer was exceptionally smart, he could keep the connection open forever, by sending malformed packets which set the socket state as ready but were subsequently discarded further down the TCP stack. Sure, but this is already way higher level than libevent would be concerned with. Additionally it's easily (relatively) handled by keeping track of the exceptional states and reacting against it. Besides, libevent has no notion of completion or even a way to provide that by a particular metric or count. It's concerned with readiness about the most basic level. If an event is fired, the timeout should be implicitly reset to 0, awaiting either the next valid event to reset the timeout again or a timeout itself for which to make the timeout event valid in the first place. -cl ___ Libevent-users mailing list Libevent-users@monkey.org http://monkey.org/mailman/listinfo/libevent-users