PEP needed? Introducing Tcl objects
I have a patch that makes the Tcl object API available to _tkinter, in the sense that Tcl invocations don't necessarily return strings, but return Tcl objects (or appropriately converted Python objects). This both helps to improve efficiency, and to improve correctness of Tkinter applications since less type guessing is needed. For backward compatibility, there is an option on the tkapp object to determine whether strings or objects are returned. This is on by default when using Tkinter, but can be turned off through setting Tkinter.want_objects to 0. I found that IDLE still works fine when using Tcl objects. Do I need to write a PEP for this change, should I post a patch to SF, or should I just apply the change to the CVS? Regards, Martin
"MvL" == Martin v Loewis <martin@v.loewis.de> writes:
MvL> Do I need to write a PEP for this change, should I post a MvL> patch to SF, or should I just apply the change to the CVS? As this is an enhancement to a library, I don't think you'd need a PEP. -Barry
martin wrote:
For backward compatibility, there is an option on the tkapp object to determine whether strings or objects are returned. This is on by default when using Tkinter
"on" as in "return strings" or "return objects" ? I doubt it's a good idea to change the return type without any warning.
Do I need to write a PEP for this change, should I post a patch to SF, or should I just apply the change to the CVS?
if the default is "use old behaviour", check it in. if you insist on changing the return types, post it to SF. </F>
"Fredrik Lundh" <fredrik@pythonware.com> writes:
For backward compatibility, there is an option on the tkapp object to determine whether strings or objects are returned. This is on by default when using Tkinter
"on" as in "return strings" or "return objects" ?
In Tkinter, it returns objects by default.
I doubt it's a good idea to change the return type without any warning.
This is not as bad as it sounds. For most functions, the return type does not change at all. Consider def winfo_depth(self): """Return the number of bits per pixel.""" return getint(self.tk.call('winfo', 'depth', self._w)) 'winfo depth' will return a Tcl int in Tcl, which is currently converted to a string in _tkinter, then converted back to an int. With the change, tk.call will already return an int, so the getint invocation becomes a no-op. For others, a conversion into string will continue to return the value that it currently returns:
l=Tkinter.Label() l.config("foreground")[3] <color object at 0x0824b4b0> str(_) 'Black'
I would expect that few if any applications will be affected; those would need to change the default after import Tkinter.
if the default is "use old behaviour", check it in.
if you insist on changing the return types, post it to SF.
I'd like to change the return types. If that is not acceptable, I'd like to produce a DeprecationWarning if Tkinter is imported and the new-style behaviour (return objects) is not enabled. Regards, Martin
While we're on the topic of Tkinter, I got an email from Jeff Hobbs (Tcl guy at AS) re: Tkinter. He suspects that in: Py_BEGIN_ALLOW_THREADS PyThread_acquire_lock(tcl_lock, 1); tcl_tstate = tstate; result = Tcl_DoOneEvent(TCL_DONT_WAIT); tcl_tstate = NULL; PyThread_release_lock(tcl_lock); if (result == 0) Sleep(20); Py_END_ALLOW_THREADS The Sleep() call is a perf problem. If anyone wants to discuss it with Jeff, I've cc'ed him here. building bridges, --da
From: martin@mira [mailto:martin@mira]On Behalf Of Martin v. Loewis ... David Ascher <DavidA@ActiveState.com> writes:
The Sleep() call is a perf problem.
It certainly is, but it is also necessary to have.
Why? I suspect if you inverted the control behavior to run the Tcl event loop as it's designed and trigger signals with Tcl_AsyncMark, you would have no problem. Alternatively, you could do Tcl_CreateEventSource, of if threading is really necessary, build Tcl with threads and use Tcl_ThreadQueueEvent. It has all the APIs to approach this from several different angles without have to toss a gratuitous Sleep in there that does nothing more than have people scratch their head and wonder why Tkinter appears so slow. BTW, I know you were tying into Tk before Tk was properly thread-safe, but those issues have been addressed (although it is highly recommended to stick to using Tk in one thread as things like X aren't guaranteed to be thread-safe). Jeff
"Jeff Hobbs" <JeffH@ActiveState.com> writes:
It certainly is, but it is also necessary to have.
Why? I suspect if you inverted the control behavior to run the Tcl event loop as it's designed and trigger signals with Tcl_AsyncMark, you would have no problem. Alternatively, you could do Tcl_CreateEventSource, of if threading is really necessary, build Tcl with threads and use Tcl_ThreadQueueEvent.
Let me first state what I think what problem this Sleep call solves: it allows a thread switch to occur, by blocking the thread so that the OS knows that it should schedule a different thread. Otherwise, this thread would hold the tcl lock essentially forever, since releasing the tcl lock would be immediately followed by regaining it. Some thread implementation won't allow, in this case, other threads blocked for the tcl lock to run. In the light of this rationale, can you please explain what Tcl_AsyncMark is and how it would avoid the problem, or what effect calling Tcl_CreateEventSource would have, or how Tcl_ThreadQueueEvent would help?
It has all the APIs to approach this from several different angles without have to toss a gratuitous Sleep in there that does nothing more than have people scratch their head and wonder why Tkinter appears so slow.
It does more than that: it avoids people thinking that their threads have blocked indefinitely, for no good reason.
BTW, I know you were tying into Tk before Tk was properly thread-safe, but those issues have been addressed (although it is highly recommended to stick to using Tk in one thread as things like X aren't guaranteed to be thread-safe).
Let's assume thread-safety of X is not a problem (as it isn't in most current installations). Are you then saying that Tk is thread-safe? What is the minimum Tk version that makes this guarantee? Where is this documented? I'm all in favour of getting rid of the Tcl lock. Regards, Martin
The Sleep() call is a perf problem.
It certainly is, but it is also necessary to have.
Why? I suspect if you inverted the control behavior to run the Tcl event loop as it's designed and trigger signals with Tcl_AsyncMark, you would have no problem. Alternatively, you could do Tcl_CreateEventSource, of if threading is really necessary, build Tcl with threads and use Tcl_ThreadQueueEvent. It has all the APIs to approach this from several different angles without have to toss a gratuitous Sleep in there that does nothing more than have people scratch their head and wonder why Tkinter appears so slow.
Jeff, I really hope you can help us with this. I know it's a twisted mess. Years ago, I asked Ousterhout's help, but he was already too busy to pay attention to a competing language designer. :-( I hope that it's possible to do something better with Tcl/Tk 8.3 that doesn't require the sleep and maintains the existing _tkinter API / semantics.
BTW, I know you were tying into Tk before Tk was properly thread-safe, but those issues have been addressed (although it is highly recommended to stick to using Tk in one thread as things like X aren't guaranteed to be thread-safe).
Are they solved in Tcl/Tk 8.3? I'd be happy to require that version. I'm not (yet) happy to require an alpha/beta of 9.0 or whatever the Tcl community is now working at. --Guido van Rossum (home page: http://www.python.org/~guido/)
I hope that it's possible to do something better with Tcl/Tk 8.3 that doesn't require the sleep and maintains the existing _tkinter API / semantics.
I guess I would need to get a better understanding of why it was designed with the sleep in the first place. Martin mentioned that it allows a thread switch to occur, but a shorter sleep interval would have done the same. Tk 8.1+ has been thread-safe, but only in 8.3 have people been pushing it a little harder (most users of threads are Tcl-only). However, there are the different models between Python and Tcl threading, and perhaps that is a reason another method wasn't attempted early. Anyway, as to Martin's questions:
In the light of this rationale, can you please explain what Tcl_AsyncMark is and how it would avoid the problem, or what effect calling Tcl_CreateEventSource would have, or how Tcl_ThreadQueueEvent would help?
Tcl_AsyncMark is what you would call if you left the while loop looking more like: [global or tls] static stopRequested = 0; [in func] while (!stopRequested && foundEvent) { foundEvent = Tcl_DoOneEvent(TCL_ALL_EVENTS); } And whenever a signal occurs, you would do: ProcessSignal() { stopRequested = 1; Tcl_AsyncMark(asyncHandler); } The asyncHandler then has it's own callback routine of your choosing. Now this might not be what you want, as this is more the design for single-threaded systems that want an event loop. There is also the Tcl_CreateEventSource route. This allows you to provide a proc that gets called in addition to the internal Tcl one for processing events. This is most often used when tieing together event sources like Tk and Gtk, or Tk and MFC, ... You may simply need to call Tcl_SetMaxBlockTime. This will prevent Tcl from indefinitely blocking when no events are received. This may be the simplest solution to create the same effect as the Sleep, but without any other negative effects. Most all of these are described in fairly good detail here: http://www.tcl.tk/man/tcl8.3/TclLib/Notifier.htm Jeff
[Jeff Hobbs]
I guess I would need to get a better understanding of why it was designed with the sleep in the first place. Martin mentioned that it allows a thread switch to occur, but a shorter sleep interval would have done the same.
I believe Martin was correct in large part. The other part is that, without a sleep at all, we would have a pure busy loop here, competing for cycles non-stop with every process on the box. About the length of the sleep, do note that Sleep(20) sleeps 20 milliseconds here (not seconds), and that the sleep is skipped so long as Tcl_DoOneEvent() says it's finding things to do. IOW, Tcl gets all the cycles it can it eat so long as it says it's busy, and doesn't generally wait more than about 0.02 seconds for another chance after it runs out of work to do. Back when 20 was first picked, machines were slow enough that an utterly idle Tkinter app in the background still showed up as consuming a measurable percentage of a CPU, thanks to this not-so-busy loop. We could afford to make the sleep shorter on faster boxes, but I'm not sure I buy the argument that we're making Tcl/Tk look sluggish. The reason we hate the loop is more that it's a miserably ugly hack.
Tim Peters <tim.one@comcast.net> writes:
I believe Martin was correct in large part. The other part is that, without a sleep at all, we would have a pure busy loop here, competing for cycles non-stop with every process on the box.
Avoiding the wait to be busy is probably the #1 reason for the sleep. The alternative to avoid a busy wait would be to do Tcl_DoOneEvent with TCL_ALL_EVENTS, however, once Tcl becomes idle, this will block, depriving any other thread of the opportunity to invoke Tcl. Of Jeff's options, invoking Tcl_SetMaxBlockTime seemed to be most promising: I want Tcl_DoOneEvent to return after 20ms, to give other Tcl threads a chance. So I invented the patch Index: _tkinter.c =================================================================== RCS file: /cvsroot/python/python/dist/src/Modules/_tkinter.c,v retrieving revision 1.123 diff -u -r1.123 _tkinter.c --- _tkinter.c 26 Jan 2002 20:21:50 -0000 1.123 +++ _tkinter.c 19 Feb 2002 08:34:17 -0000 @@ -1676,7 +1967,11 @@ { int threshold = 0; #ifdef WITH_THREAD + Tcl_Time blocktime = {0, 20000}; PyThreadState *tstate = PyThreadState_Get(); + ENTER_TCL + Tcl_SetMaxBlockTime(&blocktime); + LEAVE_TCL #endif if (!PyArg_ParseTuple(args, "|i:mainloop", &threshold)) @@ -1688,16 +1983,15 @@ !errorInCmd) { int result; + #ifdef WITH_THREAD Py_BEGIN_ALLOW_THREADS PyThread_acquire_lock(tcl_lock, 1); tcl_tstate = tstate; - result = Tcl_DoOneEvent(TCL_DONT_WAIT); + result = Tcl_DoOneEvent(0); tcl_tstate = NULL; PyThread_release_lock(tcl_lock); - if (result == 0) - Sleep(20); Py_END_ALLOW_THREADS #else result = Tcl_DoOneEvent(0); However, it does not work. The script import Tkinter import thread import time c = 0 l = Tkinter.Label(text = str(c)) l.pack() def doit(): global c while 1: c+=1 l['text']=str(c) time.sleep(1) thread.start_new(doit, ()) l.tk.mainloop() ought to continously increase the counter in the label (once a second), but doesn't, atleast not on Linux, using Tcl 8.3.3. In the strace output, it appears that it first does a select call with a timeout, but that is followed by one without time limit before Tcl_DoOneEvent returns. Jeff, any ideas as to why this is happening? Regards, Martin
Martin, ...
Of Jeff's options, invoking Tcl_SetMaxBlockTime seemed to be most promising: I want Tcl_DoOneEvent to return after 20ms, to give other Tcl threads a chance. So I invented the patch ... ought to continously increase the counter in the label (once a second), but doesn't, atleast not on Linux, using Tcl 8.3.3. In the strace output, it appears that it first does a select call with a timeout, but that is followed by one without time limit before Tcl_DoOneEvent returns.
IIRC, Tcl_SetMaxBlockTime is a one-short call - it sets the next block time, not all block times. I'm sure there was a reason for this, but that was implemented before I was a core guy. Anyway, I think you just need to try: - result = Tcl_DoOneEvent(TCL_DONT_WAIT); + Tcl_SetMaxBlockTime(&blocktime); + result = Tcl_DoOneEvent(0); and see if that satisfies the need for responsiveness as well as not blocking. Thanks, Jeff
"Jeffrey Hobbs" <jeffh@ActiveState.com> writes:
IIRC, Tcl_SetMaxBlockTime is a one-short call - it sets the next block time, not all block times. I'm sure there was a reason for this, but that was implemented before I was a core guy. Anyway, I think you just need to try:
- result = Tcl_DoOneEvent(TCL_DONT_WAIT); + Tcl_SetMaxBlockTime(&blocktime); + result = Tcl_DoOneEvent(0);
and see if that satisfies the need for responsiveness as well as not blocking.
Thanks, but that won't help. Tcl still performs a blocking select. Studying the Tcl source, it seems that the SetMaxBlockTime feature is broken in Tcl 8.3. DoOneEvent has /* * If TCL_DONT_WAIT is set, be sure to poll rather than * blocking, otherwise reset the block time to infinity. */ if (flags & TCL_DONT_WAIT) { tsdPtr->blockTime.sec = 0; tsdPtr->blockTime.usec = 0; tsdPtr->blockTimeSet = 1; } else { tsdPtr->blockTimeSet = 0; } So if TCL_DONT_WAIT is set, the blocktime is 0, otherwise, it is considered not set. It then goes on doing if ((flags & TCL_DONT_WAIT) || tsdPtr->blockTimeSet) { timePtr = &tsdPtr->blockTime; } else { timePtr = NULL; } result = Tcl_WaitForEvent(timePtr); So if TCL_DONT_WAIT isn't set, it will block; if it is, it will busy-wait. Looks like we lose either way. In-between, it invokes the setupProcs of each input source, so that they can set a maxblocktime, but I don't think _tkinter should hack itself into that process. So I don't see a solution on the path of changing how Tcl invokes select. About thread-safety: Is Tcl 8.3 thread-safe in its standard installation, so that we can just use it from multiple threads? If not, what is the compile-time check to determine whether it is thread-safe? If there is none, I really don't see a solution, and the Sleep must stay. Regards, Martin
So if TCL_DONT_WAIT isn't set, it will block; if it is, it will busy-wait. Looks like we lose either way.
In-between, it invokes the setupProcs of each input source, so that they can set a maxblocktime, but I don't think _tkinter should hack itself into that process.
That's correct - I should have looked a bit more into what I did before (I was always tying in another GUI's event loop). However, I don't see why you should not consider the extra event source. Tk uses this itself for X. It would be something like: [in tk setup] Tcl_CreateEventSource(TkinterSetupProc, NULL, NULL); /* *---------------------------------------------------------------------- * * TkinterSetupProc -- * * This procedure implements the setup part of the Tkinter * event source. It is invoked by Tcl_DoOneEvent before entering * the notifier to check for events on all displays. * * Results: * None. * * Side effects: * The maximum block time will be set to 20000 usecs to ensure that * the notifier returns control to Tcl. * *---------------------------------------------------------------------- */ static void TkinterSetupProc(clientData, flags) ClientData clientData; /* Not used. */ int flags; { static Tcl_Time blockTime = { 0, 20000 }; Tcl_SetMaxBlockTime(&blockTime); } In fact, you can look at tk/unix/tkUnixEvent.c to see something similar already done in Tk.
About thread-safety: Is Tcl 8.3 thread-safe in its standard installation, so that we can just use it from multiple threads? If not, what is the compile-time check to determine whether it is thread-safe? If there is none, I really don't see a solution, and the
You would compile with --enable-threads (both Tcl and Tk). Jeff
"Jeff Hobbs" <JeffH@ActiveState.com> writes:
That's correct - I should have looked a bit more into what I did before (I was always tying in another GUI's event loop). However, I don't see why you should not consider the extra event source. Tk uses this itself for X. It would be something like:
That does not work, either. I'm using the patch attached below, and I'm getting the output ... setupproc called 729 setupproc called 730 setupproc called 731 setupproc called 732 setupproc called 733 setupproc called 734 setupproc called 735 ... That is, even though the setupproc is called, and even though the select is not blocking anymore, DoOneEvent does not return (I don't see the "Event done" messages). Regards, Martin Index: _tkinter.c =================================================================== RCS file: /cvsroot/python/python/dist/src/Modules/_tkinter.c,v retrieving revision 1.123 diff -u -r1.123 _tkinter.c --- _tkinter.c 26 Jan 2002 20:21:50 -0000 1.123 +++ _tkinter.c 22 Feb 2002 09:56:13 -0000 @@ -133,6 +139,10 @@ These locks expand to several statements and brackets; they should not be used in branches of if statements and the like. + To give other threads a chance to access Tcl while the Tk mainloop is + runnning, an input source is registered with Tcl which results in Tcl + not blocking for more than 20ms. + */ static PyThread_type_lock tcl_lock = 0; @@ -237,24 +248,6 @@ /**** Utils ****/ -#ifdef WITH_THREAD -#ifndef MS_WINDOWS - -/* Millisecond sleep() for Unix platforms. */ - -static void -Sleep(int milli) -{ - /* XXX Too bad if you don't have select(). */ - struct timeval t; - t.tv_sec = milli/1000; - t.tv_usec = (milli%1000) * 1000; - select(0, (fd_set *)0, (fd_set *)0, (fd_set *)0, &t); -} -#endif /* MS_WINDOWS */ -#endif /* WITH_THREAD */ - - static char * AsString(PyObject *value, PyObject *tmp) { @@ -1671,6 +1948,37 @@ /** Event Loop **/ +#ifdef WITH_THREAD +static int setupproc_registered; +/* + *---------------------------------------------------------------------- + * + * TkinterSetupProc -- + * + * This procedure implements the setup part of the Tkinter + * event source. It is invoked by Tcl_DoOneEvent before entering + * the notifier to check for events on all displays. + * + * Results: + * None. + * + * Side effects: + * The maximum block time will be set to 20000 usecs to ensure that + * the notifier returns control to Tcl. + * + *---------------------------------------------------------------------- + */ + +static void +TkinterSetupProc(ClientData clientData, int flags) +{ + static Tcl_Time blockTime = { 0, 20000 }; + static int i = 0; + printf("setupproc called %d\n",++i); + Tcl_SetMaxBlockTime(&blockTime); +} +#endif + static PyObject * Tkapp_MainLoop(PyObject *self, PyObject *args) { @@ -1682,22 +1990,29 @@ if (!PyArg_ParseTuple(args, "|i:mainloop", &threshold)) return NULL; +#ifdef WITH_THREAD + if (!setupproc_registered) { + Tcl_CreateEventSource(TkinterSetupProc, NULL, NULL); + setupproc_registered = 1; + } +#endif + quitMainLoop = 0; while (Tk_GetNumMainWindows() > threshold && !quitMainLoop && !errorInCmd) { int result; + #ifdef WITH_THREAD Py_BEGIN_ALLOW_THREADS PyThread_acquire_lock(tcl_lock, 1); tcl_tstate = tstate; - result = Tcl_DoOneEvent(TCL_DONT_WAIT); + result = Tcl_DoOneEvent(0); + printf("Event done\n"); tcl_tstate = NULL; PyThread_release_lock(tcl_lock); - if (result == 0) - Sleep(20); Py_END_ALLOW_THREADS #else result = Tcl_DoOneEvent(0); @@ -2033,12 +2364,10 @@ PyThread_acquire_lock(tcl_lock, 1); tcl_tstate = event_tstate; - result = Tcl_DoOneEvent(TCL_DONT_WAIT); + result = Tcl_DoOneEvent(0); tcl_tstate = NULL; PyThread_release_lock(tcl_lock); - if (result == 0) - Sleep(20); Py_END_ALLOW_THREADS #else result = Tcl_DoOneEvent(0);
In-between, it invokes the setupProcs of each input source, so that they can set a maxblocktime, but I don't think _tkinter should hack itself into that process.
BTW in addition to my last message, you might want to create an ExitHandler that delete the event source. Also, you might add more code to the TkinterSetupProc to only set a block time if multiple threads are actually used (or only create the event source at that time). This would make simple Tkinter apps be efficient and snappy all the time. Jeff
"Jeff Hobbs" <JeffH@ActiveState.com> writes:
BTW in addition to my last message, you might want to create an ExitHandler that delete the event source. Also, you might add more code to the TkinterSetupProc to only set a block time if multiple threads are actually used (or only create the event source at that time). This would make simple Tkinter apps be efficient and snappy all the time.
I'm not sure this will be necessary (provided I get this to work at all); after all, all that the timeout will do is to setup the event loop 50 times in a second. Computers should have no problems with that these days; in a snappy Tkinter app, there will be much more than 50 events per second. Furthermore, such a change would not affect snappiness at all, only efficiency (and only slightly so). Regards, Martin
FYI, someone just added a new entry to the ancient Python "Programs using Tkinter sometimes can't shut down (Windows)" bug report that everyone gave up on. John Popplewell claims to have found the (a?) cause, in part: """ Managed to track it down to a problem inside Tcl. For the Tcl8.3.4 source distribution the problem is in the file win/tclWinNotify.c """ Beats me, but a claim so specific is probably worth checking out: <http://sf.net/tracker/?func=detail&atid=105470&aid=216289&group_id=5470>
I added a note to the bug report, which was correct. The fix was already made in the Tcl head, but I back-ported it to the 8.3-branch of Tcl for those who want to be able to grab that and work against it. Jeff
-----Original Message----- From: Tim Peters [mailto:tim.one@comcast.net]
FYI, someone just added a new entry to the ancient Python "Programs using Tkinter sometimes can't shut down (Windows)" bug report that everyone gave up on. John Popplewell claims to have found the (a?) cause, in part:
""" Managed to track it down to a problem inside Tcl. For the Tcl8.3.4 source distribution the problem is in the file win/tclWinNotify.c """
Beats me, but a claim so specific is probably worth checking out:
<http://sf.net/tracker/?func=detail&atid=105470&aid=216289&group_id=5470>
I have a patch that makes the Tcl object API available to _tkinter, in the sense that Tcl invocations don't necessarily return strings, but return Tcl objects (or appropriately converted Python objects). This both helps to improve efficiency, and to improve correctness of Tkinter applications since less type guessing is needed.
Cool!
For backward compatibility, there is an option on the tkapp object to determine whether strings or objects are returned. This is on by default when using Tkinter, but can be turned off through setting Tkinter.want_objects to 0. I found that IDLE still works fine when using Tcl objects.
Do I need to write a PEP for this change, should I post a patch to SF, or should I just apply the change to the CVS?
My only worry is about breaking old apps. I'd like to see a patch on SF. No PEP is needed IMO. --Guido van Rossum (home page: http://www.python.org/~guido/)
participants (9)
-
barry＠zope.com -
David Ascher -
Fredrik Lundh -
Guido van Rossum -
Jeff Hobbs -
Jeffrey Hobbs
-
Martin v. Loewis -
martin＠v.loewis.de
-
Tim Peters