Opened 8 years ago

Last modified 3 years ago

#1930 enhancement new

create a libevent reactor

Reported by: PenguinOfDoom Owned by:
Priority: highest Milestone:
Component: core Keywords:
Cc: therve, antoine, oubiwann, zooko@… Branch: branches/libevent-1930-4
(diff, github, buildbot, log)
Author: therve Launchpad Bug:

Description

libevent provides primitives similar to addReader/addWriter/callLater. These could be used in a reactor.

Attachments (5)

1930_libeventreactor_1.py (3.6 KB) - added by therve 8 years ago.
Basic implementation of libevent reactor
libevent.dll (48.0 KB) - added by bigdog 8 years ago.
windows libevent.dll built with visual studio 2003
_libevent.pyd (27.5 KB) - added by therve 8 years ago.
test.patch (9.7 KB) - added by antoine 7 years ago.
libevent-gc.patch (17.0 KB) - added by antoine 7 years ago.

Download all attachments as: .zip

Change History (37)

comment:1 Changed 8 years ago by exarkun

  • Owner changed from glyph to PenguinOfDoom
  • Priority changed from normal to low

Changed 8 years ago by therve

Basic implementation of libevent reactor

comment:2 Changed 8 years ago by therve

I've tried to make a libevent reactor, but I'm a bit stuck for now (some segfaults appear in the tests). I attach my current work if somebody is interested.

comment:3 Changed 8 years ago by therve

  • Cc therve added

comment:4 Changed 8 years ago by therve

OK I've found the problem in libevent/eventmodule.c, in function __libevent_ev_callback (a problem with reference counting).

I have a fix, but I'm waiting for the livent-python to be up to see if it's not already corrected in the trunk. I'll send it to anyone interested.

comment:5 Changed 8 years ago by therve

It seems lots of work has been done here: source:/branches/slyphon/libevent-4/, using the libevent C bindings directly with Pyrex (I used the python bindings), and implementing callLater (I just did the FD management).

Changed 8 years ago by bigdog

windows libevent.dll built with visual studio 2003

comment:6 Changed 8 years ago by bigdog

If you need to build libevent on windows

1) #include <winsock2.h> must appear before #include<windows.h>

2) windows does not support function , create a macro that defined function

3) add #pragma to pull in socket lib (not sure if this is required for use in twisted/python. It was required to link the test app.

Changed 8 years ago by therve

comment:7 Changed 8 years ago by therve

I attached a build of the module for windows. I had to do some little hack to make it build. The tests doesn't pass for now, I'll probably need some help for this.

Oh, and I think using libevent in Windows use select, some that may not be that interesting (for now at least).

comment:8 Changed 8 years ago by therve

  • Keywords review added
  • Owner PenguinOfDoom deleted
  • Priority changed from low to highest

The branch libevent-1930-2 is in pretty good shape. The main missing thing is Windows support, but it would need some Windows hero.

comment:9 Changed 7 years ago by exarkun

  • Keywords review removed
  • Owner set to therve
  • test_libevent
    • test_tim eout is bad :/ I'm not really sure what should be done about it, but something needs to be. real-time using tests will fail spuriously. test_loop has a similar timing issue, and is also quite long.
    • test_signalHandler is scary.
  • libeventreactor.py
    • hooray per-instance state
    • reads/writes/selectables should probably be private. they should be doc'd, too. if you want to copy some strings out of the closed-socket-1537-3 branch for this, that's cool. I imagine some refactoring is on the horizon.
  • _libevent.c
    • I think EventBase_New leaks memory when event_init fails.
    • I think EventBase_CreateEvent leaks memory when either Event_Init or PyObject_CallMethod("setEventBase") fails.
    • __libevent_ev_callback calls several Python APIs (Py_DECREF, Py_CLEAR, PyErr_WriteUnraisable) without holding the interpreter lock. The comment about how exceptions are handled has the right general idea, but neither WriteUnraisable nor SetAsyncExc is proper. I think terminating the event loop actually is the right thing to do. If I understand correctly, this will actually only cause dispatch() to raise. A reactor implemented with this module can handle the exception there and then call dispatch again. A first pass of this reactor might continue to use PyErr_WriteUnraisable (albeit correctly), but if there's no pressing need to merge this branch (as I think there isn't), doing it the right way now might make sense.
    • In Event_SetEventBase, Py_XDECREF is used after a NULL check. Might as well drop the check and use Py_DECREF, since that's the only difference between the two.
    • There might be something fishy about the order of execution at the end of Event_SetEventBase. I wonder if decref'ing the existing eventBase could ever result in the new eventBase being freed. If the existing eventBase held the only reference to the new one, and nothing else held a reference to the existing eventBase, then the new eventBase would be freed immediately before the Py_INCREF call saves it. Then the function would probably crash.

comment:10 Changed 7 years ago by therve

(In [21069])

  • Remote time related tests
  • Add some missing decrefs
  • Cleanups

Refs #1930

comment:11 Changed 7 years ago by therve

(In [22068]) Remove a stupid test, some cleanups.

Refs #1930

comment:12 Changed 7 years ago by therve

  • author set to therve
  • Branch set to branches/libevent-1930-3

(In [22069]) Branching to 'libevent-1930-3'

comment:13 Changed 7 years ago by antoine

Hi

since I wanted something fun to do I created some tests to check for refcount problems. Attaching patch: there are some failures :-)

comment:14 Changed 7 years ago by therve

(In [22263]) Apply patch from antoine, adding failing tests for refcounting.

Refs #1930

comment:15 Changed 7 years ago by antoine

Oops, I have attached an updated version just before you committed the patch. There are a few more tests for persistent events.

comment:16 Changed 7 years ago by antoine

After playing a bit, it seems the flag EV_PERSIST does not make sense for pure timers, so the persist parameter to createTimer should probably be removed. Unless I've missed something :)

Changed 7 years ago by antoine

comment:17 Changed 7 years ago by antoine

I take back what I said, EV_PERSIST does make sense for "timers" because a timer is actually the loop timeout event. Sorry for posting too quick.

Changed 7 years ago by antoine

comment:18 Changed 7 years ago by antoine

Here is a new patch against the current branch contents. It contains additional tests and also modifies _libevent.c so that all tests pass (by enabling proper garbage collection of Event and EventBase objects, and maintaining internal structures to know which events are registered).

Unfortunately there is a remaining problem I haven't managed to fix: if I enable event_base_free(...) in the EventBase deallocator, some tests fail with an assertion error in libevent. This deallocation call wasn't present in the original version either.

comment:19 Changed 7 years ago by antoine

  • Cc antoine added

comment:20 Changed 7 years ago by therve

(In [22264]) Apply another patch, solving previous problems, with more tests.

Refs #1930

comment:21 Changed 7 years ago by therve

(In [22265]) Not all platforms raise EINPROGRESS, unfortunately.

Refs #1930

comment:22 follow-up: Changed 7 years ago by therve

Thanks a lot for the patch. I wasa wondering if we could use the libevent internal structures, and not storing the events on our own. But for now it does the trick.

comment:23 Changed 7 years ago by therve

(In [22266]) Remove some time related failures, cleanups.

Refs #1930

comment:24 in reply to: ↑ 22 Changed 7 years ago by antoine

Replying to therve:

Thanks a lot for the patch. I wasa wondering if we could use the libevent internal structures, and not storing the events on our own. But for now it does the trick.

We could get rid of expiredEvents:

  • either by trusting libevent not to use the event struct once the callback returns (this is true in the current libevent version, 1.4.1-beta), so we can directly decref the EventObject in the callback
  • or by using event_base_once() for non-persistent objects, but this is incompatible with the current Python API (which separates createEvent() and addToLoop()).

comment:25 Changed 7 years ago by therve

(In [22320]) Use Py_VISIT in traverse methods

Refs #1930

comment:26 Changed 7 years ago by therve

  • Branch changed from branches/libevent-1930-3 to branches/libevent-1930-4

(In [23487]) Branching to 'libevent-1930-4'

comment:27 Changed 4 years ago by <automation>

  • Owner therve deleted

comment:28 Changed 3 years ago by oubiwann

  • Cc oubiwann added

comment:29 Changed 3 years ago by oubiwann

comment:30 Changed 3 years ago by oubiwann

gevent announced last year that it is going to switch from libevent to libev (just read their blog post about this here: http://blog.gevent.org/2011/04/28/libev-and-libevent/).

Some interesting libev benchmarks to examine:

http://libev.schmorp.de/bench.html

comment:31 Changed 3 years ago by zooko

  • Cc zooko@… added

Some searching of the Internet has led me to these bits of code that involve (libevent or libev) and python:

I've just done some reading of various libev-vs.-libevent postings, and I was kind of undecided about which of the two sounded better until I decided to look for unit tests. This exists: https://github.com/libevent/libevent/tree/master/test , but find and grep don't show anything about unit tests or regression tests in libev. A search engine led me to a mailing list post from a few years ago which mentioned that libev's regression tests were in the "EV" perl module. I found them here: http://cvs.schmorp.de/EV/t/ . They appear to have been ported to a new version of libev 15 months ago and then none of them have been changed since, although libev has undergone substantial change in that time: http://cvs.schmorp.de/libev/Changes?r1=1.251&r2=1.209

The biggest of the those tests is http://cvs.schmorp.de/EV/t/11_signal.t?view=markup , which, while I don't read perl very well, looks like a fairly minimal test case. Looking in libevent for tests of signal handling, I see there are around ten functions that look to be at testing signal handling in https://github.com/libevent/libevent/blob/master/test/regress.c . Some of them have comments explaining what sort of bug this test would catch, like "find bugs in which operations are reordered": https://github.com/libevent/libevent/blob/539466e568ab7b676eece340b43ef98cae38c97a/test/regress.c#L899

I ran the libevent tests (the README instructed me to run "make verify" and that worked). They printed out reasonable sounding output for my platform, such as "skipping kqueue" and "testing epoll" (I'm on Linux 3.2, Ubuntu.) I couldn't figure out how to run the EV tests because perl couldn't find the EV module since I hadn't installed it into the path that perl searched.

Conclusion: if I were going to spend some of my time using or supporting either libevent or libev, it would be libevent.

P.S. Oh, about that benchmark, is it saying for example that if you had 10,000 open sockets with 1000 active clients, then libev takes a mere 10 milliseconds and libevent takes around 15 milliseconds to set up watchers, prepare the sockets and poll for events? Either of those sound like pretty good results to me. Since libevent seems to have good regression tests, I would guess that it is possible to make performance improvements to it while introducing fewer new bugs. The developers also seem to have a good idea to use measurement-driven optimization: http://sourceforge.net/tracker/?func=detail&aid=3137422&group_id=50884&atid=461325

Disclosure: I know primary maintainer of libevent, Nick Matthewson, personally and I quite like him, so this doubtless biased my evaluation.

comment:32 Changed 3 years ago by oubiwann

Very thorough write-up, Zooko -- thanks! If you were a salesman, I'd be walking off the lot with a car for every day of the week :-)

I'm convinced ;-)

Note: See TracTickets for help on using tickets.