Menu
▾
▴
[Tcl-bugs] [ tcl-Patches-1960647 ] Add ability to use poll() in TclUnixWaitForFile()
|
From: SourceForge.net <no...@so...> - 2009-03-10 08:03:01
|
Patches item #1960647, was opened at 2008-05-08 18:08 Message generated for change (Comment added) made by stwo You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=310894&aid=1960647&group_id=10894 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: 25. Channel System Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: Stuart Cassoff (stwo) Assigned to: Andreas Kupries (andreas_kupries) Summary: Add ability to use poll() in TclUnixWaitForFile() Initial Comment: This patch add the autoconf detection and C code necessary to use poll() instead of select() in TclUnixWaitForFile. Since this function only waits on one file descriptor, using poll() allows for slightly smaller code, less variables and a little less complexity. The code which uses select() has been altered to use the FD_SET macros in a similar manner as to what was done to tclUnixNotfy.c in this update: http://tcl.cvs.sourceforge.net/tcl/tcl/unix/tclUnixNotfy.c?r1=1.11.2.4&r2=1.11.2.5 Please scrutinize. ---------------------------------------------------------------------- >Comment By: Stuart Cassoff (stwo) Date: 2009-03-10 01:01 Message: Returning to this a little early (I was thinking one year of inactivity), I see that the FD_SET stuff hasn't even made it in, never mind the various cleanup/redo phantom proposals that have yet to materialize. ---------------------------------------------------------------------- Comment By: Joe English (jenglish) Date: 2008-06-15 11:34 Message: Logged In: YES user_id=68433 Originator: NO See also #1994512. ---------------------------------------------------------------------- Comment By: Daniel A. Steffen (das) Date: 2008-06-15 07:11 Message: Logged In: YES user_id=90580 Originator: NO Stuart, IMO the main difficulty with this patch is that it adds a dependency on poll(), which we have to detect and make sure is useable on all supported platforms, for instance on Darwin, poll() is known to be buggy in certain OS versions and with certain fd types (e.g. with devices in OSX 10.3 and earlier, c.f. google for more), it is very possible similar problems exist on other platforms. I'm not convinced it is worth the time & effort to do this if the whole approach to call select() or poll() in TclUnixWaitForFile() directly is flawed anyway... ---------------------------------------------------------------------- Comment By: Stuart Cassoff (stwo) Date: 2008-06-15 06:57 Message: Logged In: YES user_id=143350 Originator: YES Sounds good - I await your patch. ;) In the meantime ... This does not 'attack the wrong problem', it is a simple patch designed to make a couple of simple improvements, maybe save a bit of mem & time, as well as providing me with the valuable experience. ---------------------------------------------------------------------- Comment By: Daniel A. Steffen (das) Date: 2008-06-15 06:41 Message: Logged In: YES user_id=90580 Originator: NO Stuart, my point is that rather than integrate the adjustments made to tclUnixNotfy.c (which I agree should have been made here at the same time), TclUnixWaitForFile() should instead directly use the notifier machinery in tclUnixNotfy.c to wait for its one fd, the code in Tcl_WaitForEvent() already knows how to do that... ---------------------------------------------------------------------- Comment By: Stuart Cassoff (stwo) Date: 2008-06-15 06:23 Message: Logged In: YES user_id=143350 Originator: YES The purpose of this patch is to replace select() with poll() for the case of one fd, leaving in both implementations for purposes of testing and evaluation. One thing not to overlook please (unless it's already been done or doesn't matter) are the adjustments to the code using select() to match those done here: http://tcl.cvs.sourceforge.net/tcl/tcl/unix/tclUnixNotfy.c?r1=1.11.2.4&r2=1.11.2.5 ---------------------------------------------------------------------- Comment By: Daniel A. Steffen (das) Date: 2008-06-15 04:21 Message: Logged In: YES user_id=90580 Originator: NO IMO the fact that TclUnixWaitForFile() and Tcl_Sleep() call select() directly is fundamentally wrong anyway, they should be using the same mechanism as the notifier to suspend, i.e. in the threaded case the notifier thread should be doing the select for TclUnixWaitForFile(), same as waiting/polling from Tcl_WaitForEvent(). Calling select() directly prevents being woken up from another thread (except through a signal), which e.g. prevents being canceled via TIP285 during a waitforfile or sleep. Moreover, as I discovered in my current reworking of the Mac OS X CoreFoundation notifier to add support for embedding, the notifier not being used in TclUnixWaitForFile() and Tcl_Sleep() forces an alternative event loop to #ifdef out the standard unix versions of these and replace them (in my case with versions based on CFRunLoop to ensure the embedding event loop keeps processing events during waitforfile/sleep). If an embedder wants to replace the notifier via Tcl_SetNotifier() this workaround is not possible, and as TclUnixWaitForFile()/Tcl_Sleep() are not hookable, they will very likely be a big problem for any custom event loop (such as the Xt notifier)... This would all go away if the low-level waiting action in the notifier were factored out (i.e. signaling the notifier thread and waiting on the condition variable resp. calling select() in the single-threaded case), TclUnixWaitForFile() and Tcl_Sleep() could then call this new low-level wait instead of select() directly. I intend to work on doing exactly this in the next few weeks, as part of my look at the unix notifier to delay notifier thread creation until needed, so no need to waste further cycles looking at this patch, it attacks the wrong problem IMO. ---------------------------------------------------------------------- Comment By: Alexandre Ferrieux (ferrieux) Date: 2008-06-15 01:59 Message: Logged In: YES user_id=496139 Originator: NO Joe, if I read you correctly: > Anyway, this routine already has plenty of unnecessary complexity as it > is; adding an #ifdef HAVE_POLL branch would just double the amount. ... it seems that we agree ;-) I propose to Close/Reject. Better to concentrate our energy on cleaning such things up rather than letting them proliferate. Donal, your advice ? ---------------------------------------------------------------------- Comment By: Joe English (jenglish) Date: 2008-06-15 00:02 Message: Logged In: YES user_id=68433 Originator: NO Re: poll vs. select: It used to be the case that more systems had select() than had poll(), but that was years ago. Nowadays -- like, the last 10 or 15 years -- you can count on both being available. You have to go back pretty far to find a Unix that doesn't have poll() (BSD 4.3 and SunOS 3 are the latest examples I can find.) Neither poll() nor select() were included in POSIX.2 (circa 1993), although they were available in Unices of that era. The Open Group's Unix95 standard (1995) mandated both, and they've been part of the SUS ever since then. ---------------------------------------------------------------------- Comment By: Joe English (jenglish) Date: 2008-06-14 23:01 Message: Logged In: YES user_id=68433 Originator: NO (sorry, last comment was from me, I forgot to log in). Also: TclUnixWaitForFile() is also only called with mask = (TCL_WRITABLE | TCL_EXCEPTION). Anyway, this routine already has plenty of unnecessary complexity as it is; adding an #ifdef HAVE_POLL branch would just double the amount. ---------------------------------------------------------------------- Comment By: Nobody/Anonymous (nobody) Date: 2008-06-14 22:52 Message: Logged In: NO Some notes: (1) TclUnixWaitForFile() is only used in one place in the core proper, namely WaitForConnect(). (2) In WaitForConnect(), the "timeout" parameter is always either '0' (do not wait) or -1 (wait forever). So TclUnixWaitForFile() is much more complicated than it needs to be -- it doesn't need the "mini-event loop" or all the timeout processing at all. (3) WaitForConnect() and the TCP_ASYNC_SOCKET/TCP_ASYNC_CONNECT logic is also more complicated than it needs to be (and might not be necessary at all). (4) TclUnixWaitForFile() is also used in the test suite, this time with timeout values > 0. Whether the test suite is calling it simply to test TclUnixWaitForFile() itself or if it's needed to test something "real", I was not able to determine. (5) TclUnixWaitForFile also appears in the internal stubs table. Whether there are any callers outside of the core is difficult to determine. ---------------------------------------------------------------------- Comment By: Donal K. Fellows (dkf) Date: 2008-06-14 15:03 Message: Logged In: YES user_id=79902 Originator: NO I've poked around in the guts of select() and poll() on a few systems (long ago; many details forgotten) and one thing I remember is that on some systems you have poll() being used to implement select(), and on others it is the other way round! Both are common though; might even be that both are POSIX... ---------------------------------------------------------------------- Comment By: Alexandre Ferrieux (ferrieux) Date: 2008-06-14 14:36 Message: Logged In: YES user_id=496139 Originator: NO While I agree with the idea that the poll() API goes in the right direction, I'm wondering: this introduces a branch, two families of implementations depending on HAVE_POLL. Any idea of how many systems have and don't have poll today, and how many years the situation is expected to last, with the two branches to maintain ? ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=310894&aid=1960647&group_id=10894 |