From 723f72b0fb8cbd21a0ffbbd55e2005777fdb2840 Mon Sep 17 00:00:00 2001 From: Thomas Vander Stichele Date: Thu, 27 Oct 2005 08:51:20 +0000 Subject: [PATCH] gst/: remove the hash table for miniobjects - since we can't get notified when they get destroyed, we shouldn't be ca... Original commit message from CVS: * gst/gst.override: * gst/pygstminiobject.c: remove the hash table for miniobjects - since we can't get notified when they get destroyed, we shouldn't be caching pointer mappings * testsuite/test_pad.py: update refcount tests because mini objects now have a ref for each time an object is made for it --- ChangeLog | 10 ++++ gst/gst.override | 1 - gst/pygstminiobject.c | 109 +++++++++++------------------------------- testsuite/test_pad.py | 38 ++++++++++----- 4 files changed, 64 insertions(+), 94 deletions(-) diff --git a/ChangeLog b/ChangeLog index f4816f5b0a..33c3b1edc7 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,13 @@ +2005-10-27 Thomas Vander Stichele + + * gst/gst.override: + * gst/pygstminiobject.c: + remove the hash table for miniobjects - since we can't get notified + when they get destroyed, we shouldn't be caching pointer mappings + * testsuite/test_pad.py: + update refcount tests because mini objects now have a ref for + each time an object is made for it + 2005-10-26 Thomas Vander Stichele * testsuite/test_bus.py: diff --git a/gst/gst.override b/gst/gst.override index 13ca25b7f4..e45eb85ab0 100644 --- a/gst/gst.override +++ b/gst/gst.override @@ -274,7 +274,6 @@ init #endif if (!pygst_value_init()) return; - pygst_miniobject_init(); gst_controller_init(NULL, NULL); } %% diff --git a/gst/pygstminiobject.c b/gst/pygstminiobject.c index c84b68f955..2a0c69be87 100644 --- a/gst/pygstminiobject.c +++ b/gst/pygstminiobject.c @@ -35,14 +35,6 @@ static void pygstminiobject_dealloc(PyGstMiniObject *self); GST_DEBUG_CATEGORY_EXTERN (pygst_debug); #define GST_CAT_DEFAULT pygst_debug -static GHashTable *_miniobjs; - -void -pygst_miniobject_init () -{ - _miniobjs = g_hash_table_new (NULL, NULL); -} - /** * pygstminiobject_lookup_class: * @gtype: the GType of the GstMiniObject subclass. @@ -126,32 +118,9 @@ pygstminiobject_register_class(PyObject *dict, const gchar *type_name, PyDict_SetItemString(dict, (char *)class_name, (PyObject *)type); } -/** - * pygstminiobject_register_wrapper: - * @self: the wrapper instance - * - * In the constructor of PyGTK wrappers, this function should be - * called after setting the obj member. It will tie the wrapper - * instance to the Gstminiobject so that the same wrapper instance will - * always be used for this Gstminiobject instance. It will also sink any - * floating references on the Gstminiobject. - */ void pygstminiobject_register_wrapper (PyObject *self) { - GstMiniObject *obj = ((PyGstMiniObject *) self)->obj; - PyGILState_STATE state; - - g_assert (obj); - g_assert (GST_IS_MINI_OBJECT (obj)); - - state = pyg_gil_state_ensure (); - GST_DEBUG ("inserting self %p in the table for object %p [ref:%d]", - self, obj, GST_MINI_OBJECT_REFCOUNT_VALUE (obj)); - g_hash_table_insert (_miniobjs, (gpointer) obj, (gpointer) self); - GST_DEBUG ("There are now %d elements in the hash table", - g_hash_table_size (_miniobjs)); - pyg_gil_state_release (state); } @@ -160,63 +129,46 @@ pygstminiobject_register_wrapper (PyObject *self) * @obj: a GstMiniObject instance. * * This function gets a reference to a wrapper for the given GstMiniObject - * instance. If a wrapper has already been created, a new reference - * to that wrapper will be returned. Otherwise, a wrapper instance - * will be created. + * instance. A new wrapper will always be created. * * Returns: a reference to the wrapper for the GstMiniObject. */ PyObject * pygstminiobject_new (GstMiniObject *obj) { - PyGILState_STATE state; PyGstMiniObject *self = NULL; + PyGILState_STATE state; + PyTypeObject *tp = NULL; if (obj == NULL) { Py_INCREF (Py_None); return Py_None; } - /* see if we already have a wrapper for this object */ - state = pyg_gil_state_ensure (); - self = (PyGstMiniObject *) g_hash_table_lookup (_miniobjs, (gpointer) obj); - pyg_gil_state_release (state); - - if (self != NULL) { - GST_DEBUG ("had self %p in the table for object %p", self, obj); - /* make sure the lookup returned our object */ - g_assert (self->obj); - g_assert (self->obj == obj); - GST_INFO ("Increment refcount %p", self); - Py_INCREF (self); - } else { - GST_DEBUG ("have to create wrapper for object %p", obj); - /* we don't, so create one */ - PyTypeObject *tp = pygstminiobject_lookup_class (G_OBJECT_TYPE (obj)); - if (!tp) - g_warning ("Couldn't get class for type object : %p", obj); - if (tp->tp_flags & Py_TPFLAGS_HEAPTYPE) { - GST_INFO ("Increment refcount %p", tp); - Py_INCREF (tp); - } - self = PyObject_New (PyGstMiniObject, tp); - if (self == NULL) - return NULL; - self->obj = gst_mini_object_ref (obj); - - self->inst_dict = NULL; - self->weakreflist = NULL; - - /* save wrapper pointer so we can access it later */ - GST_DEBUG ("inserting self %p in the table for object %p [ref:%d]", - self, obj, GST_MINI_OBJECT_REFCOUNT_VALUE (obj)); - state = pyg_gil_state_ensure (); - g_hash_table_insert (_miniobjs, (gpointer) obj, (gpointer) self); - GST_DEBUG ("There are now %d elements in the hash table", - g_hash_table_size (_miniobjs)); - pyg_gil_state_release (state); - + /* since mini objects cannot notify us when they get destroyed, we + * can't use a global hash to map GMO to PyO, and have to create a new + * Python object every time we see it */ + tp = pygstminiobject_lookup_class (G_OBJECT_TYPE (obj)); + GST_DEBUG ("have to create wrapper for object %p", obj); + if (!tp) + g_warning ("Couldn't get class for type object : %p", obj); + if (tp->tp_flags & Py_TPFLAGS_HEAPTYPE) { + GST_INFO ("Increment refcount %p", tp); + Py_INCREF (tp); } + state = pyg_gil_state_ensure(); + self = PyObject_New (PyGstMiniObject, tp); + pyg_gil_state_release(state); + + if (self == NULL) + return NULL; + self->obj = gst_mini_object_ref (obj); + + self->inst_dict = NULL; + self->weakreflist = NULL; + + GST_DEBUG ("created Python object %p for GstMiniObject %p [ref:%d]", + self, obj, GST_MINI_OBJECT_REFCOUNT_VALUE (obj)); return (PyObject *) self; } @@ -227,15 +179,12 @@ pygstminiobject_dealloc(PyGstMiniObject *self) PyGILState_STATE state; - GST_INFO ("At the beginning %p", self); + GST_DEBUG ("At the beginning %p", self); state = pyg_gil_state_ensure(); if (self->obj) { - GST_DEBUG ("removing self %p from the table for object %p [ref:%d]", self, + GST_DEBUG ("PyO %p unreffing GstMiniObject %p [ref:%d]", self, self->obj, GST_MINI_OBJECT_REFCOUNT_VALUE (self->obj)); - g_assert (g_hash_table_remove (_miniobjs, (gpointer) self->obj)); - GST_DEBUG ("There are now %d elements in the hash table", - g_hash_table_size (_miniobjs)); gst_mini_object_unref(self->obj); GST_DEBUG ("setting self %p -> obj to NULL", self); self->obj = NULL; @@ -248,7 +197,7 @@ pygstminiobject_dealloc(PyGstMiniObject *self) self->ob_type->tp_free((PyObject *) self); pyg_gil_state_release(state); - GST_INFO ("At the end %p", self); + GST_DEBUG ("At the end %p", self); } static int diff --git a/testsuite/test_pad.py b/testsuite/test_pad.py index 10c65c6f77..b49299174a 100644 --- a/testsuite/test_pad.py +++ b/testsuite/test_pad.py @@ -21,8 +21,9 @@ # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA from common import gst, unittest, TestCase + import sys -import gc +import time class PadTemplateTest(TestCase): def testConstructor(self): @@ -96,20 +97,26 @@ class PadPushLinkedTest(TestCase): TestCase.tearDown(self) def _chain_func(self, pad, buffer): + gst.debug('got buffer %r, id %x, with GMO rc %d'% ( + buffer, id(buffer), buffer.__grefcount__)) self.buffers.append(buffer) return gst.FLOW_OK def testNoProbe(self): self.buffer = gst.Buffer() + gst.debug('created new buffer %r, id %x' % ( + self.buffer, id(self.buffer))) self.assertEquals(self.buffer.__grefcount__, 1) gst.debug('pushing buffer on linked pad, no probe') self.assertEquals(self.src.push(self.buffer), gst.FLOW_OK) gst.debug('pushed buffer on linked pad, no probe') - # pushing it takes a ref in the python wrapper to keep buffer - # alive afterwards; fakesink will get the buffer - self.assertEquals(self.buffer.__grefcount__, 1) + # one refcount is held by our scope, another is held on + # self.buffers through _chain_func + self.assertEquals(self.buffer.__grefcount__, 2) self.assertEquals(len(self.buffers), 1) + self.buffers = None + self.assertEquals(self.buffer.__grefcount__, 1) def testFalseProbe(self): id = self.src.add_buffer_probe(self._probe_handler, False) @@ -125,9 +132,13 @@ class PadPushLinkedTest(TestCase): self.buffer = gst.Buffer() self.assertEquals(self.buffer.__grefcount__, 1) self.assertEquals(self.src.push(self.buffer), gst.FLOW_OK) - self.assertEquals(self.buffer.__grefcount__, 1) + # one refcount is held by our scope, another is held on + # self.buffers through _chain_func + self.assertEquals(self.buffer.__grefcount__, 2) self.src.remove_buffer_probe(id) self.assertEquals(len(self.buffers), 1) + self.buffers = None + self.assertEquals(self.buffer.__grefcount__, 1) def _probe_handler(self, pad, buffer, ret): return ret @@ -166,10 +177,13 @@ class PadPushProbeLinkTest(TestCase): gst.debug('pushing buffer on linked pad, no probe') self.assertEquals(self.src.push(self.buffer), gst.FLOW_OK) gst.debug('pushed buffer on linked pad, no probe') - # pushing it takes a ref in the python wrapper to keep buffer - # alive afterwards; fakesink will get the buffer - self.assertEquals(self.buffer.__grefcount__, 1) + # one refcount is held by our scope, another is held on + # self.buffers through _chain_func + self.assertEquals(self.buffer.__grefcount__, 2) self.assertEquals(len(self.buffers), 1) + self.buffers = None + self.assertEquals(self.buffer.__grefcount__, 1) + def _probe_handler(self, pad, buffer): self.src.link(self.sink) @@ -246,8 +260,6 @@ class PadProbePipeTest(TestCase): self.assertEquals(sys.getrefcount(self.pipeline), 3) self.assertEquals(self.fakesrc.__gstrefcount__, 2) self.assertEquals(sys.getrefcount(self.fakesrc), 3) - self.assertEquals(self.fakesink.__gstrefcount__, 2) - self.assertEquals(sys.getrefcount(self.fakesink), 3) gst.debug('deleting pipeline') del self.pipeline self.gccollect() @@ -367,15 +379,15 @@ class PadRefCountTest(TestCase): self.assertEquals(pad.__gstrefcount__, 2) # added to element gst.debug('deleting element and collecting') - gc.collect() + self.gccollect() del e - self.assertEquals(gc.collect(), 1) # collected the element + self.assertEquals(self.gccollect(), 1) # collected the element self.assertEquals(sys.getrefcount(pad), 3) self.assertEquals(pad.__gstrefcount__, 1) # removed from element gst.debug('deleting pad and collecting') del pad - self.assertEquals(gc.collect(), 1) # collected the pad + self.assertEquals(self.gccollect(), 1) # collected the pad gst.debug('going into teardown') if __name__ == "__main__":