On Tue, Apr 8, 2008 at 2:58 PM, [EMAIL PROTECTED]
<[EMAIL PROTECTED]> wrote:
>
>  Thank you Drew for your, as always, insightful comments.
>
>  I'm pretty sure that an object which isn't garbage collected when it
>  goes out of scope would be considered a resource leak by most Python
>  programmers. It's certainly not clear without reading the source that
>  a ClockDisplay object does any scheduling at all -- it might simply be
>  updating itself occasionally in its draw method.
>
>  I'm willing to accept that this is a documentation problem, rather
>  than an API problem. However, it is definitely a problem.
>


I agree, it may be somewhat beneficial to drive home the point
somewhere in the documentation.  While the correct usage is shown in
the Pyglet Programmer's Guide, the module docstring shows
instantiation of the ClockDisplay in isolation and the side effect
isn't mentioned in the class docstring.  I think the following (while
nice and succinct) is particularly misleading:

"""
...

The ClockDisplay class provides a simple FPS counter::

    fps_display = clock.ClockDisplay()
    fps_display.draw()

"""

The above implicitly suggests you `should' create a new ClockDisplay
instance with every clock tick.
Suggested documentation changes are attached.



>  Martin
>
>  On Apr 8, 7:28 pm, "Drew Smathers" <[EMAIL PROTECTED]> wrote:
>  > On Tue, Apr 8, 2008 at 1:23 PM, [EMAIL PROTECTED]
>
>
> >
>  > <[EMAIL PROTECTED]> wrote:
>  >
>  > >  At the moment, ClockDisplay objects schedule themselves on the default
>  > >  clock in their initializers, but provide no facility for unscheduling.
>  > >  This means that ClockDisplay objects hang around forever, even after
>  > >  they've gone out of scope. This behaviour is mentioned nowhere in the
>  > >  documentation, and can cause huge slowdown if ClockDisplay objects are
>  > >  being regularly created.
>  >
>  > This is actually a memory leak in your code, not pyglet.  You should
>  > make the clockDisplay a global singleton - rather than recreating
>  > instances.
>  >
>  > >  Obviously, one can work around this by calling
>  > >  cd.clock.unschedule(cd.update_text) when the ClockDisplay is no longer
>  > >  required, but this seems hacky.
>  >
>  > That's not a work around ... sounds like the right way to do it.  The
>  > only point of confusion might be that the scheduling is implicit - but
>  > when would you want to create a ClockDisplay and not schedule it?
>  > Unscheduling it also wouldn't buy you much performace.
>  >
>  > >  A __del__ method seems like the
>  > >  obvious way of solving things ...
>  >
>  > Not exactly ... that would cause people to believe del would actually
>  > get called by the gc.  But when the gc calls __del__ is very
>  > non-deterministic - and won't occur if a method on the object is
>  > scheduled, of course.   Try playing with the following code, for
>  > example, and you find some interesting results:
>  >
>  > a.py
>  > ------
>  > class O(object):
>  >     def __del__(self):
>  >         print 'deleting'
>  > o = O()
>  >
>  > b.py
>  > -----
>  > from testns1 import o
>  > del o
>  >
>  > >  but unfortunately the clock keeps a
>  > >  reference to the scheduled method, so this doesn't work. Anyone have
>  > >  any ideas for a better solution?
>  >
>  > Summarzing:
>  >
>  > 1. Create one and only one instance of ClockDisplay.
>  > 2. Schedule/Unschedule explicitly with helper functions (or otherwise)
>  > if you really want to do such
>  >
>  > --
>  > \\\\\/\"/\\\\\\\\\\\
>  > \\\\/ // //\/\\\\\\\
>  > \\\/ \\// /\ \/\\\\
>  > \\/ /\/ / /\/ /\ \\\
>  > \/ / /\/ /\ /\\\ \\
>  > / /\\\ /\\\ \\\\\/\
>  > \/\\\\\/\\\\\/\\\\\\
>  >  d.p.s
>
> >
>



-- 
\\\\\/\"/\\\\\\\\\\\
\\\\/ // //\/\\\\\\\
\\\/ \\// /\ \/\\\\
\\/ /\/ / /\/ /\ \\\
\/ / /\/ /\ /\\\ \\
/ /\\\ /\\\ \\\\\/\
\/\\\\\/\\\\\/\\\\\\
 d.p.s

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"pyglet-users" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/pyglet-users?hl=en
-~----------~----~----~----~------~----~------~--~---

Index: pyglet/clock.py
===================================================================
--- pyglet/clock.py     (revision 1990)
+++ pyglet/clock.py     (working copy)
@@ -105,9 +105,13 @@
 Displaying FPS
 ==============
 
-The ClockDisplay class provides a simple FPS counter::
+The ClockDisplay class provides a simple FPS counter.  You should create
+an instance of ClockDisplay once during the application's start up::
 
     fps_display = clock.ClockDisplay()
+
+Call draw on the ClockDisplay object for each frame::
+
     fps_display.draw()
    
 There are several options to change the font, color and text displayed

Reply via email to