search

flightaware / Tcl-bounties

Bounty program for improvements to Tcl and certain Tcl packages
104 stars 8 forks source link

[TIP] Intent to implement epoll()/kqueue() support for sockets on Linux/FreeBSD #14

Open lalbornoz opened 7 years ago

lalbornoz commented 7 years ago

Nov 23 4:30 PM: epoll() support on Linux in the event loop (tclUnixNotfy.c) implemented; some testing done. Nov 23 6:04 PM: kqueue/kevent() support on FreeBSD in the event loop (tclUnixNotfy.c): code written, compiles. Nov 23 7:52 PM: Cleanup; refactored code into four subroutines. Now commencing testing/bugfixing. Nov 24 12:16 PM: Code review; will contact tcl-core tomorrow. Nov 24 4:19 PM: Preliminarily pushed to https://github.com/lalbornoz/tcl (`epoll' branch.) Nov 25 1:51 PM: Initial tcl-core contact. Nov 25 5:18 PM: http://www.tcl.tk/cgi-bin/tct/tip/458.html Dec 6 7:33 PM: Supports DragonFly/Free/Net/OpenBSD and Linux; running relevant tests/* on those platforms and fixing bugs that come up.

dkfellows commented 7 years ago

http://core.tcl.tk/tcl/timeline?r=tip-458

lalbornoz commented 7 years ago

tests/ now all complete successfully on FreeBSD and Linux as of <96b5aa3016ee4e41b81534fbcccbab168d1a9338>, with a small number of tests that fail on Tcl v8.6.6 as well as w/ the new code. The output logs of $ make test w/ Tcl v8.6.6 and HEAD on both FreeBSD and Linux can be found in tcl_tests.log in the Github repository. The specific issue on FreeBSD appears to be that Linux returns EINPROGRESS for a connect(2) to an address that can synchronously be completed w/ ECONNREFUSED, whereas FreeBSD always returns ECONNREFUSED. This causes TcpConnect() to return TCL_ERROR instead of TCL_OK; the tests however expect either TCL_OK or TCL_RETURN and thus fail. FYI: the code now lives in the `tip458' branch of https://github.com/lalbornoz/tcl/.

lalbornoz commented 7 years ago

Awaiting feedback/vote/...

lehenbauer commented 7 years ago

OK, we are testing this.

lalbornoz commented 7 years ago

My new code was just committed: http://core.tcl.tk/tcl/info/858569fb0484a15478f8583287a263ac6215f20c. Assuming there are no remaining concerns, I'd like to claim the bounty.

nijtmans commented 7 years ago

TIP vote is done (accepted), and implementation is on trunk (8.7) now. Congratuations!

bll123 commented 7 years ago

FYI: The new epoll() code does not work correctly. https://core.tcl.tk/tcl/tktview?name=1a6a36d901

lalbornoz commented 7 years ago

I've pushed a fix concerning epoll_wait() timeout calculation in https://github.com/lalbornoz/tcl/commit/7249454d27938a9834bb4068d2994fdd98c24605. Can you confirm that this reduces CPU usage down to expected values w/ both the busyWait as well as the disconnected sockets tests?

bll123 commented 7 years ago

Looks much better. I ran the busy-wait test, use of dbus package test.

I don't have a specific test built for the disconnected socket, but I pulled up an older copy of my application and it seems ok.

With the patch (busy-wait test): bll-tecra:bll$ time $HOME/local87/bin/tclsh8.7 t.tcl ^C

real 0m7.413s user 0m0.029s sys 0m0.005s

dkfellows commented 7 years ago

Let me guess from the patch; the timeout was meant to be in ms, yet was being fed seconds?

lalbornoz commented 7 years ago

Precisely; the kqueue(2)-based notifier correctly converts between struct time{val,spec} and thus only Linux was/is affected.